Am 20.02.2014 01:36, schrieb Laurent Pinchart: > Hi Oleksij, > > On Monday 17 February 2014 10:40:35 Oleksij Rempel wrote: >> Am 17.02.2014 10:28, schrieb Laurent Pinchart: >>> On Monday 17 February 2014 10:05:41 Oleksij Rempel wrote: >>>> Am 17.02.2014 01:26, schrieb Laurent Pinchart: >>>>> On Sunday 16 February 2014 10:59:32 Oleksij Rempel wrote: > > [snip] > >>>>> I don't know whether Microsoft has decided to address this issue in >>>>> their own way by sending a CLEAR_FEATURE(HALT) request when stopping the >>>>> video stream, or if that's a standard procedure in their UVC driver that >>>>> hasn't been added specifically for bulk endpoints. Neither the UVC >>>>> specification nor the USB specification forbid sending a >>>>> CLEAR_FEATURE(HALT) request to the streaming interface bulk endpoint in >>>>> this case. >>>>> >>>>> It should be noted that the USB specification allows devices to stall a >>>>> SET_INTERFACE(0) request for an interface that has a single alternate >>>>> setting (section 9.4.10). In that case the Linux USB core sends a >>>>> CLEAR_FEATURE(HALT) request to every endpoint of that interface. >>>> >>>> I tested this webcam with Windows Xp and 8. It was handled in same way: >>>> only CLEAR_FEATURE(HALT) was send. >>>> >>>>>> Logitech C905: (UVC - 1.00; ISO) >>>>>> • SET INTERFACE (Interface=1) >>>>>> • CLEAR HALT (Interface=0x7) Halt interrupt EP >>>>> >>>>> The CLEAR_FEATURE(HALT) request is targeted at endpoints, not >>>>> interfaces. >>>>> >>>>> I suppose you mean endpoint 0x87 here (EP 7 IN) ? >>>> >>>> yes >>>> >>>>> Similarly, with your bulk >>>>> webcam, to which endpoint is the CLEAR_FEATURE(HALT) request sent ? >>>> >>>> EP 1 IN (0x81) >>>> >>>>>> Logitech C600: (UVC - 1.00; ISO) >>>>>> • SET INTERFACE (Interface=1) >>>>>> • CLEAR HALT (Interface=0x7) Halt interrupt EP >>>>>> >>>>>> Logitech E3500: (UVC - 1.00; ISO) >>>>>> • CLEAR HALT (Interface=0x7) Halt interrupt EP >>>>>> • SET INTERFACE (Interface=1) (different order then in c600 and c905 ) >>>>>> >>>>>> Logitech C920: (UVC 1.00; ISO) >>>>>> • SET INTERFACE (Interface=1) >>>>>> • Interrupt is EP 3, why it is not halted? >>>>>> >>>>>> Asus UX31A (UVC - 1.00; ISO) - 04f2:b330 Chicony Electronics Co., Ltd >>>>>> Asus 720p CMOS webcam >>>>>> • SET INTERFACE (Interface=1) >>>>>> • Interrupt is EP 3, why it is not halted? >>>>>> >>>>>> So far it looks like Windows8 use CLEAR HALT for Bulk and SET INTERFACE >>>>>> for Iso. >>>>> >>>>> What about Windows XP and Windows 7 ? Do they use the same scheme ? >>>> >>>> Windows XP uses only SET INTERFACE on this webcam. Windows 7 not tested >>>> for now - should i? >>> >>> If Windows XP and Windows 8 behave the same way I suppose we can skip >>> Windows 7. >>> >>>>>> But i don't understand why Windows8 trying to halt Interrupt if EP=7 >>>>>> and not doing that if EP=3? >>>>> >>>>> No idea... I wonder if the Windows UVC driver includes device quirks. >>>> >>>> Yes, Windows 8 knows more about this webcam's then i would like to. For >>>> example it uses intensively Logitech specific XUs (without installing >>>> Logitech software): Right Light registers; some think what looks like >>>> watch dog. May be it knows how to disable LED :) NSA friendly system... >>> >>> Nice... If you've reversed-engineered RightLight information would be >>> welcome :-) >> >> That was easy: >> https://wikidevi.com/wiki/82066163-7050-ab49-b8cc-b3855e8d2252 >> See attachment with a testprogram. > > That's really interesting. Do you think we could integrate that in libv4l ?
May be, we should retest if we really can get better exposure. What was
your experience with it?
I'll collect more information about other XUs used by win driver. Right
now i have fallowing information:
- on stream start and stream end windows set LED in automatic mode.
- clear_halt is probably workaround for some bug which i get if i stop
the stream from virtual machine.
>>> What would you think about calling both usb_set_interface() and
>>> usb_clear_halt() for bulk-based devices ? Something like
>>>
>>> - usb_set_interface(stream->dev->udev, stream->intfnum, 0);
>>> + if (stream->intf->num_altsetting == 1)
>>> + usb_clear_halt(stream->dev->udev,
>>> + stream->header.bEndpointAddress);
>>
>> It works too. I was just afraid to be too different form MS implementation.
>> Manufactures usually test it against MS driver so it sort of dictate
>> specification.
>
> I think I agree with you. Could you please test the following patch ?
It is not working. Your patch is setting Interface to 0x80, but it
should set it to 0x81.
> From 04f3b43ba80ef5f3fbc26f81b8309dc166226b75 Mon Sep 17 00:00:00 2001
> From: Oleksij Rempel <li...@rempel-privat.de>
> Date: Sun, 16 Feb 2014 10:59:32 +0100
> Subject: [PATCH] uvcvideo: Do not use usb_set_interface on bulk EP
>
[snip]
>
> Replace selection of alternate setting 0 with clearing of the endpoint
> halt feature at video stream stop for bulk-based devices. Let's refrain
> from blaming Microsoft this time, as it's not clear whether this
> Windows-specific but USB-compliant behaviour was specifically developed
> to handle bulkd-based UVC devices, or if the camera just took advantage
> of it.
>
> CC: stable@vger.kernel.org
> Signed-off-by: Oleksij Rempel <li...@rempel-privat.de>
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_video.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c
> index 103cd4e..29147fd 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1850,7 +1850,19 @@ int uvc_video_enable(struct uvc_streaming *stream, int
> enable)
>
> if (!enable) {
> uvc_uninit_video(stream, 1);
> - usb_set_interface(stream->dev->udev, stream->intfnum, 0);
> + if (stream->intf->num_altsetting > 1) {
> + usb_set_interface(stream->dev->udev,
> + stream->intfnum, 0);
> + } else {
> + /* UVC doesn't specify how to inform a bulk-based device
> + * when the video stream is stopped. Windows sends a
> + * CLEAR_FEATURE(HALT) request to the video streaming
> + * bulk endpoint, mimic the same behaviour.
> + */
> + usb_clear_halt(stream->dev->udev,
> + stream->header.bEndpointAddress);
> + }
> +
> uvc_queue_enable(&stream->queue, 0);
> uvc_video_clock_cleanup(stream);
> return 0;
>
--
Regards,
Oleksij
signature.asc
Description: OpenPGP digital signature
