From patchwork Wed Aug 5 10:12:03 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 27616 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 7C7DDC3308 for ; Wed, 5 Aug 2026 10:12:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6A17D680D1; Wed, 5 Aug 2026 12:12:13 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="h/RNT/XB"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 402586805D for ; Wed, 5 Aug 2026 12:12:12 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:93e6:205b:94ed:a2d1]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AEADF1C76; Wed, 5 Aug 2026 12:11:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1785924661; bh=zQOKAUshOijsu3mT3+1RHb303HDgXJZD+m5TTKNff/4=; h=From:To:Cc:Subject:Date:From; b=h/RNT/XB5t+viQRUnzjfF33IMWbBnoOPp930kFAHl6TimzHxEYGdkEMqhpRv+KmqB L96rIdmDRD5yVo0pofK7mT1nFW5U0WEiGgeXJ3wIRyi4vTSRid53AdWxjEmqG4hVed 5GXTyemRjSmTOTLYYHwJP792Q+BuTDhNHHV7eowI= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3] sensor: camera_sensor_legacy/raw: Support separate H/V flips Date: Wed, 5 Aug 2026 12:12:03 +0200 Message-ID: <20260805101207.1177471-1-stefan.klug@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" CameraSensorLegacy and CameraSensorRaw support flipping on the sensor side only if the driver supports both, V4L2_CID_HFLIP and V4L2_CID_VFLIP. In cases where a driver only supports a single flip mode, flipping support gets disabled completely. This is unexpected and annoying when validating sensor drivers. Fix that by handling vertical and horizontal flips separately. Signed-off-by: Stefan Klug Reviewed-by: Jacopo Mondi --- Changes in v3: - Swapped if and replaced ternary operator by || in support logic - Improved description in sensor_driver_requirements Changes in v2: - Replaced duplicate query flip code with lambda - Reworked apply flip code to call setControls only once - Replaced a & by && - Applied the change to CameraSensorRaw as well - Small comment/logging changes --- Hi all, I had this patch somewhere in my tree for a long time. Now I stumbled over this issue again and had to manually rebase the changes. So maybe it is time to upstream :-) Version 3 adresses the comments from last review. Best regards, Stefan Documentation/sensor_driver_requirements.rst | 2 + src/libcamera/sensor/camera_sensor_legacy.cpp | 76 ++++++++++-------- src/libcamera/sensor/camera_sensor_raw.cpp | 79 +++++++++++-------- 3 files changed, 90 insertions(+), 67 deletions(-) diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst index 0e516b34a215..1caa2f96d682 100644 --- a/Documentation/sensor_driver_requirements.rst +++ b/Documentation/sensor_driver_requirements.rst @@ -73,6 +73,8 @@ In order to support rotating the image the sensor driver should support The controls must be writable from userspace. In case of a RAW Bayer sensors, drivers should correctly report if vertical/horizontal flips modify the Bayer pattern ordering by reporting the `V4L2_CTRL_FLAG_MODIFY_LAYOUT` control flag. +If the flag is set for one of the two controls, it shall be set for the other +one as well. The sensor driver should implement support for the V4L2 Selection API, specifically it should implement support for the diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index 6a683821f219..751d6f3e1c04 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -133,7 +133,8 @@ private: Size pixelArraySize_; Rectangle activeArea_; const BayerFormat *bayerFormat_; - bool supportFlips_; + bool supportHFlips_; + bool supportVFlips_; bool flipsAlterBayerOrder_; Orientation mountingOrientation_; @@ -153,7 +154,7 @@ private: CameraSensorLegacy::CameraSensorLegacy(const MediaEntity *entity) : entity_(entity), pad_(UINT_MAX), staticProps_(nullptr), - bayerFormat_(nullptr), supportFlips_(false), + bayerFormat_(nullptr), supportHFlips_(false), supportVFlips_(false), flipsAlterBayerOrder_(false), properties_(properties::properties) { } @@ -359,26 +360,31 @@ int CameraSensorLegacy::validateSensorDriver() } } - /* - * Verify if sensor supports horizontal/vertical flips - * - * \todo Handle horizontal and vertical flips independently. - */ - const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP); - const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP); - if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) && - vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { - supportFlips_ = true; + /* Verify if sensor supports horizontal/vertical flips. */ + auto queryFlip = [&](uint32_t id, const char *name) -> std::pair { + const struct v4l2_query_ext_ctrl *flipInfo = subdev_->controlInfo(id); + if (!flipInfo || flipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) { + LOG(CameraSensor, Debug) + << "Camera sensor does not support " << name << " flip"; + return { false, false }; + } - if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT || - vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT) - flipsAlterBayerOrder_ = true; + return { true, flipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT }; + }; + + auto hflip = queryFlip(V4L2_CID_HFLIP, "horizontal"); + auto vflip = queryFlip(V4L2_CID_HFLIP, "vertical"); + + if (hflip.first && vflip.first && (hflip.second != vflip.second)) { + LOG(CameraSensor, Error) + << "Flips with differing alter bayer order properties" + << " are unsupported. Disabling flipping."; + } else { + supportHFlips_ = hflip.first; + supportVFlips_ = vflip.first; + flipsAlterBayerOrder_ = hflip.second || vflip.second; } - if (!supportFlips_) - LOG(CameraSensor, Debug) - << "Camera sensor does not support horizontal/vertical flip"; - /* * Make sure the required selection targets are supported. * @@ -759,14 +765,16 @@ CameraSensorLegacy::getFormat(Span mbusCodes, int CameraSensorLegacy::setFormat(V4L2SubdeviceFormat *format, Transform transform) { /* Configure flips if the sensor supports that. */ - if (supportFlips_) { - ControlList flipCtrls(subdev_->controls()); - + ControlList flipCtrls(subdev_->controls()); + if (supportHFlips_) flipCtrls.set(V4L2_CID_HFLIP, static_cast(!!(transform & Transform::HFlip))); + + if (supportVFlips_) flipCtrls.set(V4L2_CID_VFLIP, static_cast(!!(transform & Transform::VFlip))); + if (!flipCtrls.empty()) { int ret = subdev_->setControls(&flipCtrls); if (ret) return ret; @@ -937,17 +945,8 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const Transform CameraSensorLegacy::computeTransform(Orientation *orientation) const { /* - * If we cannot do any flips we cannot change the native camera mounting - * orientation. - */ - if (!supportFlips_) { - *orientation = mountingOrientation_; - return Transform::Identity; - } - - /* - * Now compute the required transform to obtain 'orientation' starting - * from the mounting rotation. + * Compute the required transform to obtain 'orientation' starting from + * the mounting rotation. * * As a note: * orientation / mountingOrientation_ = transform @@ -955,6 +954,17 @@ Transform CameraSensorLegacy::computeTransform(Orientation *orientation) const */ Transform transform = *orientation / mountingOrientation_; + /* If we cannot do the required flips, fall back to identity. */ + if (!supportHFlips_ && !!(transform & Transform::HFlip)) { + *orientation = mountingOrientation_; + return Transform::Identity; + } + + if (!supportVFlips_ && !!(transform & Transform::VFlip)) { + *orientation = mountingOrientation_; + return Transform::Identity; + } + /* * If transform contains any Transpose we cannot do it, so adjust * 'orientation' to report the image native orientation and return Identity. diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp index 10eba0331fe8..52718bf0c3f5 100644 --- a/src/libcamera/sensor/camera_sensor_raw.cpp +++ b/src/libcamera/sensor/camera_sensor_raw.cpp @@ -144,7 +144,8 @@ private: Size pixelArraySize_; Rectangle activeArea_; BayerFormat::Order cfaPattern_; - bool supportFlips_; + bool supportHFlips_; + bool supportVFlips_; bool flipsAlterBayerOrder_; Orientation mountingOrientation_; @@ -162,8 +163,9 @@ private: */ CameraSensorRaw::CameraSensorRaw(const MediaEntity *entity) - : entity_(entity), staticProps_(nullptr), supportFlips_(false), - flipsAlterBayerOrder_(false), properties_(properties::properties) + : entity_(entity), staticProps_(nullptr), supportHFlips_(false), + supportVFlips_(false), flipsAlterBayerOrder_(false), + properties_(properties::properties) { } @@ -479,26 +481,31 @@ std::optional CameraSensorRaw::init() return { ret }; } - /* - * Verify if sensor supports horizontal/vertical flips - * - * \todo Handle horizontal and vertical flips independently. - */ - const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP); - const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP); - if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) && - vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { - supportFlips_ = true; + /* Verify if sensor supports horizontal/vertical flips. */ + auto queryFlip = [&](uint32_t id, const char *name) -> std::pair { + const struct v4l2_query_ext_ctrl *flipInfo = subdev_->controlInfo(id); + if (!flipInfo || flipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) { + LOG(CameraSensor, Debug) + << "Camera sensor does not support " << name << " flip"; + return { false, false }; + } - if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT || - vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT) - flipsAlterBayerOrder_ = true; + return { true, flipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT }; + }; + + auto hflip = queryFlip(V4L2_CID_HFLIP, "horizontal"); + auto vflip = queryFlip(V4L2_CID_HFLIP, "vertical"); + + if (hflip.first && vflip.first && (hflip.second != vflip.second)) { + LOG(CameraSensor, Error) + << "Flips with differing alter bayer order properties" + << " are unsupported. Disabling flipping."; + } else { + supportHFlips_ = hflip.first; + supportVFlips_ = vflip.first; + flipsAlterBayerOrder_ = hflip.second || vflip.second; } - if (!supportFlips_) - LOG(CameraSensor, Debug) - << "Camera sensor does not support horizontal/vertical flip"; - /* * 5. Discover ancillary devices. * @@ -819,14 +826,16 @@ CameraSensorRaw::getFormat(Span mbusCodes, int CameraSensorRaw::setFormat(V4L2SubdeviceFormat *format, Transform transform) { /* Configure flips if the sensor supports that. */ - if (supportFlips_) { - ControlList flipCtrls(subdev_->controls()); - + ControlList flipCtrls(subdev_->controls()); + if (supportHFlips_) flipCtrls.set(V4L2_CID_HFLIP, static_cast(!!(transform & Transform::HFlip))); + + if (supportVFlips_) flipCtrls.set(V4L2_CID_VFLIP, static_cast(!!(transform & Transform::VFlip))); + if (!flipCtrls.empty()) { int ret = subdev_->setControls(&flipCtrls); if (ret) return ret; @@ -1054,17 +1063,8 @@ int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const Transform CameraSensorRaw::computeTransform(Orientation *orientation) const { /* - * If we cannot do any flips we cannot change the native camera mounting - * orientation. - */ - if (!supportFlips_) { - *orientation = mountingOrientation_; - return Transform::Identity; - } - - /* - * Now compute the required transform to obtain 'orientation' starting - * from the mounting rotation. + * Compute the required transform to obtain 'orientation' starting from + * the mounting rotation. * * As a note: * orientation / mountingOrientation_ = transform @@ -1072,6 +1072,17 @@ Transform CameraSensorRaw::computeTransform(Orientation *orientation) const */ Transform transform = *orientation / mountingOrientation_; + /* If we cannot do the required flips, fall back to identity. */ + if (!supportHFlips_ && !!(transform & Transform::HFlip)) { + *orientation = mountingOrientation_; + return Transform::Identity; + } + + if (!supportVFlips_ && !!(transform & Transform::VFlip)) { + *orientation = mountingOrientation_; + return Transform::Identity; + } + /* * If transform contains any Transpose we cannot do it, so adjust * 'orientation' to report the image native orientation and return Identity.