[v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime` control handling
diff mbox series

Message ID 20250506155314.1544104-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime` control handling
Related show

Commit Message

Barnabás Pőcze May 6, 2025, 3:53 p.m. UTC
The documentation of `ExposureTime` states that its value is ignored if
`ExposureTimeMode` is not manual. This is currently not handled, and if
`ExposureTimeMode` is automatic and `ExposureTime` is set, then the
controls will be rejected, as expected.

To fix that, keep track of the current value of `ExposureTimeMode`, and
only set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode is
manual.

However, the current state of `V4L2_CID_EXPOSURE_AUTO` is not retrieved
when streaming starts, thus if the user does not set `ExposureTimeMode`
then it will remain in an "unset" state and all attempts to set
`ExposureTime` will be ignored. This is done for two reasons:

  * the current values of controls cannot be retrieved with libcamera,
    therefore this unset state cannot be observed by the user;
  * the user will have to set `ExposureTimeMode` either way if they
    want to guarantee that setting `ExposureTime` will have any effect.

If `ExposureTimeMode` is not exposed by the camera, but `ExposeTime` is
supported, then the exposure mode is assumed to be manual to allow
changing `V4L2_CID_EXPOSURE_ABSOLUTE`.

To be able to handle requests that set both `ExposureTime{,Mode}`, process
`ExposureTimeMode` first directly in `processControls()`, and store this
new mode in a temporary location. Then have `processControl()` act on this
temporary state, and only persist the temporary state if `setControls()`
on the video device succeeds.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=242
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
changes in v4:
  * first two patches of v3 already merged
  * last two patches of v3 combined into this single patch
  * do not retrieve V4L2_CID_EXPOSURE_AUTO when streaming starts

changes in v3:
  * fix default value: set it based on the v4l2 default
  * store current value of `ExposureTimeMode`
  * fix https://bugs.libcamera.org/show_bug.cgi?id=242 as well

changes in v2:
  * split into two patches
  * make it compile with old libstdc++ when _GLIBCXX_DEBUG is set

v3: https://patchwork.libcamera.org/patch/22959/
    https://patchwork.libcamera.org/patch/22958/
v2: https://patchwork.libcamera.org/project/libcamera/list/?series=5006
v1: https://patchwork.libcamera.org/project/libcamera/list/?series=4974
---
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 67 ++++++++++++++------
 1 file changed, 46 insertions(+), 21 deletions(-)

--
2.49.0

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 586e932d2..af0c8dedd 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -53,6 +53,8 @@  public:

 	const std::string &id() const { return id_; }

+	bool supportsExposureTimeMode() const { return autoExposureMode_ || manualExposureMode_; }
+
 	Mutex openLock_;
 	std::unique_ptr<V4L2VideoDevice> video_;
 	Stream stream_;
@@ -61,6 +63,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_ = {};
+
 private:
 	bool generateId();

@@ -98,7 +109,7 @@  public:
 	bool match(DeviceEnumerator *enumerator) override;

 private:
-	int processControl(const UVCCameraData *data, ControlList *controls,
+	int processControl(const UVCCameraData::State &state, ControlList *controls,
 			   unsigned int id, const ControlValue &value);
 	int processControls(UVCCameraData *data, Request *request);

@@ -302,6 +313,9 @@  int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList
 		return ret;
 	}

+	if (!data->supportsExposureTimeMode())
+		data->state_.exp = controls::ExposureTimeModeManual;
+
 	return 0;
 }

@@ -310,9 +324,10 @@  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,
+int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,
 				       unsigned int id, const ControlValue &value)
 {
 	uint32_t cid;
@@ -359,26 +374,13 @@  int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c
 	}

 	case V4L2_CID_EXPOSURE_AUTO: {
-		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;
-
-		controls->set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(*mode));
+		/* Handled directly in `processControls()`. */
 		break;
 	}

 	case V4L2_CID_EXPOSURE_ABSOLUTE:
-		controls->set(cid, value.get<int32_t>() / 100);
+		if (state.exp == controls::ExposureTimeModeManual)
+			controls->set(cid, value.get<int32_t>() / 100);
 		break;

 	case V4L2_CID_CONTRAST:
@@ -413,9 +415,30 @@  int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c
 int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 {
 	ControlList controls(data->video_->controls());
+	const auto &reqControls = request->controls();
+	auto newState = data->state_;

-	for (const auto &[id, value] : request->controls())
-		processControl(data, &controls, id, value);
+	if (const auto exp = reqControls.get(controls::ExposureTimeMode)) {
+		std::optional<v4l2_exposure_auto_type> mode;
+
+		switch (*exp) {
+		case controls::ExposureTimeModeAuto:
+			mode = data->autoExposureMode_;
+			break;
+		case controls::ExposureTimeModeManual:
+			mode = data->manualExposureMode_;
+			break;
+		}
+
+		if (!mode)
+			return -EINVAL;
+
+		controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(*mode));
+		newState.exp = static_cast<controls::ExposureTimeModeEnum>(*exp);
+	}
+
+	for (const auto &[id, value] : reqControls)
+		processControl(newState, &controls, id, value);

 	for (const auto &ctrl : controls)
 		LOG(UVC, Debug)
@@ -428,7 +451,9 @@  int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 		return ret < 0 ? ret : -EINVAL;
 	}

-	return ret;
+	data->state_ = newState;
+
+	return 0;
 }

 int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)