Bug 114689

Summary: gnome-volume-manager does not detect Traveler SX-330Z camera
Product: [openSUSE] SUSE LINUX 10.0 Reporter: Marcus Meissner <meissner>
Component: GNOMEAssignee: Jeff Stedfast <fejj>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: aj, dkukawka, ro
Version: Beta 3   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Found By: Development Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: gnome-volume-manager-libgphoto.patch

Description Marcus Meissner 2005-09-01 12:20:17 UTC
The Traveller SX-330Z camera is not detected by gnome-volume-manager 
and so neither listed or opened for f-spot import. 
 
The camera is supported by libgphoto2 just fine. 
The camera is neither PTP nor USB massstorage.
Comment 1 Marcus Meissner 2005-09-01 12:21:15 UTC
Created attachment 48448 [details]
gnome-volume-manager-libgphoto.patch
Comment 2 Marcus Meissner 2005-09-01 12:25:18 UTC
Date: Tue, 30 Aug 2005 14:23:45 -0400 
From: Jeffrey Stedfast <fejj@novell.com> 
User-Agent: Mozilla Thunderbird 1.0.6-1.1.fc4 (X11/20050720) 
To: meissner@suse.de 
Subject: g-v-m libgphoto2 patch 
 
I've removed the patch from the build because it isn't correct. The 
problem is that with your patch, USB Mass Storage cameras will match 2 
device_added events. The first one will be caught by the libgphoto2 
camera.access_method change that you made, the second will be caught 
when HAL emits the device_added event for the camera's volume which will 
also invoke the camera import command (which is how USB Mass Storage 
camera devices are currently ahndled). 
 
what needs to happen is that the camera.access_method value needs to be  
different for usb vs ptp vs proprietary-access cameras. My proposal   
would be such: 
 
- USB Mass Storage cameras will have the value be, say, "storage" 
- PTP (which is a standard protocol) will have the value "ptp" 
- Proprietary access cameras would have the value "libgphoto2" if 
libgphoto2 could handle them else "proprietary" for any unknown 
proprietary access methods (for proprietary access methods that we can 
identify but aren't supported by libgphoto2, we could always replace 
"proprietary" with the actual protocol name) 
 
*Then* the gvm_udi_is_ptp_camera() check could be changed to match "ptp" 
|| "libgphoto2" without fear of the double-match that currently exists 
by checking libgphoto2 as the access_method value. 
 
Jeff 
 
Comment 3 Marcus Meissner 2005-09-01 12:25:52 UTC
Date: Wed, 31 Aug 2005 16:20:46 -0400 
From: Jeffrey Stedfast <fejj@novell.com> 
User-Agent: Mozilla Thunderbird 1.0.6-1.1.fc4 (X11/20050720) 
To: Marcus Meissner <meissner@suse.de> 
Subject: Re: g-v-m libgphoto2 patch 
 
 
 
Marcus Meissner wrote: 
 
>On Tue, Aug 30, 2005 at 02:23:45PM -0400, Jeffrey Stedfast wrote: 
> 
> 
>>I've removed the patch from the build because it isn't correct. The 
>>problem is that with your patch, USB Mass Storage cameras will match 2 
>>device_added events. The first one will be caught by the libgphoto2 
>>camera.access_method change that you made, the second will be caught 
>>when HAL emits the device_added event for the camera's volume which will 
>>also invoke the camera import command (which is how USB Mass Storage 
>>camera devices are currently ahndled). 
>> 
>> 
> 
>Huh? 
> 
>Only some cameras have both libgphoto2 and mass storage support 
>(Olympus, some Nikons and some Pentax) do. 
> 
>Perhaps you meant PTP here? 
> 
>    
 
the problem is that usb mass storage camera devices are also advertised 
as camera.access_method = "libgphoto2", at least they are on my machine. 
 
my ptp camera was advertised as "ptp", but now *also* "libgphoto2" 
 
if you chanegd the access_method variable to be "libgphoto2" for both, 
then it causes problems because usb mass storage cameras are handled at 
a different place in the code (e.g. when a storage volume is found... on 
usb mass storage cameras, the volume is detected after the camera device 
itself) 
 
hence the double detection. 
 
there's no way to distinguish the 2 and there NEEDS to be. 
 
