| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230607071027.GJ14101@pendragon.ideasonboard.com>
Date: Wed, 7 Jun 2023 10:10:27 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Michael Riesch <michael.riesch@...fvision.net>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
libcamera-devel@...ts.libcamera.org,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Hans Verkuil <hverkuil-cisco@...all.nl>,
Matthias Fend <Matthias.Fend@...fvision.net>,
Dave Stevenson <dave.stevenson@...pberrypi.com>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v2 5/6] media: v4l2-ctrls: add lens calibration
controls
Hi Michael,
Thank you for the patch.
On Tue, Apr 25, 2023 at 11:45:15AM +0200, Michael Riesch wrote:
> Add the controls V4L2_CID_LENS_CALIB_CONTROL and V4L2_CID_LENS_CALIB_STATUS
> that facilitate the control of the lens group calibration procedure.
>
> Signed-off-by: Michael Riesch <michael.riesch@...fvision.net>
> ---
> .../userspace-api/media/v4l/ext-ctrls-camera.rst | 35 ++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 +++
> include/uapi/linux/v4l2-controls.h | 12 ++++++++
> 3 files changed, 51 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> index a17620ab03b9..8b54a0f3a617 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> @@ -297,6 +297,41 @@ enum v4l2_auto_focus_range -
> (V4L2_CID_ZOOM_ABSOLUTE and V4L2_CID_ZOOM_RELATIVE). The unit is
> driver-specific. The value should be a positive integer.
>
> +``V4L2_CID_LENS_CALIB_CONTROL (bitmask)``
> + Control the calibration procedure (or individual parts thereof) of the lens
> + groups. For example, this could include the mechanical range detection
> + of zoom lens motors. This is a write-only control.
> +
> +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}|
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_LENS_CALIB_STOP``
> + - Stop the lens calibration procedure.
> + * - ``V4L2_LENS_CALIB_START``
> + - Start the complete lens calibration procedure.
> +
> +``V4L2_CID_LENS_CALIB_STATUS (bitmask)``
> + The status of the calibration procedure (or individual parts thereof) of
> + the lens groups. This is a read-only control.
> +
> +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}|
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_LENS_CALIB_IDLE``
> + - Lens calibration procedure has not yet been started.
> + * - ``V4L2_LENS_CALIB_BUSY``
> + - Lens calibration procedure is in progress.
> + * - ``V4L2_LENS_CALIB_COMPLETE``
> + - Lens calibration procedure is complete.
> + * - ``V4L2_LENS_CALIB_FAILED``
> + - Lens calibration procedure has failed.
> +
This sounds *very* ad-hoc for the device you're working on. Does it
really make sense as a standardized V4L2 control ? A device-specific
control could be better.
If you want to standardize this, I'd like to see multiple use cases, as
well as a detailed documentation of the calibration process.
> ``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 3ef465ba73bd..faddfecba6d9 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1050,6 +1050,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_ZOOM_STATUS: return "Zoom, Status";
> case V4L2_CID_FOCUS_SPEED: return "Focus, Speed";
> case V4L2_CID_ZOOM_SPEED: return "Zoom, Speed";
> + case V4L2_CID_LENS_CALIB_CONTROL: return "Lens Calibration, Control";
> + case V4L2_CID_LENS_CALIB_STATUS: return "Lens Calibration, Status";
>
> /* FM Radio Modulator controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1596,6 +1598,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_FOCUS_RELATIVE:
> case V4L2_CID_IRIS_RELATIVE:
> case V4L2_CID_ZOOM_RELATIVE:
> + case V4L2_CID_LENS_CALIB_CONTROL:
> *flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
> V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> break;
> @@ -1603,6 +1606,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_FOCUS_STATUS:
> case V4L2_CID_ZOOM_CURRENT:
> case V4L2_CID_ZOOM_STATUS:
> + case V4L2_CID_LENS_CALIB_STATUS:
> *flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE;
> break;
> case V4L2_CID_FLASH_STROBE_STATUS:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 8d84508d4db8..24c0eb5f4d29 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1004,6 +1004,18 @@ enum v4l2_auto_focus_range {
> #define V4L2_CID_FOCUS_SPEED (V4L2_CID_CAMERA_CLASS_BASE+41)
> #define V4L2_CID_ZOOM_SPEED (V4L2_CID_CAMERA_CLASS_BASE+42)
>
> +#define V4L2_LENS_CALIB_STOP (0 << 0)
> +#define V4L2_LENS_CALIB_START (1 << 0)
> +
> +#define V4L2_CID_LENS_CALIB_CONTROL (V4L2_CID_CAMERA_CLASS_BASE+43)
> +
> +#define V4L2_LENS_CALIB_IDLE (0 << 0)
> +#define V4L2_LENS_CALIB_BUSY (1 << 0)
> +#define V4L2_LENS_CALIB_COMPLETE (1 << 1)
> +#define V4L2_LENS_CALIB_FAILED (1 << 2)
> +
> +#define V4L2_CID_LENS_CALIB_STATUS (V4L2_CID_CAMERA_CLASS_BASE+44)
> +
> /* FM Modulator class control IDs */
>
> #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
>
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists