Hi,

First I have to say I'm sorry for a bit slow reply, the patch didn't work out of the box and I had to check out where the problem was...

On 26.11.2008, at 5.21, Laurent Pinchart wrote:
This is not completely true. Controls are added to the entity controls list in
ascending index order, so manual controls will always come before auto
controls (at least for the processing unit).

Ah, in that case it is true because of how they are sorted. I didn't dare to count on it though...

However it works again for me and that's what matters now. I hope to get
this thing resolved some day in the official version too though. :)

Let's work on this then :-)

The patch indeed looked a bit messy at first. I've been trying to rework it
for the past hour but ran into various issues and I now understand
implementing a clean solution is not as easy as it might seem.

There are several places where control blacklisting can be implemented. If we want to use information taken from uvc_control_info (such as the control selector) the location you picked is the right one. However, the driver then
needs to walk the entities and controls lists over again and free data
allocated during the initialisation process.

Another solution would be to blacklist controls before they are allocated. We would then process the bmControls bitmask instead of the controls list. This approach is more efficient, but probably less generic as it won't allow the
driver to blacklist controls based on advanced information taken from
uvc_control_info.

As both solutions have advantages and drawbacks let's remember Linus Torvald and his famous quote about overdesign. Here's a simplified version of the patch that optionally (based on a device quirk) removes processing unit auto
controls when no corresponding manual control is available.

Could you please test the patch and report the results ?

I agree that this version is cleaner than mine, at least it doesn't free memory blocks and hold the mutex while doing it. :) Also adding the device quirk was something that I was thinking should be added but wanted to ask for comments first.

The latest patch had a small thinko though, you wrote "entity- >extension" in the controls and size getting when it should be "entity->processing" in this case. I'll attach a new tested version of the patch where that is fixed, it should have no other changes. Thanks again and tell let me know if there's something else I could/ should do. :)


Juho

Attachment: auto_control_fixup.diff
Description: Binary data




_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel

Reply via email to