> 
> 
>>what needs to happen is that the camera.access_method value needs to be 
>>different for usb vs ptp vs proprietary-access cameras. My proposal 
>>would be such: 
>> 
>>- USB Mass Storage cameras will have the value be, say, "storage" 
>>- PTP (which is a standard protocol) will have the value "ptp" 
>>- Proprietary access cameras would have the value "libgphoto2" if 
>>libgphoto2 could handle them else "proprietary" for any unknown  
>>proprietary access methods (for proprietary access methods that we can 
>>identify but aren't supported by libgphoto2, we could always replace 
>>"proprietary" with the actual protocol name) 
>> 
>> 
> 
>PTP cameras are handled by libgphoto2 too, so those 2 last cases 
>are basically the same. 
> 
>  
 
that doesn't make it a good idea to use "libgphoto2" as the 
camera.access_method for everything that libgphoto2 can handle. it's 
better to have the actual protocol so that apps writing down the line 
that don't use libgphoto2 can still work. Knowing that libgphoto2 can 
access them is worth fuck-all. 
 
> 
> 
>>*Then* the gvm_udi_is_ptp_camera() check could be changed to match "ptp" 
>>|| "libgphoto2" without fear of the double-match that currently exists 
>>by checking libgphoto2 as the access_method value. 
>> 
>> 
> 
>I don't really understand the problem :/ 
>But yes, double opens should not happen. 
> 
>But with the current version, one of the older proprietary cameras just 
>doesn't open automatically at all, neither is shown in g-v-m (but it is 
>supported by the f-spot import). 
> 
> 
 
presuming your camera's access_method was listed as "libgphoto2", a way 
to distinguish access methods needs to be worked out. perhaps a key 
camera.libgphoto2_support = true would be the way to go about this instead. 
 
USB Mass Storage camera: 
info.category = 'camera' 
camera.access_method = 'storage' 
camera.libgphoto2_support = 'true' 
 
PTP camera: 
info.category = 'camera' 
camera.access_method = 'ptp' 
camera.libgphoto2_support = 'true' 
 
Proprietary access method camera with libgphoto2 support: 
info.category = 'camera' 
camera.access_method = 'proprietary' 
camera.libgphoto2_support = 'true' 
 
Proprietary access method camera without libgphoto2 support: 
info.category = 'camera' 
camera.access_method = 'proprietary' 
camera.libgphoto2_support = 'false' 
 
 
At this point I could change g-v-m's udi_is_ptp_camera() to match 
against all non-storage cameras with libgphoto2_support = true 
 
>Is there a clear definition of the camera.* hal fields btw? 
> 
> 
 
not that I'm aware of 
 
 
Jeff 
 
Comment 4 JP Rosevear 2005-09-04 21:33:15 UTC
I'm unclear on what this report is about Marcus - does gvm not detect your
camera before or after Jeff removed your patch from GVM?
Comment 5 Marcus Meissner 2005-09-05 11:39:28 UTC
Its from before my patch, aj asked me to open it as tracker. 
 
and I just tested the newest g-v-m in autobuild, and the bug is back. 
 
The problem is: 
g-v-m handles only USB mass storage and PTP cameras.  
None of the other proprietary cameras supported by libgphoto2 are handled. 
 
I already have a HAL FDI file generated from libgphoto2s usb maps which 
would allow g-v-ms use, but fejj had concerns.  
 
I have not yet found the time to fully understand and address his concerns. 
 
In todays camera market the affected cameras are a minority. 
Comment 6 Marcus Meissner 2005-09-15 08:30:25 UTC
*** Bug 117172 has been marked as a duplicate of this bug. ***
Comment 7 Jeff Stedfast 2005-09-16 15:31:42 UTC
I submitted a proposal to the HAL maintainers the other day and they seemed to
like the idea:

From: 	Jeffrey Stedfast <fejj@novell.com>
To: 	hal@lists.freedesktop.org
Subject: 	camera.* attribute proposal
Date: 	Wed, 31 Aug 2005 16:36:37 -0400
Mailer: 	Mozilla Thunderbird 1.0.6-1.1.fc4 (X11/20050720)


Since the camera.* attributes seem to not be terribly useful atm and are 
working at making my life more difficult, I'd like to propose something 
that would work for me and (I hope) everyone else.

Currently there is a camera.access_method which, afaict, can currently 
hold at least 2 values: 'ptp' and 'libgphoto2' where 'libgphoto2' 
incldues at least usb mass storage cameras and some proprietary protocols.

