Re: [PATCH] media: uvcvideo: query pan/tilt position from the device on every read

[Michael Jordan <jordan.mymail@gmail.com>]

Hi Ricardo,

On Tue, 18 Aug 2026 13:30:57 +0200, Ricardo Ribalda <ribalda@chromium.org> wrote:
> I believe that if AUTO_UPDATE would be properly initialized, then, the
> values will always be fresh when you run G_CTRL o G_EXT_CTRLS from
> userspace:
>
> G_EXT_CTRLS
>  uvc_ioctl_g_ext_ctls()
>    uvc_ctrl_rollback()
>      uvc_ctrl_commit()
>         uvc_ctrl_commit_entity()
>             ctrl->loaded=0

You are right, and this corrects what I wrote on 14 August. The rollback
at the end of every G_EXT_CTRLS runs uvc_ctrl_commit_entity(), which
clears ->loaded for an AUTO_UPDATE control at the top of the loop, before
the dirty/rollback checks, so the following read re-queries the device. I
had only counted the Control Change interrupt and the SET_CUR commit as
cache-invalidation points and missed that the read path invalidates its
own cache. So on this device, once AUTO_UPDATE is restored, polling
G_EXT_CTRLS returns fresh data with no volatile flag involved. My claim
that "correcting the firmware would not make the position observable" was
wrong.

> Until they reply maybe you want to:
>   - rename uvc_ctrl_fixup_xu_info to uvc_ctrl_fixup_flags
>   - modify uvc_ctrl_get_flags so it calls uvc_ctrl_fixup_flags
>   - add your device information to uvc_ctrl_fixup_flags

I did this and tested it on the hardware. For the record, the device's
GET_INFO for CT_PANTILT_ABSOLUTE (entity 1, selector 0x0d) returns 0x03 --
GET|SET, no AUTOUPDATE bit -- so uvc_ctrl_get_flags() masks off the
UVC_CTRL_FLAG_AUTO_UPDATE that the static uvc_ctrls[] entry sets for this
control. Calling the fixup from uvc_ctrl_get_flags() unconditionally (so it
also runs when GET_INFO succeeds) and adding

  { USB_DEVICE(0x3564, 0xfef8), 1, UVC_CT_PANTILT_ABSOLUTE_CONTROL,
    UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_RANGE |
    UVC_CTRL_FLAG_RESTORE | UVC_CTRL_FLAG_AUTO_UPDATE }

restores it. One detail worth noting: the existing fixup does
info->flags = fixups[i].flags, a full replace rather than a masked OR, so
a standard-control entry has to spell out the whole flag set, not just the
missing bit.

With that in place I verified on an OBSBOT Tiny 2 (3564:fef8) that polling
G_EXT_CTRLS tracks a physical slew on both axes, including motion the host
never commanded (moving the gimbal by hand), while a stock module returns
the stale value indefinitely for the same reads. So your suggestion covers
my use case, and I am going to carry the quirk rather than the volatile
change.

I have also switched my userspace to always send both axes in one
S_EXT_CTRLS, as you suggested. Both mappings land in UVC_CTRL_DATA_CURRENT
before the single commit, so whatever the read-modify-write loaded is fully
overwritten and the merge source stops mattering -- the write side is a
non-issue now regardless of the flags.

Given the rollback point, the general-case argument for the volatile patch
is weaker than I framed it: for the ioctl polling path a device with
correct (or fixed-up) flags does not need it. The only things it still does
that the quirk does not are to advertise V4L2_CTRL_FLAG_VOLATILE to
userspace and to keep the write-side RMW reading from a separate buffer,
and with always-send-both-axes I need neither. So I will not push the
volatile patch further unless you or Hans think the userspace-visible
VOLATILE flag is worth having on its own.

If it is useful I am happy to send the fixup as a proper patch with a
Suggested-by: you, or to leave it to you if you would rather fold it in.
Thanks for the review and the pointer -- both were right.

Best regards,
Michael Jordan