Re: Re: [PATCH] media: uvcvideo: query pan/tilt position from the device on every read
Ricardo Ribalda <[email protected]>
| Newsgroups | org.kernel.vger.linux-media,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CANiDSCuNhC0-rDiBChZ02Q-PKtc=_a1ptukNUtMNMRWLUa2HeQ@mail.gmail.com> |
Hi Michael On Sat, 15 Aug 2026 at 01:11, Michael Jordan <[email protected]> wrote: > > Hi Ricardo, > > On Fri, 31 Jul 2026 19:14:51 +0200, Ricardo Ribalda <[email protected]> wrote: > > You definitely have to ping the vendor to make sure they fix their firmware. > > Done -- I wrote to OBSBOT on 4 August covering the GET_INFO stub along > with two other compliance defects I found on the device (a control > default outside the control's own min/max, and PanTilt-Relative > returning a constant). No response so far. I will report back if that > changes. > > One thing I should have made explicit earlier, since it bears on > whether this patch is the right general-case fix rather than a quirk > for one broken camera: correcting that firmware would not make the > position observable. > > The driver clears the control cache in exactly two places -- > uvc_ctrl_status_event() on a Control Change interrupt, and > uvc_ctrl_commit_entity() after a SET_CUR commit for auto-update > controls -- and __uvc_ctrl_load_cur() re-arms it after each read. So on > a fully compliant device a host-commanded move yields two samples, both > endpoints: the first G_CTRL after the commit reaches the device, but it > runs microseconds after the command, when the actuator has not yet > moved, and every later poll returns that value frozen until the > end-of-movement interrupt. Under autonomous tracking there is no commit > and no defined interrupt, so nothing invalidates the cache at all. 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 So all the values captured with G_EXT_CTRL() will show fresh data. > > So the limit is the caching model rather than the device. That is the > case I would want the patch judged on, and it applies to correct > firmware too. > > Worth flagging one side effect if OBSBOT does fix GET_INFO: with > AUTO_UPDATE reported, uvc_ctrl_commit_entity() would begin clearing > ctrl->loaded on this camera, so single-axis writes issued tens of > milliseconds apart would start merging against live data and cancelling > each other -- the second failure mode I measured. It would move the > device from one horn of the read-modify-write question to the other, > which is the pre-existing problem you identified and which this patch > deliberately does not touch. > > No action needed from my side while we wait for Laurent, HansG or > HansV. I am still happy to respin with the autonomous-tracking case > argued in the commit message rather than in this thread, if that would > help whoever picks it up. 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 and then modify your userspace code, so you allways do S_EXT_CTRLS() with the information of both axis. I believe that should cover your usecase. > > Best regards, > Michael Jordan > > On Sat, Aug 01, 2026 01:00 AM, Michael Jordan <[email protected]> wrote: >> >> Hi Ricardo, >> >> On Fri, 31 Jul 2026 19:14:51 +0200, Ricardo Ribalda <[email protected]> wrote: >> > How accurate does the mapping frame/position need to be? >> > >> > You do not know how big the camera pipeline is. Maybe you are >> > processing frame NOW-4 when you read position NOW >> >> Not accurate at all, and I should have been clearer earlier: I picked >> "the former" for the continuous part of your question, not for the >> frame-tagging example. Nothing in my use case correlates a position >> with a particular frame. The position is an input to a control loop >> ("where am I, so I can ask for ten degrees left of here") and a number >> on a UI, and the gimbal's own time constants are hundreds of >> milliseconds per move, so a few frames of pipeline skew is far below >> anything that matters. >> >> If someone did want frame-accurate pose, I agree a G_CTRL read would be >> the wrong instrument entirely -- that is a per-frame metadata problem, >> and this patch would not be the right vehicle for it. >> >> > unrelated note: if you need to set the two axis at the set time, you >> > can use V4L.VIDIOC_S_EXT_CTRLS >> > [...] >> > In fact you could use that today to avoid part of the kernel issue. >> > Always send pan and tilt when you are tracking the movement. >> >> Thanks -- and you are right that this sidesteps the merge problem >> completely, which I had not appreciated. Both mappings land in >> UVC_CTRL_DATA_CURRENT before the single commit, so whatever >> __uvc_ctrl_load_cur() put there is fully overwritten and it no longer >> matters which source the RMW pulled from. I will switch my userspace to >> always send both axes. It does not touch the read side, but it makes >> the write side a non-issue for me in the meantime. >> >> > If AUTO_UPDATE is present you would get fresh data when you poll the >> > control. >> >> One clarification for the record, since it bears on what you are asking >> HansV to weigh: that holds via the interrupt rather than via the poll. >> uvc_ctrl_status_event() clears ctrl->loaded, so a poll after a >> notification does re-read the device -- but polls between notifications >> still return the cache, because __uvc_ctrl_load_cur() returns early on >> ctrl->loaded and nothing else clears it outside a commit. So on a >> compliant autoupdate device the update rate is the device's signalling >> rate, and for pan/tilt 4.2.2.1.15 only promises that signal at the end >> of the movement. Good enough for "it stopped, here is where"; not for >> sampling a trajectory. I mention it only so the comparison is accurate, >> not to relitigate -- your framing of the two problems matches how I see >> them. >> >> > You definitely have to ping the vendor to make sure they fix their >> > firmware. >> >> Agreed, and I will. I am not counting on a timeline. >> >> > Let's wait a bit for Laurent or HansG (or even Hans Verkuil) to >> > comment. >> >> Sounds good -- no rush at all from my side, and I am happy to respin >> against whatever shape you all land on. Enjoy the time off. >> >> Best regards, >> Michael Jordan -- Ricardo Ribalda