Hi Laurent, On 6/7/23 09:07, Laurent Pinchart wrote: > Hi Michael, > > Thank you for the patch. > > On Tue, Apr 25, 2023 at 11:45:14AM +0200, Michael Riesch wrote: >> Add the controls V4L2_CID_FOCUS_SPEED and V4L2_CID_ZOOM_SPEED that set >> the speed of the zoom lens group and focus lens group, respectively. > > Ah, now the UVC relative focus and zoom controls could be implemented > ;-) They still don't match the definition of the corresponding V4L2 > controls, so I'm still tempted to deprecate both. There is also V4L2_CID_ZOOM_CONTINUOUS that could be mapped to the UVC relative zoom, if I am not mistaken. I had a similar V4L2_CID_FOCUS_CONTINUOUS control in mind briefly, but for our use case this would not be general enough. >> Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> >> --- >> Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst | 10 ++++++++++ >> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++ >> include/uapi/linux/v4l2-controls.h | 2 ++ >> 3 files changed, 14 insertions(+) >> >> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst >> index 3ea4175f9619..a17620ab03b9 100644 >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst >> @@ -174,6 +174,11 @@ enum v4l2_exposure_metering - >> will not transition from this state until another action is performed >> by an application. >> >> +``V4L2_CID_FOCUS_SPEED (integer)`` >> + Set the speed with which the focus lens group of the camera is moved >> + (V4L2_CID_FOCUS_ABSOLUTE and V4L2_CID_FOCUS_RELATIVE). The unit is >> + driver-specific. The value should be a positive integer. >> + > > Could you explain your expected use cases for focus and zoom speed ? As I outlined in the cover letter, the speed of the lenses can be adjusted. This can be used e.g., in the autofocus algorithm, where a coarse scan and a fine grained scan can be implemented using different speeds. The adjustable zoom speed control is simply there to give the user the choice to zoom in/out fast or slow. Should this go into the documentation? Best regards, Michael > >> ``V4L2_CID_FOCUS_AUTO (boolean)`` >> Enables continuous automatic focus adjustments. The effect of manual >> focus adjustments while this feature is enabled is undefined, >> @@ -287,6 +292,11 @@ enum v4l2_auto_focus_range - >> not transition from this state until another action is performed by an >> application. >> >> +``V4L2_CID_ZOOM_SPEED (integer)`` >> + Set the speed with which the zoom lens group of the camera is moved >> + (V4L2_CID_ZOOM_ABSOLUTE and V4L2_CID_ZOOM_RELATIVE). The unit is >> + driver-specific. The value should be a positive integer. >> + >> ``V4L2_CID_IRIS_ABSOLUTE (integer)`` >> This control sets the camera's aperture to the specified value. The >> unit is undefined. Larger values open the iris wider, smaller values >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> index 794ef3ab0c02..3ef465ba73bd 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c >> @@ -1048,6 +1048,8 @@ const char *v4l2_ctrl_get_name(u32 id) >> case V4L2_CID_FOCUS_STATUS: return "Focus, Status"; >> case V4L2_CID_ZOOM_CURRENT: return "Zoom, Current"; >> case V4L2_CID_ZOOM_STATUS: return "Zoom, Status"; >> + case V4L2_CID_FOCUS_SPEED: return "Focus, Speed"; >> + case V4L2_CID_ZOOM_SPEED: return "Zoom, Speed"; >> >> /* FM Radio Modulator controls */ >> /* Keep the order of the 'case's the same as in v4l2-controls.h! */ >> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h >> index 793ee8c65e87..8d84508d4db8 100644 >> --- a/include/uapi/linux/v4l2-controls.h >> +++ b/include/uapi/linux/v4l2-controls.h >> @@ -1001,6 +1001,8 @@ enum v4l2_auto_focus_range { >> #define V4L2_CID_FOCUS_STATUS (V4L2_CID_CAMERA_CLASS_BASE+38) >> #define V4L2_CID_ZOOM_CURRENT (V4L2_CID_CAMERA_CLASS_BASE+39) >> #define V4L2_CID_ZOOM_STATUS (V4L2_CID_CAMERA_CLASS_BASE+40) >> +#define V4L2_CID_FOCUS_SPEED (V4L2_CID_CAMERA_CLASS_BASE+41) >> +#define V4L2_CID_ZOOM_SPEED (V4L2_CID_CAMERA_CLASS_BASE+42) >> >> /* FM Modulator class control IDs */ >> >