diff mbox series

[RFC,v2,4/6] media: v4l2-ctrls: add lens group speed controls for zoom and focus

Message ID 20230406-feature-controls-lens-v2-4-faa8ad2bc404@wolfvision.net
State New
Headers
Series media: v4l2-ctrls: add controls for complex lens controller devices |

Commit Message

Michael Riesch April 25, 2023, 9:45 a.m. UTC 
  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.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 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(+)

Comments

Laurent Pinchart June 7, 2023, 7:07 a.m. UTC | #1 
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.

> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  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 ?

>  ``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 */
>
Michael Riesch June 13, 2023, 7:49 a.m. UTC | #2 
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@wolfvision.net>
>> ---
>>  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 */
>>  
>
diff mbox series

Patch

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.
+
 ``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 */