[PATCH v3 3/4] libcamera: pipeline: uvcvideo: Retrieve current exposure mode on start

[Jacopo Mondi]

Hi Barnabás

On Fri, Mar 14, 2025 at 06:42:47PM +0100, Barnabás Pőcze wrote:
> If `ExposureTimeMode` is supported, then retrieve the current value
> of `V4L2_CID_EXPOSURE_AUTO` in `PipelineHandlerUVC::start()`, and
> convert it to the appropriate `ExposureTimeModeEnum` value. If it
> is not supported or the conversion fails, then fall back to assuming
> that `ExposureTimeModeManual` is in effect.
>
> This is necessary to be able to properly handle the `ExposureTime`
> control as its value is required to be ignored if `ExposureTimeMode`
> is not manual.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 4ff79c291..7d882ebe1 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -62,6 +62,15 @@ public:
>  	std::optional<v4l2_exposure_auto_type> autoExposureMode_;
>  	std::optional<v4l2_exposure_auto_type> manualExposureMode_;
>
> +	struct State {
> +		std::optional<controls::ExposureTimeModeEnum> exp;
> +
> +		void reset()
> +		{
> +			exp.reset();
> +		}
> +	} state_;

Do you expect this to grow ? Otherwise there's no much difference than
just using an optional<> directly.

> +
>  private:
>  	bool generateId();
>
> @@ -303,6 +312,16 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList
>  		return ret;
>  	}
>
> +	if (data->autoExposureMode_ || data->manualExposureMode_) {
> +		const auto &ctrls = data->video_->getControls({ V4L2_CID_EXPOSURE_AUTO });
> +		const auto &value = ctrls.get(V4L2_CID_EXPOSURE_AUTO);

Is this safe in case there's no V4L2_CID_EXPOSURE_AUTO ? Isn't it
safer to check

                if (!ctrls.empty()) {
                        const auto &value = ctrls.get(V4L2_CID_EXPOSURE_AUTO);
			data->state_.exp = v4l2ToExposureMode(value.get<int32_t>());

                }
> +		if (!value.isNone())
> +			data->state_.exp = v4l2ToExposureMode(value.get<int32_t>());
> +	}
> +
> +	if (!data->state_.exp)
> +		data->state_.exp = controls::ExposureTimeModeManual;
> +

I presume this is safer than just relying on the control default
value, specifically in start-stop-start sessions (if controls are not
reset in between ofc)

>  	return 0;
>  }
>
> @@ -311,6 +330,7 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)
>  	UVCCameraData *data = cameraData(camera);
>  	data->video_->streamOff();
>  	data->video_->releaseBuffers();
> +	data->state_.reset();
>  }
>
>  int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,
> --
> 2.48.1
>