From patchwork Wed Nov 26 09:38:53 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 25212 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 AAA71C32DE for ; Wed, 26 Nov 2025 09:39:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B6D1B60AA9; Wed, 26 Nov 2025 10:39:13 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="CrWyUcCY"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A8F0609E0 for ; Wed, 26 Nov 2025 10:39:07 +0100 (CET) Received: from [192.168.0.172] (mob-5-90-63-219.net.vodafone.it [5.90.63.219]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AA36211EB; Wed, 26 Nov 2025 10:36:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1764149817; bh=JquQKFGDNT2oXXwkYN2POmo8yNxXO2Mp7Nl8lJNbuO8=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=CrWyUcCYD1o10pAT5n91x7wP7hbHGbycDYUQBhhIFN25Dg6V7hxC7c0ZUoJ6YmJd9 EUgXpJHDshytuTu4BPrbH0khAO2jJF1Cj0z/yigSWs7yGTPMsO6OJd9AFn7Evvzh3i 84mFewTGEO9TvR6MZK7eLBfViutxzpqduI3JuZkY= From: Jacopo Mondi 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 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 X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=10228; i=jacopo.mondi@ideasonboard.com; h=from:subject:message-id; bh=JquQKFGDNT2oXXwkYN2POmo8yNxXO2Mp7Nl8lJNbuO8=; b=owEBbQKS/ZANAwAKAXI0Bo8WoVY8AcsmYgBpJsq502gPHQyhi6sGADZGvy4gej0Emux0PfRar nWqRcuDxh+JAjMEAAEKAB0WIQS1xD1IgJogio9YOMByNAaPFqFWPAUCaSbKuQAKCRByNAaPFqFW PIWYD/9YDKjKlyfVTQIn36tXBKy4ygN1J3OyJm6+Jbf70CKcVDsR/160Z3m6oULkZfZ2EwaYxLQ Xsfb8snIA+uuSQMRMVJPG218VPakDO6jBN2cniFfJWjy5kYEE6m66PSVbY2qWdJnngw6AeFSR+X mBEaFCGBbPVqMD7DyCzfu6ISWdA1/vnBTlTU0xzmitJeCbofN3c6x+VEMTv0zbF5Sq6jRYf3qPK NWu3G6YgeOayc/ZNyN6vPrUzT7z/LKavqJg0DFhJaMZdnWU/92GH00vu3/WLQkgBE88CFDH62Z5 dYb948nIZBgcW9v1P9P3LLlOycdSY58Vav0n+KlbpqKX0uuP2PyerwwIPqdqDgzho8koS/bz80I /gEubfDM9DVHXHcfrgBjLksyOd59rtU7uonSZb2ofQftdOAumfm91BBEUKY6ugCAvotfLXawOIT JfgcfMF4j7uGf+hosZP/yLBbSC7ukaQJeOEtWQpvkIo49XGr7CxxTiH2gCBx2BP5T5aijGY0RB0 0FHvPbzjdYbXK5Hv4ry6el0/EesH4kvxSWwNv+dKJ71fIA0hVi6kQekljpOxNbvJrzRZG/WJ3bD bTmzRB2hYl105Yi/rkhlDmFic8m0ClxaLFnY8EzuimG06JMtr7T1SlLfynTvyW9vL3guYUEaZkt F/DPvN2C3GUOD4w== X-Developer-Key: i=jacopo.mondi@ideasonboard.com; a=openpgp; fpr=72392EDC88144A65C701EA9BA5826A2587AD026B 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 ControlList inside a Request is initialized with the Camera ControlInfoMap, which lists the limits of the controls supported by the Camera. The Request class currently exposes the control list it contains through the Request::controls() function as a raw pointer, potentially allowing applications to replace the Request's control list with a different one. The following pattern: request->controls() = {}; is then currently valid but opens the door to potential issues, as the new list might be initialized with a different info map or without any info map at all. To avoid applications to override the control list contained in a Request, make the Request::controls() function return a const reference and move the non-const version to the Request::Private class for internal usage. Add to the Request class 4 overloads of the setControl() function that allows applications to set controls on a Request control list without accessing it. Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295 Signed-off-by: Jacopo Mondi --- include/libcamera/internal/request.h | 1 + include/libcamera/request.h | 27 +++++++++++++++++- src/android/camera_device.cpp | 8 +++--- src/apps/cam/camera_session.cpp | 2 +- src/gstreamer/gstlibcamera-controls.cpp.in | 2 +- src/libcamera/camera.cpp | 2 +- src/libcamera/request.cpp | 46 +++++++++++++++++++++--------- src/py/libcamera/py_main.cpp | 2 +- src/v4l2/v4l2_camera.cpp | 2 +- 9 files changed, 69 insertions(+), 23 deletions(-) diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h index 643f67cabf5778f7515c0117d06d4e0a3fdec4f8..4807036b7ae22c0e90886889ff5741fd9c5c0e9f 100644 --- a/include/libcamera/internal/request.h +++ b/include/libcamera/internal/request.h @@ -36,6 +36,7 @@ public: Camera *camera() const { return camera_; } bool hasPendingBuffers() const; + ControlList &controls(); ControlList &metadata() { return *metadata_; } bool completeBuffer(FrameBuffer *buffer); diff --git a/include/libcamera/request.h b/include/libcamera/request.h index c9aeddb62680923dc3490a23dc6c3f70f70cc075..31860e1e1e7680ded00148b7d96ca03f820d1ebc 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -49,7 +49,7 @@ public: void reuse(ReuseFlag flags = Default); - ControlList &controls() { return *controls_; } + const ControlList &controls() const { return *controls_; } const ControlList &metadata() const; const BufferMap &buffers() const { return bufferMap_; } int addBuffer(const Stream *stream, FrameBuffer *buffer, @@ -64,6 +64,31 @@ public: std::string toString() const; + template + void setControl(const Control &ctrl, const V &value) + { + controls_->set(ctrl, value); + } + +#ifndef __DOXYGEN__ + template + void setControl(const Control> &ctrl, + const Span &values) + { + controls_->set(ctrl, values); + } +#endif + + void setControls(const ControlList &other) + { + controls_->merge(other); + } + + void setControl(unsigned int id, const ControlValue &value) + { + controls_->set(id, value); + } + private: LIBCAMERA_DISABLE_COPY(Request) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 80ff248c2aa316d2f7ccc59d4621a7c86770bf5f..80145f6d103546b4214316f58ecf283accd108ff 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -804,19 +804,19 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor) return 0; /* Translate the Android request settings to libcamera controls. */ - ControlList &controls = descriptor->request_->controls(); + Request *req = descriptor->request_.get(); camera_metadata_ro_entry_t entry; if (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) { const int32_t *data = entry.data.i32; Rectangle cropRegion{ data[0], data[1], static_cast(data[2]), static_cast(data[3]) }; - controls.set(controls::ScalerCrop, cropRegion); + req->setControl(controls::ScalerCrop, cropRegion); } if (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) { const uint8_t *data = entry.data.u8; - controls.set(controls::draft::FaceDetectMode, data[0]); + req->setControl(controls::draft::FaceDetectMode, data[0]); } if (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) { @@ -854,7 +854,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor) return -EINVAL; } - controls.set(controls::draft::TestPatternMode, testPatternMode); + req->setControl(controls::draft::TestPatternMode, testPatternMode); } return 0; diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..246806a0502cec465ac85989f7c93d5885427767 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request) return 0; if (script_) - request->controls() = script_->frameControls(queueCount_); + request->setControls(script_->frameControls(queueCount_)); queueCount_++; diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in b/src/gstreamer/gstlibcamera-controls.cpp.in index 6faf3ee7a6c3401bb42c3720505ea9d5d6f4186a..40a543e755e29f29b797417a58537fc2d05fa9ea 100644 --- a/src/gstreamer/gstlibcamera-controls.cpp.in +++ b/src/gstreamer/gstlibcamera-controls.cpp.in @@ -276,7 +276,7 @@ void GstCameraControls::setCamera(const std::shared_ptr &cam) void GstCameraControls::applyControls(std::unique_ptr &request) { - request->controls().merge(controls_); + request->setControls(controls_); controls_.clear(); } diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..1918c6f4340d790e93ac23baa906f038cd30f0bc 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -1366,7 +1366,7 @@ int Camera::queueRequest(Request *request) } /* Pre-process AeEnable. */ - patchControlList(request->controls()); + patchControlList(request->_d()->controls()); d->pipe_->invokeMethod(&PipelineHandler::queueRequest, ConnectionTypeQueued, request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930..c8c47f4c75cad116efcdd95032b5def19c900d95 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -87,6 +87,16 @@ bool Request::Private::hasPendingBuffers() const return !pending_.empty(); } +/** + * \fn Request::Private::controls() + * \brief Retrieve the request's ControlList + * \return A reference to the ControlList in this request + */ +ControlList &Request::Private::controls() +{ + return *_o()->controls_; +} + /** * \fn Request::Private::metadata() * \brief Retrieve the request's metadata @@ -414,19 +424,9 @@ void Request::reuse(ReuseFlag flags) } /** - * \fn Request::controls() - * \brief Retrieve the request's ControlList - * - * Requests store a list of controls to be applied to all frames captured for - * the request. They are created with an empty list of controls that can be - * accessed through this function. Control values can be retrieved using - * ControlList::get() and updated using ControlList::set(). - * - * Only controls supported by the camera to which this request will be - * submitted shall be included in the controls list. Attempting to add an - * unsupported control causes undefined behaviour. - * - * \return A reference to the ControlList in this request + * \fn Request::controls() const + * \brief Retrieve a const reference to the request's ControlList + * \return A const reference to the ControlList in this request */ /** @@ -623,4 +623,24 @@ std::ostream &operator<<(std::ostream &out, const Request &r) return out; } +/** + * \fn Request::setControl(const Control &ctrl, const V &value) + * \brief Set control \a ctrl in the Request + * \param[in] ctrl The control + * \param[in] value The control value + */ + +/** + * \fn Request::setControls(const ControlList &other) + * \brief Merge the control list \a other in the Request + * \param[in] other The control list to add to the Request + */ + +/** + * \fn Request::setControl(unsigned int id, const ControlValue &value) + * \brief Set control \a id in the Request to \a value + * \param[in] id The control numerical id + * \param[in] value The control value + */ + } /* namespace libcamera */ diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index a983ea75c35669cc5d1ff433eddfdd99e9388e7c..97cc6cfbe7fbc4ece5c7a779a9ae6c8c7d23cf7b 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -463,7 +463,7 @@ PYBIND11_MODULE(_libcamera, m) .def_property_readonly("sequence", &Request::sequence) .def_property_readonly("has_pending_buffers", &Request::hasPendingBuffers) .def("set_control", [](Request &self, const ControlId &id, py::object value) { - self.controls().set(id.id(), pyToControlValue(value, id.type())); + self.setControl(id.id(), pyToControlValue(value, id.type())); }) .def_property_readonly("metadata", [](Request &self) { /* Convert ControlList to std container */ diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 94d138cd5710b9bd5c83cacd3cbd963f71aeb3c7..7e918cc3e803cdad67043f1dbdc212d5b57e37f3 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -269,7 +269,7 @@ int V4L2Camera::qbuf(unsigned int index) return 0; } - request->controls().merge(std::move(controls_)); + request->setControls(controls_); ret = camera_->queueRequest(request); if (ret < 0) {