{"id":25212,"url":"https://patchwork.libcamera.org/api/patches/25212/?format=json","web_url":"https://patchwork.libcamera.org/patch/25212/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20251126-cam-control-override-v2-3-305024a1f190@ideasonboard.com>","date":"2025-11-26T09:38:53","name":"[v2,3/3] libcamera: request: Remove write-access to controls_","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"646573959748d933adff3b105230c0651aa9fd03","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/?format=json","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/25212/mbox/","series":[{"id":5614,"url":"https://patchwork.libcamera.org/api/series/5614/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5614","date":"2025-11-26T09:38:50","name":"libcamera: ipc: ControlLists without valid idmap break IPC","version":2,"mbox":"https://patchwork.libcamera.org/series/5614/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/25212/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/25212/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 AAA71C32DE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 26 Nov 2025 09:39:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6D1B60AA9;\n\tWed, 26 Nov 2025 10:39:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A8F0609E0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 26 Nov 2025 10:39:07 +0100 (CET)","from [192.168.0.172] (mob-5-90-63-219.net.vodafone.it\n\t[5.90.63.219])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA36211EB;\n\tWed, 26 Nov 2025 10:36:57 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CrWyUcCY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764149817;\n\tbh=JquQKFGDNT2oXXwkYN2POmo8yNxXO2Mp7Nl8lJNbuO8=;\n\th=From:Date:Subject:References:In-Reply-To:To:Cc:From;\n\tb=CrWyUcCYD1o10pAT5n91x7wP7hbHGbycDYUQBhhIFN25Dg6V7hxC7c0ZUoJ6YmJd9\n\tEUgXpJHDshytuTu4BPrbH0khAO2jJF1Cj0z/yigSWs7yGTPMsO6OJd9AFn7Evvzh3i\n\t84mFewTGEO9TvR6MZK7eLBfViutxzpqduI3JuZkY=","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Wed, 26 Nov 2025 10:38:53 +0100","Subject":"[PATCH v2 3/3] libcamera: request: Remove write-access to controls_","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"7bit","Message-Id":"<20251126-cam-control-override-v2-3-305024a1f190@ideasonboard.com>","References":"<20251126-cam-control-override-v2-0-305024a1f190@ideasonboard.com>","In-Reply-To":"<20251126-cam-control-override-v2-0-305024a1f190@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","X-Mailer":"b4 0.14.2","X-Developer-Signature":"v=1; a=openpgp-sha256; l=10228;\n\ti=jacopo.mondi@ideasonboard.com; h=from:subject:message-id;\n\tbh=JquQKFGDNT2oXXwkYN2POmo8yNxXO2Mp7Nl8lJNbuO8=;\n\tb=owEBbQKS/ZANAwAKAXI0Bo8WoVY8AcsmYgBpJsq502gPHQyhi6sGADZGvy4gej0Emux0PfRar\n\tnWqRcuDxh+JAjMEAAEKAB0WIQS1xD1IgJogio9YOMByNAaPFqFWPAUCaSbKuQAKCRByNAaPFqFW\n\tPIWYD/9YDKjKlyfVTQIn36tXBKy4ygN1J3OyJm6+Jbf70CKcVDsR/160Z3m6oULkZfZ2EwaYxLQ\n\tXsfb8snIA+uuSQMRMVJPG218VPakDO6jBN2cniFfJWjy5kYEE6m66PSVbY2qWdJnngw6AeFSR+X\n\tmBEaFCGBbPVqMD7DyCzfu6ISWdA1/vnBTlTU0xzmitJeCbofN3c6x+VEMTv0zbF5Sq6jRYf3qPK\n\tNWu3G6YgeOayc/ZNyN6vPrUzT7z/LKavqJg0DFhJaMZdnWU/92GH00vu3/WLQkgBE88CFDH62Z5\n\tdYb948nIZBgcW9v1P9P3LLlOycdSY58Vav0n+KlbpqKX0uuP2PyerwwIPqdqDgzho8koS/bz80I\n\t/gEubfDM9DVHXHcfrgBjLksyOd59rtU7uonSZb2ofQftdOAumfm91BBEUKY6ugCAvotfLXawOIT\n\tJfgcfMF4j7uGf+hosZP/yLBbSC7ukaQJeOEtWQpvkIo49XGr7CxxTiH2gCBx2BP5T5aijGY0RB0\n\t0FHvPbzjdYbXK5Hv4ry6el0/EesH4kvxSWwNv+dKJ71fIA0hVi6kQekljpOxNbvJrzRZG/WJ3bD\n\tbTmzRB2hYl105Yi/rkhlDmFic8m0ClxaLFnY8EzuimG06JMtr7T1SlLfynTvyW9vL3guYUEaZkt\n\tF/DPvN2C3GUOD4w==","X-Developer-Key":"i=jacopo.mondi@ideasonboard.com; a=openpgp;\n\tfpr=72392EDC88144A65C701EA9BA5826A2587AD026B","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":"The ControlList inside a Request is initialized with the Camera\nControlInfoMap, which lists the limits of the controls supported\nby the Camera.\n\nThe Request class currently exposes the control list it contains through\nthe Request::controls() function as a raw pointer, potentially allowing\napplications to replace the Request's control list with a different one.\n\nThe following pattern:\n\n\trequest->controls() = {};\n\nis then currently valid but opens the door to potential issues, as the\nnew list might be initialized with a different info map or without any\ninfo map at all.\n\nTo avoid applications to override the control list contained in a\nRequest, make the Request::controls() function return a const reference\nand move the non-const version to the Request::Private class for\ninternal usage.\n\nAdd to the Request class 4 overloads of the setControl() function that\nallows applications to set controls on a Request control list without\naccessing it.\n\nCloses: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\nSigned-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n---\n include/libcamera/internal/request.h       |  1 +\n include/libcamera/request.h                | 27 +++++++++++++++++-\n src/android/camera_device.cpp              |  8 +++---\n src/apps/cam/camera_session.cpp            |  2 +-\n src/gstreamer/gstlibcamera-controls.cpp.in |  2 +-\n src/libcamera/camera.cpp                   |  2 +-\n src/libcamera/request.cpp                  | 46 +++++++++++++++++++++---------\n src/py/libcamera/py_main.cpp               |  2 +-\n src/v4l2/v4l2_camera.cpp                   |  2 +-\n 9 files changed, 69 insertions(+), 23 deletions(-)","diff":"diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\nindex 643f67cabf5778f7515c0117d06d4e0a3fdec4f8..4807036b7ae22c0e90886889ff5741fd9c5c0e9f 100644\n--- a/include/libcamera/internal/request.h\n+++ b/include/libcamera/internal/request.h\n@@ -36,6 +36,7 @@ public:\n \tCamera *camera() const { return camera_; }\n \tbool hasPendingBuffers() const;\n \n+\tControlList &controls();\n \tControlList &metadata() { return *metadata_; }\n \n \tbool completeBuffer(FrameBuffer *buffer);\ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex c9aeddb62680923dc3490a23dc6c3f70f70cc075..31860e1e1e7680ded00148b7d96ca03f820d1ebc 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -49,7 +49,7 @@ public:\n \n \tvoid reuse(ReuseFlag flags = Default);\n \n-\tControlList &controls() { return *controls_; }\n+\tconst ControlList &controls() const { return *controls_; }\n \tconst ControlList &metadata() const;\n \tconst BufferMap &buffers() const { return bufferMap_; }\n \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n@@ -64,6 +64,31 @@ public:\n \n \tstd::string toString() const;\n \n+\ttemplate<typename T, typename V>\n+\tvoid setControl(const Control<T> &ctrl, const V &value)\n+\t{\n+\t\tcontrols_->set(ctrl, value);\n+\t}\n+\n+#ifndef __DOXYGEN__\n+\ttemplate<typename T, typename V, std::size_t Size>\n+\tvoid setControl(const Control<Span<const T, Size>> &ctrl,\n+\t\t\tconst Span<const V, Size> &values)\n+\t{\n+\t\tcontrols_->set(ctrl, values);\n+\t}\n+#endif\n+\n+\tvoid setControls(const ControlList &other)\n+\t{\n+\t\tcontrols_->merge(other);\n+\t}\n+\n+\tvoid setControl(unsigned int id, const ControlValue &value)\n+\t{\n+\t\tcontrols_->set(id, value);\n+\t}\n+\n private:\n \tLIBCAMERA_DISABLE_COPY(Request)\n \ndiff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 80ff248c2aa316d2f7ccc59d4621a7c86770bf5f..80145f6d103546b4214316f58ecf283accd108ff 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -804,19 +804,19 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n \t\treturn 0;\n \n \t/* Translate the Android request settings to libcamera controls. */\n-\tControlList &controls = descriptor->request_->controls();\n+\tRequest *req = descriptor->request_.get();\n \tcamera_metadata_ro_entry_t entry;\n \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n \t\tconst int32_t *data = entry.data.i32;\n \t\tRectangle cropRegion{ data[0], data[1],\n \t\t\t\t      static_cast<unsigned int>(data[2]),\n \t\t\t\t      static_cast<unsigned int>(data[3]) };\n-\t\tcontrols.set(controls::ScalerCrop, cropRegion);\n+\t\treq->setControl(controls::ScalerCrop, cropRegion);\n \t}\n \n \tif (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) {\n \t\tconst uint8_t *data = entry.data.u8;\n-\t\tcontrols.set(controls::draft::FaceDetectMode, data[0]);\n+\t\treq->setControl(controls::draft::FaceDetectMode, data[0]);\n \t}\n \n \tif (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) {\n@@ -854,7 +854,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n \t\t\treturn -EINVAL;\n \t\t}\n \n-\t\tcontrols.set(controls::draft::TestPatternMode, testPatternMode);\n+\t\treq->setControl(controls::draft::TestPatternMode, testPatternMode);\n \t}\n \n \treturn 0;\ndiff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\nindex 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..246806a0502cec465ac85989f7c93d5885427767 100644\n--- a/src/apps/cam/camera_session.cpp\n+++ b/src/apps/cam/camera_session.cpp\n@@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n \t\treturn 0;\n \n \tif (script_)\n-\t\trequest->controls() = script_->frameControls(queueCount_);\n+\t\trequest->setControls(script_->frameControls(queueCount_));\n \n \tqueueCount_++;\n \ndiff --git a/src/gstreamer/gstlibcamera-controls.cpp.in b/src/gstreamer/gstlibcamera-controls.cpp.in\nindex 6faf3ee7a6c3401bb42c3720505ea9d5d6f4186a..40a543e755e29f29b797417a58537fc2d05fa9ea 100644\n--- a/src/gstreamer/gstlibcamera-controls.cpp.in\n+++ b/src/gstreamer/gstlibcamera-controls.cpp.in\n@@ -276,7 +276,7 @@ void GstCameraControls::setCamera(const std::shared_ptr<libcamera::Camera> &cam)\n \n void GstCameraControls::applyControls(std::unique_ptr<libcamera::Request> &request)\n {\n-\trequest->controls().merge(controls_);\n+\trequest->setControls(controls_);\n \tcontrols_.clear();\n }\n \ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 2e1e146a25b13b94f3a0df5935c0861f78c949ed..1918c6f4340d790e93ac23baa906f038cd30f0bc 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -1366,7 +1366,7 @@ int Camera::queueRequest(Request *request)\n \t}\n \n \t/* Pre-process AeEnable. */\n-\tpatchControlList(request->controls());\n+\tpatchControlList(request->_d()->controls());\n \n \td->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n \t\t\t       ConnectionTypeQueued, request);\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930..c8c47f4c75cad116efcdd95032b5def19c900d95 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -87,6 +87,16 @@ bool Request::Private::hasPendingBuffers() const\n \treturn !pending_.empty();\n }\n \n+/**\n+ * \\fn Request::Private::controls()\n+ * \\brief Retrieve the request's ControlList\n+ * \\return A reference to the ControlList in this request\n+ */\n+ControlList &Request::Private::controls()\n+{\n+\treturn *_o<Request>()->controls_;\n+}\n+\n /**\n  * \\fn Request::Private::metadata()\n  * \\brief Retrieve the request's metadata\n@@ -414,19 +424,9 @@ void Request::reuse(ReuseFlag flags)\n }\n \n /**\n- * \\fn Request::controls()\n- * \\brief Retrieve the request's ControlList\n- *\n- * Requests store a list of controls to be applied to all frames captured for\n- * the request. They are created with an empty list of controls that can be\n- * accessed through this function. Control values can be retrieved using\n- * ControlList::get() and updated using ControlList::set().\n- *\n- * Only controls supported by the camera to which this request will be\n- * submitted shall be included in the controls list. Attempting to add an\n- * unsupported control causes undefined behaviour.\n- *\n- * \\return A reference to the ControlList in this request\n+ * \\fn Request::controls() const\n+ * \\brief Retrieve a const reference to the request's ControlList\n+ * \\return A const reference to the ControlList in this request\n  */\n \n /**\n@@ -623,4 +623,24 @@ std::ostream &operator<<(std::ostream &out, const Request &r)\n \treturn out;\n }\n \n+/**\n+ * \\fn Request::setControl(const Control<T> &ctrl, const V &value)\n+ * \\brief Set control \\a ctrl in the Request\n+ * \\param[in] ctrl The control\n+ * \\param[in] value The control value\n+ */\n+\n+/**\n+ * \\fn Request::setControls(const ControlList &other)\n+ * \\brief Merge the control list \\a other in the Request\n+ * \\param[in] other The control list to add to the Request\n+ */\n+\n+/**\n+ * \\fn Request::setControl(unsigned int id, const ControlValue &value)\n+ * \\brief Set control \\a id in the Request to \\a value\n+ * \\param[in] id The control numerical id\n+ * \\param[in] value The control value\n+ */\n+\n } /* namespace libcamera */\ndiff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\nindex a983ea75c35669cc5d1ff433eddfdd99e9388e7c..97cc6cfbe7fbc4ece5c7a779a9ae6c8c7d23cf7b 100644\n--- a/src/py/libcamera/py_main.cpp\n+++ b/src/py/libcamera/py_main.cpp\n@@ -463,7 +463,7 @@ PYBIND11_MODULE(_libcamera, m)\n \t\t.def_property_readonly(\"sequence\", &Request::sequence)\n \t\t.def_property_readonly(\"has_pending_buffers\", &Request::hasPendingBuffers)\n \t\t.def(\"set_control\", [](Request &self, const ControlId &id, py::object value) {\n-\t\t\tself.controls().set(id.id(), pyToControlValue(value, id.type()));\n+\t\t\tself.setControl(id.id(), pyToControlValue(value, id.type()));\n \t\t})\n \t\t.def_property_readonly(\"metadata\", [](Request &self) {\n \t\t\t/* Convert ControlList to std container */\ndiff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\nindex 94d138cd5710b9bd5c83cacd3cbd963f71aeb3c7..7e918cc3e803cdad67043f1dbdc212d5b57e37f3 100644\n--- a/src/v4l2/v4l2_camera.cpp\n+++ b/src/v4l2/v4l2_camera.cpp\n@@ -269,7 +269,7 @@ int V4L2Camera::qbuf(unsigned int index)\n \t\treturn 0;\n \t}\n \n-\trequest->controls().merge(std::move(controls_));\n+\trequest->setControls(controls_);\n \n \tret = camera_->queueRequest(request);\n \tif (ret < 0) {\n","prefixes":["v2","3/3"]}