[libcamera-devel,v10,06/13] media: v4l2-ctrls: Add camera orientation and rotation

Message ID 20200508100158.3437161-7-jacopo@jmondi.org
State Superseded
Headers show
Series
  • media: report camera properties
Related show

Commit Message

Jacopo Mondi May 8, 2020, 10:01 a.m. UTC
Add support for the newly defined V4L2_CID_CAMERA_ORIENTATION
and V4L2_CID_CAMERA_SENSOR_ROTATION read-only controls used to report
the camera device mounting position and orientation respectively.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
 include/uapi/linux/v4l2-controls.h   |  7 +++++++
 2 files changed, 22 insertions(+)

Comments

Hans Verkuil May 8, 2020, 11:09 a.m. UTC | #1
On 08/05/2020 12:01, Jacopo Mondi wrote:
> Add support for the newly defined V4L2_CID_CAMERA_ORIENTATION
> and V4L2_CID_CAMERA_SENSOR_ROTATION read-only controls used to report
> the camera device mounting position and orientation respectively.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
>  include/uapi/linux/v4l2-controls.h   |  7 +++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 1c617b42a944d..97765a57654d2 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -583,6 +583,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"Annex B Start Code",
>  		NULL,
>  	};
> +	static const char * const camera_orientation[] = {
> +		"Front Camera",
> +		"Back Camera",
> +		"External Camera",

You can drop 'Camera' here. The name of the control itself already specifies that
it is about the camera orientation, so that does (and should) not be repeated here.

> +		NULL,
> +	};
>  
>  	switch (id) {
>  	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -708,6 +714,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		return hevc_decode_mode;
>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
>  		return hevc_start_code;
> +	case V4L2_CID_CAMERA_ORIENTATION:
> +		return camera_orientation;
>  	default:
>  		return NULL;
>  	}
> @@ -1020,6 +1028,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_PAN_SPEED:		return "Pan, Speed";
>  	case V4L2_CID_TILT_SPEED:		return "Tilt, Speed";
>  	case V4L2_CID_UNIT_CELL_SIZE:		return "Unit Cell Size";
> +	case V4L2_CID_CAMERA_ORIENTATION:	return "Camera Orientation";
> +	case V4L2_CID_CAMERA_SENSOR_ROTATION:	return "Camera Sensor Rotation";
>  
>  	/* FM Radio Modulator controls */
>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1295,6 +1305,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
> +	case V4L2_CID_CAMERA_ORIENTATION:

Just move this up to just below 'case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE'
and add 'case V4L2_CID_CAMERA_ORIENTATION:' to the switch at the end of
this function that sets the READ_ONLY flag.

I.e. this function has two switches: one that sets the type and one at the
end that sets the flags. I see that the code is not entirely consistent anymore,
but we should try to keep to how it is supposed to be used.

> +		*type = V4L2_CTRL_TYPE_MENU;
> +		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +		break;
>  	case V4L2_CID_LINK_FREQ:
>  		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
>  		break;
> @@ -1346,6 +1360,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		break;
>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
>  	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
> +	case V4L2_CID_CAMERA_SENSOR_ROTATION:
>  		*type = V4L2_CTRL_TYPE_INTEGER;
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;

Same here.

>  		break;
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 0ba1005c96519..3da37c9cf5046 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -923,6 +923,13 @@ enum v4l2_auto_focus_range {
>  #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
>  #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
>  
> +#define V4L2_CID_CAMERA_ORIENTATION		(V4L2_CID_CAMERA_CLASS_BASE+34)
> +#define V4L2_ORIENTATION_FRONT			0
> +#define V4L2_ORIENTATION_BACK			1
> +#define V4L2_ORIENTATION_EXTERNAL		2
> +
> +#define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
> +
>  /* FM Modulator class control IDs */
>  
>  #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
> 

Regards,

	Hans

Patch

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 1c617b42a944d..97765a57654d2 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -583,6 +583,12 @@  const char * const *v4l2_ctrl_get_menu(u32 id)
 		"Annex B Start Code",
 		NULL,
 	};
+	static const char * const camera_orientation[] = {
+		"Front Camera",
+		"Back Camera",
+		"External Camera",
+		NULL,
+	};
 
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -708,6 +714,8 @@  const char * const *v4l2_ctrl_get_menu(u32 id)
 		return hevc_decode_mode;
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
 		return hevc_start_code;
+	case V4L2_CID_CAMERA_ORIENTATION:
+		return camera_orientation;
 	default:
 		return NULL;
 	}
@@ -1020,6 +1028,8 @@  const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_PAN_SPEED:		return "Pan, Speed";
 	case V4L2_CID_TILT_SPEED:		return "Tilt, Speed";
 	case V4L2_CID_UNIT_CELL_SIZE:		return "Unit Cell Size";
+	case V4L2_CID_CAMERA_ORIENTATION:	return "Camera Orientation";
+	case V4L2_CID_CAMERA_SENSOR_ROTATION:	return "Camera Sensor Rotation";
 
 	/* FM Radio Modulator controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1295,6 +1305,10 @@  void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
+	case V4L2_CID_CAMERA_ORIENTATION:
+		*type = V4L2_CTRL_TYPE_MENU;
+		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
+		break;
 	case V4L2_CID_LINK_FREQ:
 		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
 		break;
@@ -1346,6 +1360,7 @@  void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
 	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
+	case V4L2_CID_CAMERA_SENSOR_ROTATION:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 0ba1005c96519..3da37c9cf5046 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -923,6 +923,13 @@  enum v4l2_auto_focus_range {
 #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
 
+#define V4L2_CID_CAMERA_ORIENTATION		(V4L2_CID_CAMERA_CLASS_BASE+34)
+#define V4L2_ORIENTATION_FRONT			0
+#define V4L2_ORIENTATION_BACK			1
+#define V4L2_ORIENTATION_EXTERNAL		2
+
+#define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
+
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)