From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from out3-smtp.messagingengine.com ([66.111.4.27]:43985 "EHLO out3-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753623AbaCMKsV (ORCPT ); Thu, 13 Mar 2014 06:48:21 -0400 Message-Id: <1394707700.15658.93976573.78252B46@webmail.messagingengine.com> From: Will Manley To: Laurent Pinchart Cc: linux-media@vger.kernel.org MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain Reply-To: will@williammanley.net In-Reply-To: <1854099.LO0jorujWf@avalon> References: <1394647711-25291-1-git-send-email-will@williammanley.net> <1854099.LO0jorujWf@avalon> Subject: Re: [PATCH] uvcvideo: Work around buggy Logitech C920 firmware Date: Thu, 13 Mar 2014 10:48:20 +0000 Sender: linux-media-owner@vger.kernel.org List-ID: On Thu, 13 Mar 2014, at 10:23, Laurent Pinchart wrote: > First of all, could you please CC me in the future for uvcvideo patches ? Will do. > On Wednesday 12 March 2014 18:08:31 William Manley wrote: > > The uvcvideo webcam driver exposes the v4l2 control "Exposure (Absolute)" > > which allows the user to control the exposure time of the webcam, > > essentially controlling the brightness of the received image. By default > > the webcam automatically adjusts the exposure time automatically but the > > if you set the control "Exposure, Auto"="Manual Mode" the user can fix > > the exposure time. > > > > Unfortunately it seems that the Logitech C920 has a firmware bug where > > it will forget that it's in manual mode temporarily during initialisation. > > This means that the camera doesn't respect the exposure time that the user > > requested if they request it before starting to stream video. They end up > > with a video stream which is either too bright or too dark and must reset > > the controls after video starts streaming. > > I've asked Logitech whether they can confirm this is a known issue. I'm > not > sure when I'll have a reply though. Great :) > > This patch works around this camera bug by re-uploading the cached > > controls to the camera immediately after initialising the camera. > > I'm a bit concerned about this. As you noticed UVC camera are often > buggy, and > small changes in the driver can fix problems with one model and break > others. > Sending a bunch of SET_CUR requests at once right after starting the > stream is > something that has the potential to crash firmwares (yes, they can be > that > fragile, unfortunately). Good point. I can add a quirk such that it only happens with the C920. > I would like to get a better understanding of the problem first. As I > don't > have a C920, could you please perform two tests for me ? > > I would first like to know what the camera reports as its exposure time > after > starting the stream. If you get the exposure time at that point (by > sending a > GET_CUR request, bypassing the driver cache), do you get the value you > had > previously set (which, from your explanation, would be incorrect, as the > exposure time has changed based on your findings), or a different value ? > Does > the camera change the exposure priority control autonomously as well, or > just > the exposure time ? It's a bit of a strange behaviour. I'd already tried littering the code with (uncached) GET_CUR requests. It seems that the value changes sometime during the call to usb_set_interface in uvc_init_video. Strangely enough though calling uvc_ctrl_restore_values immediately after uvc_init_video has no effect. It must be put after the usb_submit_urb loop to fix the problem. > Then, I would like to know whether the camera sends a control update > event > when you start the stream, or if it just changes the exposure time > without > notifying the driver. Wireshark tells me that it is sending a control update event, but it seems like uvcvideo ignores it. I had to add the flag UVC_CTRL_FLAG_AUTO_UPDATE to the uvc_control_info entry for "Exposure (Auto)" for the new value to be properly reported to userspace. Thanks Will