From patchwork Tue Jan 28 12:14:01 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: 22656 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 2FFCCC323E for ; Tue, 28 Jan 2025 12:14:08 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C52FF68557; Tue, 28 Jan 2025 13:14:07 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="x96FKtNm"; dkim-atps=neutral Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CC0AC68557 for ; Tue, 28 Jan 2025 13:14:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1738066446; x=1738325646; bh=oHxofpdrwJrlHjbxC+EwDE/hgtI0/pNMoAYVz5tyKAo=; h=Date:To:From:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=x96FKtNmFI9+NaLdVnMxkELdhC/QPk1SuCzSk9KTnSk5xHS4fKCqcpxWAW7MF47aB w6F1uKVSMxEgl6mjcUHQ4/Dy+EzqJzAArjhtf0umtg9+F7pnW4NU57SuZ+tUNyqB2Z rXOfDJftp/hR75BmcHBbaOa4VHc0pgmymERpZPbnFzGgRDNGD2Fx4ICoBwwySbpkOb zRvTtfTdFdB67FOess71q+joU0BgTMph2P23nhpvxz+qSbwXYLUZH1TVcNsllNJP1M unZ4svHmahyW23aarrxAZysSrQdzT9NDZ/HTEuAWuVOhVkhXfM+ABVCRg9rD8dqX18 viqZBUyDS9yHA== Date: Tue, 28 Jan 2025 12:14:01 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH v1] libcamera: pipeline: uvcvideo: Fix `ExposureTimeMode` control Message-ID: <20250128121352.494582-2-pobrn@protonmail.com> In-Reply-To: <20250128121352.494582-1-pobrn@protonmail.com> References: <20250128121352.494582-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: f5edd462d52793cf638c10418612f4fbf7a3e65c 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" `ControlInfo(Span{...})` calls the incorrect constructor of `ControlInfo`. The intended constructor to be called is `ControlInfo(Span, ...)` however that is not called because a span of `const int32_t` is passed. Instead, the constructor `ControlInfo(const ControlValue &min, const ControlValue &max, ...)` will be called. To fix this, simply pass a span of `ControlValue` instead. Furthermore, the mapping in `UVCCameraData::processControl()` is also not entirely correct because the control value is retrieved as a `bool` instead of - the correct type - `int32_t`. Additionally, the available modes are not taken into account. To fix this, stores the available modes for `V4L2_CID_EXPOSURE_AUTO` and select the appropriate mode based on the mapping established in the comment. Fixes: bad8d591f8acfa ("libcamera: uvcvideo: Register ExposureTimeMode control") Signed-off-by: Barnabás Pőcze --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 81 +++++++++++++------- 1 file changed, 53 insertions(+), 28 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index dedcac89b..7821cceb0 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -6,6 +6,7 @@ */ #include +#include #include #include #include @@ -58,6 +59,13 @@ public: Stream stream_; std::map> formats_; + std::bitset availableExposureModes_; + private: bool generateId(); @@ -95,8 +103,8 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - int processControl(ControlList *controls, unsigned int id, - const ControlValue &value); + int processControl(UVCCameraData *data, ControlList *controls, + unsigned int id, const ControlValue &value); int processControls(UVCCameraData *data, Request *request); bool acquireDevice(Camera *camera) override; @@ -289,8 +297,8 @@ void PipelineHandlerUVC::stopDevice(Camera *camera) data->video_->releaseBuffers(); } -int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id, - const ControlValue &value) +int PipelineHandlerUVC::processControl(UVCCameraData *data, ControlList *controls, + unsigned int id, const ControlValue &value) { uint32_t cid; @@ -334,10 +342,27 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id, } case V4L2_CID_EXPOSURE_AUTO: { - int32_t ivalue = value.get() - ? V4L2_EXPOSURE_APERTURE_PRIORITY - : V4L2_EXPOSURE_MANUAL; - controls->set(V4L2_CID_EXPOSURE_AUTO, ivalue); + switch (value.get()) { + case controls::ExposureTimeModeAuto: + if (data->availableExposureModes_[V4L2_EXPOSURE_AUTO]) + controls->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_AUTO); + else if (data->availableExposureModes_[V4L2_EXPOSURE_APERTURE_PRIORITY]) + controls->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_APERTURE_PRIORITY); + else + ASSERT(false); + break; + case controls::ExposureTimeModeManual: + if (data->availableExposureModes_[V4L2_EXPOSURE_MANUAL]) + controls->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL); + else if (data->availableExposureModes_[V4L2_EXPOSURE_SHUTTER_PRIORITY]) + controls->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_SHUTTER_PRIORITY); + else + ASSERT(false); + break; + default: + ASSERT(false); + break; + } break; } @@ -375,7 +400,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) @@ -725,25 +750,25 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info, * ExposureTimeModeManual = { V4L2_EXPOSURE_MANUAL, * V4L2_EXPOSURE_SHUTTER_PRIORITY } */ - std::array values{}; - - auto it = std::find_if(v4l2Values.begin(), v4l2Values.end(), - [&](const ControlValue &val) { - return (val.get() == V4L2_EXPOSURE_APERTURE_PRIORITY || - val.get() == V4L2_EXPOSURE_AUTO) ? true : false; - }); - if (it != v4l2Values.end()) - values.back() = static_cast(controls::ExposureTimeModeAuto); - - it = std::find_if(v4l2Values.begin(), v4l2Values.end(), - [&](const ControlValue &val) { - return (val.get() == V4L2_EXPOSURE_SHUTTER_PRIORITY || - val.get() == V4L2_EXPOSURE_MANUAL) ? true : false; - }); - if (it != v4l2Values.end()) - values.back() = static_cast(controls::ExposureTimeModeManual); - - info = ControlInfo{Span{values}, values[0]}; + for (const ControlValue &value : v4l2Values) { + auto x = value.get(); + if (0 <= x && size_t(x) < availableExposureModes_.size()) + availableExposureModes_[x] = true; + } + + std::array values; + std::size_t count = 0; + + if (availableExposureModes_[V4L2_EXPOSURE_AUTO] || availableExposureModes_[V4L2_EXPOSURE_APERTURE_PRIORITY]) + values[count++] = controls::ExposureTimeModeAuto; + + if (availableExposureModes_[V4L2_EXPOSURE_MANUAL] || availableExposureModes_[V4L2_EXPOSURE_SHUTTER_PRIORITY]) + values[count++] = controls::ExposureTimeModeManual; + + if (count == 0) + return; + + info = ControlInfo{Span{ values.data(), count }, values[0]}; break; } case V4L2_CID_EXPOSURE_ABSOLUTE: