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

[Ricardo Ribalda]

Hi Michael

On Fri, 21 Aug 2026 at 11:03, Michael Jordan <jordan.mymail@xxxxxxxxx> wrote:
>
> Hi Ricardo,
>
> On Tue, 18 Aug 2026 13:30:57 +0200, Ricardo Ribalda <ribalda@xxxxxxxxxxxx> 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.


Happy to hear that it works for you! A proper patchset would be
awesome. I do not think we have to wait for Hans(es) to start
reviewing it.

How does this sound?:

1) A patch to add V4L2_CTRL_FLAG_VOLATILE for all the
UVC_CTRL_FLAG_AUTO_UPDATE controls ( I think that is missing)
2) A patch to rename uvc_ctrl_fixup_xu_info to uvc_ctrl_fixup_flags
and modify uvc_ctrl_get_flags so it calls it.
3) A patch to add the quirk for your device. In the commit message,
please include the lsusb -v output of your device and mention that you
pinged the vendor to fix their firmware.

The Suggested-by tag would be great :)

Regards!


>
> Best regards,
> Michael Jordan



-- 
Ricardo Ribalda