[PATCH v9 03/12] libcamera: uvcvideo: Register ExposureTimeMode control

[Barnabás Pőcze]

2025. január 20., hétfő 21:44 keltezéssel, Laurent Pinchart <laurent.pinchart at ideasonboard.com> írta:

> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> Port the UVC pipeline handler to use the new ExposureTimeMode control
> when processing Camera controls in place of the AeEnable control.
> 
> The V4L2_CID_EXPOSURE_AUTO control allows 4 possible values, which
> map to ExposureTimeModeAuto and ExposureTimeModeManual.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 53 ++++++++++++++++++--
>  1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 8c2c6baf3575..7470b56270f6 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -298,7 +298,7 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  		cid = V4L2_CID_CONTRAST;
>  	else if (id == controls::Saturation)
>  		cid = V4L2_CID_SATURATION;
> -	else if (id == controls::AeEnable)
> +	else if (id == controls::ExposureTimeMode)
>  		cid = V4L2_CID_EXPOSURE_AUTO;
>  	else if (id == controls::ExposureTime)
>  		cid = V4L2_CID_EXPOSURE_ABSOLUTE;
> @@ -647,7 +647,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  		id = &controls::Saturation;
>  		break;
>  	case V4L2_CID_EXPOSURE_AUTO:
> -		id = &controls::AeEnable;
> +		id = &controls::ExposureTimeMode;
>  		break;
>  	case V4L2_CID_EXPOSURE_ABSOLUTE:
>  		id = &controls::ExposureTime;
> @@ -660,6 +660,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  	}
> 
>  	/* Map the control info. */
> +	const std::vector<ControlValue> &v4l2Values = v4l2Info.values();
>  	int32_t min = v4l2Info.min().get<int32_t>();
>  	int32_t max = v4l2Info.max().get<int32_t>();
>  	int32_t def = v4l2Info.def().get<int32_t>();
> @@ -697,10 +698,52 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  		};
>  		break;
> 
> -	case V4L2_CID_EXPOSURE_AUTO:
> -		info = ControlInfo{ false, true, true };
> -		break;
> +	case V4L2_CID_EXPOSURE_AUTO: {
> +		/*
> +		 * From the V4L2_CID_EXPOSURE_AUTO documentation:
> +		 *
> +		 * ------------------------------------------------------------
> +		 * V4L2_EXPOSURE_AUTO:
> +		 * Automatic exposure time, automatic iris aperture.
> +		 *
> +		 * V4L2_EXPOSURE_MANUAL:
> +		 * Manual exposure time, manual iris.
> +		 *
> +		 * V4L2_EXPOSURE_SHUTTER_PRIORITY:
> +		 * Manual exposure time, auto iris.
> +		 *
> +		 * V4L2_EXPOSURE_APERTURE_PRIORITY:
> +		 * Auto exposure time, manual iris.
> +		 *-------------------------------------------------------------
> +		 *
> +		 * ExposureTimeModeAuto = { V4L2_EXPOSURE_AUTO,
> +		 * 			    V4L2_EXPOSURE_APERTURE_PRIORITY }
> +		 *
> +		 *
> +		 * ExposureTimeModeManual = { V4L2_EXPOSURE_MANUAL,
> +		 *			      V4L2_EXPOSURE_SHUTTER_PRIORITY }
> +		 */
> +		std::array<int32_t, 2> values{};
> 
> +		auto it = std::find_if(v4l2Values.begin(), v4l2Values.end(),
> +			[&](const ControlValue &val) {
> +				return (val.get<int32_t>() == V4L2_EXPOSURE_APERTURE_PRIORITY ||
> +					val.get<int32_t>() == V4L2_EXPOSURE_AUTO) ? true : false;
> +			});
> +		if (it != v4l2Values.end())
> +			values.back() = static_cast<int32_t>(controls::ExposureTimeModeAuto);
> +
> +		it = std::find_if(v4l2Values.begin(), v4l2Values.end(),
> +			[&](const ControlValue &val) {
> +				return (val.get<int32_t>() == V4L2_EXPOSURE_SHUTTER_PRIORITY ||
> +					val.get<int32_t>() == V4L2_EXPOSURE_MANUAL) ? true : false;
> +			});
> +		if (it != v4l2Values.end())
> +			values.back() = static_cast<int32_t>(controls::ExposureTimeModeManual);
> +
> +		info = ControlInfo{Span<int32_t>{values}, values[0]};

I think this does not call the intended constructor. Shouldn't it be something like this?

@@ -725,7 +724,8 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
                 * ExposureTimeModeManual = { V4L2_EXPOSURE_MANUAL,
                 *                            V4L2_EXPOSURE_SHUTTER_PRIORITY }
                 */
-               std::array<int32_t, 2> values{};
+               std::array<ControlValue, 2> values{};
+               std::size_t i = 0;
 
                auto it = std::find_if(v4l2Values.begin(), v4l2Values.end(),
                        [&](const ControlValue &val) {
@@ -733,7 +733,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
                                        val.get<int32_t>() == V4L2_EXPOSURE_AUTO) ? true : false;
                        });
                if (it != v4l2Values.end())
-                       values.back() = static_cast<int32_t>(controls::ExposureTimeModeAuto);
+                       values[i++] = static_cast<int32_t>(controls::ExposureTimeModeAuto);
 
                it = std::find_if(v4l2Values.begin(), v4l2Values.end(),
                        [&](const ControlValue &val) {
@@ -741,9 +741,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
                                        val.get<int32_t>() == V4L2_EXPOSURE_MANUAL) ? true : false;
                        });
                if (it != v4l2Values.end())
-                       values.back() = static_cast<int32_t>(controls::ExposureTimeModeManual);
+                       values[i++] = static_cast<int32_t>(controls::ExposureTimeModeManual);
 
-               info = ControlInfo{Span<int32_t>{values}, values[0]};
+               info = ControlInfo{Span<const ControlValue>{values.data(), i}, values[0]};
                break;
        }
        case V4L2_CID_EXPOSURE_ABSOLUTE:

I think `PipelineHandlerUVC::processControl()` is also problematic:

	case V4L2_CID_EXPOSURE_AUTO: {
		int32_t ivalue = value.get<bool>()
			       ? V4L2_EXPOSURE_APERTURE_PRIORITY
			       : V4L2_EXPOSURE_MANUAL;
		controls->set(V4L2_CID_EXPOSURE_AUTO, ivalue);
		break;
	}

The control's type is no longer boolean, no?


Regards,
Barnabás Pőcze

> +		break;
> +	}
>  	case V4L2_CID_EXPOSURE_ABSOLUTE:
>  		/*
>  		 * ExposureTime is in units of 1 µs, and UVC expects
> --
> Regards,
> 
> Laurent Pinchart
> 
>