From patchwork Tue Mar 24 12:32:13 2026 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: 26321 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 69288BE086 for ; Tue, 24 Mar 2026 12:32:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EBC4C62784; Tue, 24 Mar 2026 13:32:19 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OFARsvtD"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2BDD0622AD for ; Tue, 24 Mar 2026 13:32:18 +0100 (CET) Received: from pb-laptop.local (185.221.143.129.nat.pool.zt.hu [185.221.143.129]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C4994225 for ; Tue, 24 Mar 2026 13:31:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774355460; bh=u9MtYestEjUN5b0tDlOfSTHp37URP6Ygarwmakksb5g=; h=From:To:Subject:Date:From; b=OFARsvtDYA0D0ytRyGp853v/7puzeCO1AHuLwa+PiMFYILVPKN+5RxeLfz6AHN8Ye UcSR7593q+jdw6nNP3sHvfLPaqOR8FLjilRyfSEXcezVJwx/s9t9lNsmi0OKKZqFHk 8uJen98GTent8wnE94C2u65Dvbiopf3JtYZ1zcaM= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v1 1/2] libcamera: v4l2_device: setControls(): Do not return index Date: Tue, 24 Mar 2026 13:32:13 +0100 Message-ID: <20260324123214.1762198-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.53.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" This behaviour should be removed for the following reasons: * a `ControlList` is an unordered container, an index is largely meaningless; * most callers either ignore the return value, or blindly return it, likely contradicting their own documentation (e.g. `CameraSensorLegacy::setFormat()`), and the error is already very visibly logged; * an error index of 0 is indistinguishable from success with this approach. Instead, return `-EIO` instead of the index. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham --- Documentation/guides/pipeline-handler.rst | 6 ++---- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc/vimc.cpp | 2 +- src/libcamera/sensor/camera_sensor.cpp | 15 +-------------- src/libcamera/v4l2_device.cpp | 14 ++++++-------- 5 files changed, 11 insertions(+), 28 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 083cdb745..b33d8b857 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -1076,7 +1076,7 @@ device: ret = data->video_->setControls(&controls); if (ret) { LOG(VIVID, Error) << "Failed to set controls: " << ret; - return ret < 0 ? ret : -EINVAL; + return ret; } These controls configure VIVID to use a default test pattern, and enable all @@ -1285,10 +1285,8 @@ before being set. << " to " << ctrl.second.toString(); int ret = data->video_->setControls(&controls); - if (ret) { + if (ret) LOG(VIVID, Error) << "Failed to set controls: " << ret; - return ret < 0 ? ret : -EINVAL; - } return ret; } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 3bd51733d..3435a7604 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -439,7 +439,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList & int ret = data->video_->setControls(&controls); if (ret) { LOG(UVC, Error) << "Failed to set controls: " << ret; - return ret < 0 ? ret : -EINVAL; + return ret; } return ret; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 08b100752..716f90818 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -434,7 +434,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) int ret = data->sensor_->setControls(&controls); if (ret) { LOG(VIMC, Error) << "Failed to set controls: " << ret; - return ret < 0 ? ret : -EINVAL; + return ret; } return ret; diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp index 05390d1e1..602518afc 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -379,21 +379,8 @@ int CameraSensor::setEmbeddedDataEnabled(bool enable) * ctrls entry. The control identifiers are defined by the V4L2 specification * (V4L2_CID_*). * - * If any control in \a ctrls is not supported by the device, is disabled (i.e. - * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other - * error occurs during validation of the requested controls, no control is - * written and this function returns -EINVAL. - * - * If an error occurs while writing the controls, the index of the first - * control that couldn't be written is returned. All controls below that index - * are written and their values are updated in \a ctrls, while all other - * controls are not written and their values are not changed. - * - * \sa V4L2Device::setControls() - * * \return 0 on success or an error code otherwise - * \retval -EINVAL One of the control is not supported or not accessible - * \retval i The index of the control that failed + * \sa V4L2Device::setControls() */ /** diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index b49d73b1c..53bd7865a 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -285,18 +285,16 @@ ControlList V4L2Device::getControls(Span ids, const V4L2Request * \param[in] request An optional request * * This function writes the value of all controls contained in \a ctrls, and - * stores the values actually applied to the device in the corresponding - * \a ctrls entry. + * updates \a ctrls with the values actually applied to the device. * * If any control in \a ctrls is not supported by the device, is disabled (i.e. * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, if any other error * occurs during validation of the requested controls, no control is written and * this function returns -EINVAL. * - * If an error occurs while writing the controls, the index of the first - * control that couldn't be written is returned. All controls below that index - * are written and their values are updated in \a ctrls, while all other - * controls are not written and their values are not changed. + * If an error occurs while writing the controls, -EIO is returned. All controls + * that were successfully written have their values are updated in \a ctrls, + * while all other controls are not written and their values are not changed. * * If \a request is set, the controls will be applied to that request. If the * device doesn't support requests, -EACCESS will be returned. If \a request is @@ -304,8 +302,8 @@ ControlList V4L2Device::getControls(Span ids, const V4L2Request * * \return 0 on success or an error code otherwise * \retval -EINVAL One of the controls is not supported or not accessible + * \retval -EIO One or more controls were rejected by the device * \retval -EACCESS The device does not support requests - * \retval i The index of the control that failed */ int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) { @@ -419,7 +417,7 @@ int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) << ": " << strerror(-ret); v4l2Ctrls.resize(errorIdx); - ret = errorIdx; + ret = -EIO; } updateControls(ctrls, v4l2Ctrls);