At 10:06 PM 10/17/2002 -0700, you wrote:
I'm not sure why those are a problem either. The dev_set_drvdata calls were used because the cpia driver uses them. I've used the cpia driver as my example of a webcam driver that works. I've probably misused something in the process.The use of the dev_set_drvdata family of calls. I'm not sure why, but those seem to keep the driver from being inserted, removed, and reinserted. Replacing those with the use of private_data field in the usb_interface struct seems to fix that (I removed the usb_put_device call along with these, so that my be to blame instead).
You're actually looking at a bug there. The proc interface for the shutter (which you don't seem to like) should be setting needsDummyRead. As the code says, the camera doesn't take changes in shutter speed until the frame after the one you capture. Any time shutter speed changes, you need to do a dummy read to get a properly exposed image.The control message allocation and async urbs, as previously mentioned.The is_initialized and needsDummyRead variables are unneeded as the things "protected" by those variables can/should just be done when the camera is initialized.
In both places, framebuf is only allocated if it is not already allocated. I think in both places the allocation is protected by a semaphore. Maybe it is wrong, could you give you more info?The disconnect handling is wrong for the same reasons mine was. There is no synchronization with the open/release routines, no making sure that no communication with the camera is going on, and the cam structure is freed without checking if users still have the camera open. framebuf is allocated in both open and mmap calls. Not necessarily wrong... however, it is possible that if the pointer was overwritten that the memory would just be reallocated/leaked instead of oopsing, which would indicate a problem.
The read function is written as such because the early usblib-based camera control software was heavily used and I wanted to make it possible for the already existing scripts to work as they had before. Thus the "complexity" of my read function is there so that users could do:There are some other, more subjective things that I take issue with such as the read function being unnecesarily complicated, the proc interface not being necessary, raw_image being allocated in open rather than probe, etc.
cp /dev/video/video0 mypic.raw
and it would work. The raw file could then be sent through ImageMagick to convert it into something usable.
I don't mind the proc interface going provided that there is a way to change the shutter speed independently for every camera connected. I have a lot of experience with users of this camera. Some have the camera up a tree (looking at scenes that vary in lighting throughout the day) and some have many cameras connected to a security monitoring system (with different cameras being under different lighting conditions). The gain control does not have enough range to cope with this, and gain degrades image quality anyway.
Here's why I disagree:Some of these things are easy, some are a bit more complicated. My biggest issue is that the driver that I submitted is a superset of the one that was merged. I did a lot of work to write/test/debug/etc. that code, and my doing that again would be a step backwards, in my mind.
Shutter speed. I'm not sure why, your code to set up the frame request packet looks a lot like my code to do the same, but you only included the slow shutter speed calculation. You also have a shutter speed bug, the camera can't actually capture anything slower than 1/4 sec. (it will take the value, but not give you the exposure).
Superior Color decode. Your color decoder looks like a lightly touched version of Andy Armstrong's original color decoder. In my driver I decided not to use that one, opting instead for Monroe Williams' better color decoder.
Removed V4L bug. Your V4L code looks like a modified copy of my 3comhc.c driver, right down to a bug in VIDIOCMCAPTURE.
In any case, I will assume responsibility for merging and debugging if you are not interested. I was of the opinion that if my driver were modified to include proper disconnect protection and asynchronous captures the driver would be feature complete until we reverse engineer the formats other than 512x242.
-Joe
-------------------------------------------------------
This sf.net emial is sponsored by: Influence the future of Java(TM) technology. Join the Java Community Process(SM) (JCP(SM)) program now. http://ad.doubleclick.net/clk;4699841;7576298;k?
http://www.sun.com/javavote
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
