[PATCH v3 2/4] libcamera: pipeline: uvcvideo: Fix `ExposureTimeMode` control setting

[Jacopo Mondi]

Hi Barnabás

On Fri, Mar 14, 2025 at 06:42:46PM +0100, Barnabás Pőcze wrote:
> The mapping in `UVCCameraData::processControl()` is not entirely correct
> because the control value is retrieved as a `bool` instead of `int32_t`.
> Additionally, the available modes are not taken into account.
>
> Retrieve the control value with the right type, `int32_t`, check if the
> requested mode is available, and if so, set the appropriate v4l2 control
> value selected by `addControl()` earlier.
>
> Fixes: bad8d591f8acfa ("libcamera: uvcvideo: Register ExposureTimeMode control")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 29 ++++++++++++++------
>  1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 5c9025d9b..4ff79c291 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -99,8 +99,8 @@ public:
>  	bool match(DeviceEnumerator *enumerator) override;
>
>  private:
> -	int processControl(ControlList *controls, unsigned int id,
> -			   const ControlValue &value);
> +	int processControl(const UVCCameraData *data, ControlList *controls,
> +			   unsigned int id, const ControlValue &value);
>  	int processControls(UVCCameraData *data, Request *request);
>
>  	bool acquireDevice(Camera *camera) override;
> @@ -313,8 +313,8 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)
>  	data->video_->releaseBuffers();
>  }
>
> -int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
> -				       const ControlValue &value)
> +int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,
> +				       unsigned int id, const ControlValue &value)
>  {
>  	uint32_t cid;
>
> @@ -358,10 +358,21 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  	}
>
>  	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);
> +		std::optional<v4l2_exposure_auto_type> mode;
> +
> +		switch (value.get<int32_t>()) {
> +		case controls::ExposureTimeModeAuto:
> +			mode = data->autoExposureMode_;
> +			break;
> +		case controls::ExposureTimeModeManual:
> +			mode = data->manualExposureMode_;
> +			break;
> +		}
> +
> +		if (!mode)
> +			return -EINVAL;

Here as well I don't think mode can be not initialized.

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

> +
> +		controls->set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(*mode));
>  		break;
>  	}
>
> @@ -399,7 +410,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
>  	ControlList controls(data->video_->controls());
>
>  	for (const auto &[id, value] : request->controls())
> -		processControl(&controls, id, value);
> +		processControl(data, &controls, id, value);
>
>  	for (const auto &ctrl : controls)
>  		LOG(UVC, Debug)
> --
> 2.48.1
>