From patchwork Tue May 6 15:53:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23340 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B5EB5C3226 for ; Tue, 6 May 2025 15:53:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E3CB568B2C; Tue, 6 May 2025 17:53:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ViaUr73o"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E61E968AD5 for ; Tue, 6 May 2025 17:53:18 +0200 (CEST) Received: from pb-laptop.local (185.221.141.56.nat.pool.zt.hu [185.221.141.56]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0A29956D for ; Tue, 6 May 2025 17:53:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1746546788; bh=yIF04YwxBY1jJ7AoqdakHk7Rj858MNZUQ89xMTN4Wfo=; h=From:To:Subject:Date:From; b=ViaUr73org1VBmF4smxbKLPa33qFYMobegC3UtS9+/jpLO79CrrQRlQ7gnx9396SI ozwRoRH7IHlRofFVhzfeeJ9kk8w7SfiujQL8FeGVKwf7wJor4W/H5/6ihoVthJwNhp 42qFdgu2kSF12T+9VGujrQjI1aq8C4Lno6j97xM4= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime` control handling Date: Tue, 6 May 2025 17:53:14 +0200 Message-ID: <20250506155314.1544104-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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 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 video_; Stream stream_; @@ -61,6 +63,15 @@ public: std::optional autoExposureMode_; std::optional manualExposureMode_; + struct State { + std::optional 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 mode; - - switch (value.get()) { - 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(*mode)); + /* Handled directly in `processControls()`. */ break; } case V4L2_CID_EXPOSURE_ABSOLUTE: - controls->set(cid, value.get() / 100); + if (state.exp == controls::ExposureTimeModeManual) + controls->set(cid, value.get() / 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 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(*mode)); + newState.exp = static_cast(*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)