Hi Laurent, On Wed, Jun 07, 2023 at 09:55:20AM +0300, Laurent Pinchart wrote: > On Wed, Jun 07, 2023 at 09:42:07AM +0300, Laurent Pinchart wrote: > > On Tue, Jun 06, 2023 at 03:15:33PM +0200, Michael Riesch wrote: > > > On 6/6/23 12:36, Laurent Pinchart wrote: > > > > On Tue, Apr 25, 2023 at 11:45:12AM +0200, Michael Riesch wrote: > > > >> The control V4L2_CID_FOCUS_RELATIVE only makes sense if the device cannot > > > >> handle absolute focal point positioning with V4L2_CID_FOCUS_ABSOLUTE. > > > >> Clarify this in the documentation. > > > >> > > > >> Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> > > > >> --- > > > >> Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst | 4 +++- > > > >> 1 file changed, 3 insertions(+), 1 deletion(-) > > > >> > > > >> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > > > >> index df29150dce7b..42cf4c3cda0c 100644 > > > >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > > > >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > > > >> @@ -147,7 +147,9 @@ enum v4l2_exposure_metering - > > > >> This control moves the focal point of the camera by the specified > > > >> amount. The unit is undefined. Positive values move the focus closer > > > >> to the camera, negative values towards infinity. This is a > > > >> - write-only control. > > > >> + write-only control. It should be implemented only if the device cannot > > > >> + handle absolute values. > > > >> + > > > > > > > > Extra blank line. > > > > > > Will fix that. > > > > > > > I don't think this is right. The control was added for the UVC driver, > > > > and there are devices that implement both absolute and relative focus. > > > > > > I am by no means an expert here and just following Sakari's suggestion > > > here (see [0]). I can drop the patch, leave it as-is, or modify it. > > > Whatever makes sense to you guys. But maybe I should leave this to > > > someone more knowledgeable in this area and drop the patch from my > > > series. The changes above are completely orthogonal to my work. > > > > V4L2_CID_FOCUS_RELATIVE is an annoying control. It was introduced for > > UVC, and to my surprise, it turns out it has never been implemented in > > the uvcvideo driver. The 3 devices I know of that implement the UVC > > relative focus control also implement the UVC absolute focus control. > > > > I'm tempted to deprecate this control completely. Sakari, any opinion ? > > This is how the UVC relative focus control is defined. > > 4.2.2.1.7 Focus (Relative) Control > > The Focus (Relative) Control is used to move the focus lens group to > specify the distance to the optimally focused target. > > The bFocusRelative field indicates whether the focus lens group is > stopped or is moving for near or for infinity direction. A value of 1 > indicates that the focus lens group is moved for near direction. A > value of 0 indicates that the focus lens group is stopped. And a value > of 0xFF indicates that the lens group is moved for infinity direction. > The GET_MIN, GET_MAX, GET_RES and GET_DEF requests will return zero > for this field. > > The bSpeed field indicates the speed of the lens group movement. A low > number indicates a slow speed and a high number indicates a high > speed. The GET_MIN, GET_MAX and GET_RES requests are used to retrieve > the range and resolution for this field. The GET_DEF request is used > to retrieve the default value for this field. If the control does not > support speed control, it will return the value 1 in this field for > all these requests. > > If both Relative and Absolute Controls are supported, a SET_CUR to the > Relative Control with a value other than 0x00 shall result in a > Control Change interrupt for the Absolute Control at the end of the > movement (see section 2.4.2.2, “Status Interrupt Endpoint”). The end > of movement can be due to physical device limits, or due to an > explicit request by the host to stop the movement. If the end of > movement is due to physical device limits (such as a limit in range of > motion), a Control Change interrupt shall be generated for this > Relative Control. If there is no limit in range of motion, a Control > Change interrupt is not required. > > It seems there's no way we can just map this to V4L2_CID_FOCUS_RELATIVE, > making the V4L2 relative focus control quite useless. I'd deprecate this control as it doesn't have any use. The control documentation + other references in V4L2 control framework could be even removed but the control ID definition needs to stay as user space may refer to it. -- Regards, Sakari Ailus
