From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Hans de Goede <hansg@kernel.org>
Cc: Michael Jordan <jordan.mymail@gmail.com>,
Ricardo Ribalda <ribalda@chromium.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil+cisco@kernel.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/3] media: uvcvideo: live pan/tilt/zoom readback on the OBSBOT Tiny 2
Date: Mon, 28 Sep 2026 14:49:00 +0300 [thread overview]
Message-ID: <20260928114900.GC157191@killaraus.ideasonboard.com> (raw)
In-Reply-To: <f8134f74-9f32-4c98-96b3-ceeb978abb11@kernel.org>
On Mon, Sep 28, 2026 at 01:01:11PM +0200, Hans de Goede wrote:
> Hi All,
>
> On 2-Sep-26 02:25, Michael Jordan wrote:
> > The OBSBOT Tiny 2's GET_INFO strips the AUTOUPDATE bit from its pan,
> > tilt and zoom controls, so uvcvideo caches them and userspace can never
> > observe the actuator's live state. This series reports AUTO_UPDATE
> > controls as volatile to userspace, generalises the existing XU flags
> > fixup table to all controls, and adds entries for the three affected
> > controls on this camera.
> >
> > Changes in v2 (following Ricardo's review of v1 [1]):
> >
> > - Patch 1: unchanged; picked up Ricardo's Reviewed-by.
> > - Patch 2: uvc_ctrl_fixup_flags() now returns bool and runs at the
> > start of uvc_ctrl_get_flags(), before the allocation, skipping the
> > GET_INFO query entirely for controls the table covers (Ricardo).
> > - Patch 3: as asked, I checked whether the camera's other
> > AUTO_UPDATE-flagged controls suffer the same bug. Two more do:
> > CT_PANTILT_RELATIVE and CT_ZOOM_ABSOLUTE, both verified on hardware
> > to change autonomously and report live values on GET_CUR; entries
> > added for both. Exposure, white balance and focus turned out to be
> > write-only on this firmware (their autos work, but GET_CUR echoes the
> > last SET_CUR), so they gain nothing from AUTO_UPDATE and were left
> > alone; details in the commit message. The commit message also no
> > longer claims the capability byte is the same for every control --
> > probing every control showed it is computed per control, just wrong
> > for the PTZ ones. Dropped Ricardo's Reviewed-by since the patch
> > changed materially.
> >
> > The full lsusb -v output was posted in reply to v1's patch 3, in the
> > thread at [1].
> >
> > [1] https://lore.kernel.org/linux-media/20260828152557.653475-1-jordan.mymail@gmail.com/
>
> Michael, thank you for the patch. Patches 1/2 look good to me:
>
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>
> Ricardo (and Michael, I wonder, in the light of Michael already having
> found a second camera with the same issue and also in the light of your
> "media: uvcvideo: Automatically handle invalid uvc_versions" series
> if it would not be better to try to fix this up automatically instead
> of relying on device quirks?
>
> Specifically the UVC_CTRL_FLAG_AUTO_UPDATE flag is already there
> in the default flags for these controls (and a bunch of others)
> in uvc_ctrls[].
The UVC_CTRL_FLAG_AUTO_UPDATE flag in uvc_ctrls indicates which control
*may* be auto-update controls. In many devices, lots of those will not
be auto-update controls. For instance, UVC_PU_HUE_CONTROL may only
auto-update when auto-hue is enabled. Devices that implement manual hue
only will not update this control.
We use the flags in uvc_ctrls as a fallback if getting control info from
the device fails (in uvc_ctrl_get_flags()). I think that's the right
thing to do.
> I wonder if we should simply always honor UVC_CTRL_FLAG_AUTO_UPDATE
> from uvc_ctrls[] even when we do get a valid GET_INFO request and
> simply or in the UVC_CTRL_FLAG_AUTO_UPDATE from uvc_ctrls[] if it
> is there?
>
> That avoids needing to adding more and more quirks for this.
>
> Regards,
>
> Hans
>
> > Michael Jordan (3):
> > media: uvcvideo: report AUTO_UPDATE controls as volatile
> > media: uvcvideo: generalise the XU flags fixup to all controls
> > media: uvcvideo: fix up missing AUTO_UPDATE on the OBSBOT Tiny 2
> >
> > drivers/media/usb/uvc/uvc_ctrl.c | 122 ++++++++++++++++++++-----------
> > 1 file changed, 81 insertions(+), 41 deletions(-)
--
Regards,
Laurent Pinchart
prev parent reply other threads:[~2026-09-28 11:49 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-02 0:25 Michael Jordan
2026-09-02 0:25 ` [PATCH v2 1/3] media: uvcvideo: report AUTO_UPDATE controls as volatile Michael Jordan
2026-09-02 0:25 ` [PATCH v2 2/3] media: uvcvideo: generalise the XU flags fixup to all controls Michael Jordan
2026-09-02 6:38 ` Ricardo Ribalda
2026-09-28 11:44 ` Laurent Pinchart
2026-09-28 14:34 ` Michael Jordan
2026-09-02 0:25 ` [PATCH v2 3/3] media: uvcvideo: fix up missing AUTO_UPDATE on the OBSBOT Tiny 2 Michael Jordan
2026-09-02 6:34 ` Ricardo Ribalda
2026-09-27 22:06 ` Michael Jordan
2026-09-28 6:57 ` Ricardo Ribalda
2026-09-28 11:18 ` Ricardo Ribalda
2026-09-28 14:34 ` Michael Jordan
2026-09-28 11:57 ` Laurent Pinchart
2026-09-28 14:34 ` Michael Jordan
2026-09-28 14:55 ` Hans de Goede
2026-09-28 18:53 ` Laurent Pinchart
2026-09-28 11:01 ` [PATCH v2 0/3] media: uvcvideo: live pan/tilt/zoom readback " Hans de Goede
2026-09-28 11:12 ` Ricardo Ribalda
2026-09-28 11:14 ` Hans de Goede
2026-09-28 11:49 ` Laurent Pinchart [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260928114900.GC157191@killaraus.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=hansg@kernel.org \
--cc=hverkuil+cisco@kernel.org \
--cc=jordan.mymail@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=ribalda@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®