Patch Detail
Show a patch.
GET /api/1.1/patches/22656/?format=api
{ "id": 22656, "url": "https://patchwork.libcamera.org/api/1.1/patches/22656/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22656/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250128121352.494582-2-pobrn@protonmail.com>", "date": "2025-01-28T12:14:01", "name": "[RFC,v1] libcamera: pipeline: uvcvideo: Fix `ExposureTimeMode` control", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "71e11576a5860c83616ad7db61c496d1a9c66c3a", "submitter": { "id": 133, "url": "https://patchwork.libcamera.org/api/1.1/people/133/?format=api", "name": "Pőcze Barnabás", "email": "pobrn@protonmail.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22656/mbox/", "series": [ { "id": 4974, "url": "https://patchwork.libcamera.org/api/1.1/series/4974/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4974", "date": "2025-01-28T12:14:01", "name": "[RFC,v1] libcamera: pipeline: uvcvideo: Fix `ExposureTimeMode` control", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4974/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22656/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22656/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2FFCCC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 28 Jan 2025 12:14:08 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C52FF68557;\n\tTue, 28 Jan 2025 13:14:07 +0100 (CET)", "from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC0AC68557\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 28 Jan 2025 13:14:06 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"x96FKtNm\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1738066446; x=1738325646;\n\tbh=oHxofpdrwJrlHjbxC+EwDE/hgtI0/pNMoAYVz5tyKAo=;\n\th=Date:To:From:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=x96FKtNmFI9+NaLdVnMxkELdhC/QPk1SuCzSk9KTnSk5xHS4fKCqcpxWAW7MF47aB\n\tw6F1uKVSMxEgl6mjcUHQ4/Dy+EzqJzAArjhtf0umtg9+F7pnW4NU57SuZ+tUNyqB2Z\n\trXOfDJftp/hR75BmcHBbaOa4VHc0pgmymERpZPbnFzGgRDNGD2Fx4ICoBwwySbpkOb\n\tzRvTtfTdFdB67FOess71q+joU0BgTMph2P23nhpvxz+qSbwXYLUZH1TVcNsllNJP1M\n\tunZ4svHmahyW23aarrxAZysSrQdzT9NDZ/HTEuAWuVOhVkhXfM+ABVCRg9rD8dqX18\n\tviqZBUyDS9yHA==", "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?= <pobrn@protonmail.com>", "Subject": "[RFC PATCH v1] libcamera: pipeline: uvcvideo: Fix `ExposureTimeMode`\n\tcontrol", "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", "Content-Type": "text/plain; charset=utf-8", "Content-Transfer-Encoding": "quoted-printable", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "`ControlInfo(Span<const int32_t>{...})` calls the incorrect constructor\nof `ControlInfo`. The intended constructor to be called is\n`ControlInfo(Span<const ControlValue>, ...)` however that is not called\nbecause a span of `const int32_t` is passed. Instead, the constructor\n`ControlInfo(const ControlValue &min, const ControlValue &max, ...)`\nwill be called.\n\nTo fix this, simply pass a span of `ControlValue` instead.\n\nFurthermore, the mapping in `UVCCameraData::processControl()` is also not\nentirely correct because the control value is retrieved as a `bool`\ninstead of - the correct type - `int32_t`. Additionally, the available\nmodes are not taken into account.\n\nTo fix this, stores the available modes for `V4L2_CID_EXPOSURE_AUTO`\nand select the appropriate mode based on the mapping established\nin the comment.\n\nFixes: bad8d591f8acfa (\"libcamera: uvcvideo: Register ExposureTimeMode control\")\nSigned-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n---\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 81 +++++++++++++-------\n 1 file changed, 53 insertions(+), 28 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\nindex dedcac89b..7821cceb0 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -6,6 +6,7 @@\n */\n \n #include <algorithm>\n+#include <bitset>\n #include <cmath>\n #include <fstream>\n #include <map>\n@@ -58,6 +59,13 @@ public:\n \tStream stream_;\n \tstd::map<PixelFormat, std::vector<SizeRange>> formats_;\n \n+\tstd::bitset<std::max({\n+\t\tV4L2_EXPOSURE_AUTO,\n+\t\tV4L2_EXPOSURE_MANUAL,\n+\t\tV4L2_EXPOSURE_APERTURE_PRIORITY,\n+\t\tV4L2_EXPOSURE_SHUTTER_PRIORITY,\n+\t}) + 1> availableExposureModes_;\n+\n private:\n \tbool generateId();\n \n@@ -95,8 +103,8 @@ public:\n \tbool match(DeviceEnumerator *enumerator) override;\n \n private:\n-\tint processControl(ControlList *controls, unsigned int id,\n-\t\t\t const ControlValue &value);\n+\tint processControl(UVCCameraData *data, ControlList *controls,\n+\t\t\t unsigned int id, const ControlValue &value);\n \tint processControls(UVCCameraData *data, Request *request);\n \n \tbool acquireDevice(Camera *camera) override;\n@@ -289,8 +297,8 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n \tdata->video_->releaseBuffers();\n }\n \n-int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n-\t\t\t\t const ControlValue &value)\n+int PipelineHandlerUVC::processControl(UVCCameraData *data, ControlList *controls,\n+\t\t\t\t unsigned int id, const ControlValue &value)\n {\n \tuint32_t cid;\n \n@@ -334,10 +342,27 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n \t}\n \n \tcase V4L2_CID_EXPOSURE_AUTO: {\n-\t\tint32_t ivalue = value.get<bool>()\n-\t\t\t ? V4L2_EXPOSURE_APERTURE_PRIORITY\n-\t\t\t : V4L2_EXPOSURE_MANUAL;\n-\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, ivalue);\n+\t\tswitch (value.get<int32_t>()) {\n+\t\tcase controls::ExposureTimeModeAuto:\n+\t\t\tif (data->availableExposureModes_[V4L2_EXPOSURE_AUTO])\n+\t\t\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_AUTO);\n+\t\t\telse if (data->availableExposureModes_[V4L2_EXPOSURE_APERTURE_PRIORITY])\n+\t\t\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_APERTURE_PRIORITY);\n+\t\t\telse\n+\t\t\t\tASSERT(false);\n+\t\t\tbreak;\n+\t\tcase controls::ExposureTimeModeManual:\n+\t\t\tif (data->availableExposureModes_[V4L2_EXPOSURE_MANUAL])\n+\t\t\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);\n+\t\t\telse if (data->availableExposureModes_[V4L2_EXPOSURE_SHUTTER_PRIORITY])\n+\t\t\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_SHUTTER_PRIORITY);\n+\t\t\telse\n+\t\t\t\tASSERT(false);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tASSERT(false);\n+\t\t\tbreak;\n+\t\t}\n \t\tbreak;\n \t}\n \n@@ -375,7 +400,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n \tControlList controls(data->video_->controls());\n \n \tfor (const auto &[id, value] : request->controls())\n-\t\tprocessControl(&controls, id, value);\n+\t\tprocessControl(data, &controls, id, value);\n \n \tfor (const auto &ctrl : controls)\n \t\tLOG(UVC, Debug)\n@@ -725,25 +750,25 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n \t\t * ExposureTimeModeManual = { V4L2_EXPOSURE_MANUAL,\n \t\t *\t\t\t V4L2_EXPOSURE_SHUTTER_PRIORITY }\n \t\t */\n-\t\tstd::array<int32_t, 2> values{};\n-\n-\t\tauto it = std::find_if(v4l2Values.begin(), v4l2Values.end(),\n-\t\t\t[&](const ControlValue &val) {\n-\t\t\t\treturn (val.get<int32_t>() == V4L2_EXPOSURE_APERTURE_PRIORITY ||\n-\t\t\t\t\tval.get<int32_t>() == V4L2_EXPOSURE_AUTO) ? true : false;\n-\t\t\t});\n-\t\tif (it != v4l2Values.end())\n-\t\t\tvalues.back() = static_cast<int32_t>(controls::ExposureTimeModeAuto);\n-\n-\t\tit = std::find_if(v4l2Values.begin(), v4l2Values.end(),\n-\t\t\t[&](const ControlValue &val) {\n-\t\t\t\treturn (val.get<int32_t>() == V4L2_EXPOSURE_SHUTTER_PRIORITY ||\n-\t\t\t\t\tval.get<int32_t>() == V4L2_EXPOSURE_MANUAL) ? true : false;\n-\t\t\t});\n-\t\tif (it != v4l2Values.end())\n-\t\t\tvalues.back() = static_cast<int32_t>(controls::ExposureTimeModeManual);\n-\n-\t\tinfo = ControlInfo{Span<int32_t>{values}, values[0]};\n+\t\tfor (const ControlValue &value : v4l2Values) {\n+\t\t\tauto x = value.get<int32_t>();\n+\t\t\tif (0 <= x && size_t(x) < availableExposureModes_.size())\n+\t\t\t\tavailableExposureModes_[x] = true;\n+\t\t}\n+\n+\t\tstd::array<ControlValue, 2> values;\n+\t\tstd::size_t count = 0;\n+\n+\t\tif (availableExposureModes_[V4L2_EXPOSURE_AUTO] || availableExposureModes_[V4L2_EXPOSURE_APERTURE_PRIORITY])\n+\t\t\tvalues[count++] = controls::ExposureTimeModeAuto;\n+\n+\t\tif (availableExposureModes_[V4L2_EXPOSURE_MANUAL] || availableExposureModes_[V4L2_EXPOSURE_SHUTTER_PRIORITY])\n+\t\t\tvalues[count++] = controls::ExposureTimeModeManual;\n+\n+\t\tif (count == 0)\n+\t\t\treturn;\n+\n+\t\tinfo = ControlInfo{Span<const ControlValue>{ values.data(), count }, values[0]};\n \t\tbreak;\n \t}\n \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n", "prefixes": [ "RFC", "v1" ] }