The 'ptp' access_method value is actually meaningful. It tells me that 
the camera supports the ptp protocol. Horrah for standards!

'libgphoto2' tells me that libgphoto2 can access it. whoopty-doo. 
libgphoto2 can work with ptp cameras, usb mass storage cameras and a 
variety of proprietary protocols. If you stop and think about this, it 
pretty much means there's no use for 'ptp'.

if you get rid of 'ptp' tho, then the value becomes totally useless. 
might as well be camera.libgphoto2_support = true / false, but that 
isn't all that useful either.

Now, here's what I propose:

I propose that access_method becomes what it suggests it means, the 
actual access_method: 'ptp' 'storage' 'proprietary' etc...
I also propose that camera.libgphoto2_support = true / false be present 
for those who all they care about is that.

this means we have basically 4 possible scenarios (unless we want to get 
even more specific about proprietary access_methods)

USB Mass Storage camera:
info.category = 'camera'
camera.access_method = 'storage'
camera.libgphoto2_support = 'true'

PTP camera:
info.category = 'camera'
camera.access_method = 'ptp'
camera.libgphoto2_support = 'true'

Proprietary access method camera with libgphoto2 support:
info.category = 'camera'
camera.access_method = 'proprietary'
camera.libgphoto2_support = 'true'

Proprietary access method camera without libgphoto2 support:
info.category = 'camera'
camera.access_method = 'proprietary'
camera.libgphoto2_support = 'false'


Of course, feel free to rename 'storage' and 'proprietary' to whatever 
you guys would prefer. And as I hinted at above, feel free to replace 
'proprietary' with actual proprietary protocol names if that's what 
you'd prefer to do (I think in the long run it'd be a nice addition but 
in the interest of "easy fix" 'proprietary' would satisfy me)

Is this doable?

Jeff
Comment 8 Jeff Stedfast 2005-09-16 15:35:16 UTC
One response I got worth noting (other than the "hurrah! we love your idea!"
replies) was:

From: 	Jeffrey Stedfast <fejj@novell.com>
To: 	Artem Kachitchkine <Artem.Kachitchkin@Sun.COM>
Cc: 	hal@lists.freedesktop.org
Subject: 	Re: camera.* attribute proposal
Date: 	Wed, 31 Aug 2005 20:51:50 -0400
Mailer: 	Evolution 2.2.0 	


On Wed, 2005-08-31 at 16:39 -0700, Artem Kachitchkine wrote:
> > Currently there is a camera.access_method which, afaict, can currently 
> > hold at least 2 values: 'ptp' and 'libgphoto2' where 'libgphoto2' 
> > incldues at least usb mass storage cameras and some proprietary protocols.
> 
> According to the spec:
> 
>
http://cvs.freedesktop.org/*checkout*/hal/hal/doc/spec/hal-spec.html?only_with_tag=HEAD#device-properties-camera
> 
> camera.access_method can be either 'storage' or 'user'.
> And there's the camera.libgphoto2.support boolean property that says 
> whether the device is supported by libgphoto2.

yes, but unfortunately the only 2 values that get set in practice are
"ptp" and "libgphoto2" (at least on FC4 and SuSE "10")
Comment 9 Marcus Meissner 2005-09-16 15:55:24 UTC
there is a small miss conception.... 
 
libgphoto2 itself now supports mass storage cameras too (by using hal & 
mounts). 
 
The FDI file however that I export from libgphoto2 is the list of USB ids that 
libgphoto2 supports directly and that have non-mass storage drivers. 
 
However ... there is a slight overlap between massstorage cameras and above 
list, notably for the Olympus cameras and some Nikons and Pentax cameras. This 
overlap is probably just around 20 entries. 
 
The mass of mass storage cameras is NOT in the libgphoto2 exported FDI map. 
Comment 10 Jeff Stedfast 2005-09-16 17:59:50 UTC
g-v-m needs the distinction. we need to mount mass-storage cameras.

(I know that libgphoto2 can handle mass storage)
Comment 11 Jeff Stedfast 2005-10-12 17:42:44 UTC
this should now work
Comment 12 Jeff Stedfast 2005-10-18 15:27:16 UTC
*** Bug 128975 has been marked as a duplicate of this bug. ***