From patchwork Tue Jun 20 14:29:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 18753 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 A592ABD78E for ; Tue, 20 Jun 2023 14:29:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4C065628CC; Tue, 20 Jun 2023 16:29:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1687271362; bh=ybjACyq3vQ4lsNMBp29sW93fC5ZrCQ/CfKJ76mC74aE=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=fsfXkODHQL1du4hm8tvSp8watsRseTchckpiwy8GxPFCnsngLuKq4haIqYM68XaVj wfFmfwsZ66Jta+4iancIGEUlyGA8OUUg7QVVP2kaITyvyCGyLc2dyz7FHde1jXfT1S 9A2IHmCbCMEzN9RKyCwjbWydU3ejiSo1HUvEufapetYJH2zWA0HcHVjnrKZVJ30ZyU 6KQFq9wzsMf/iklrR8ZPIE8T0CxEvTq0D9kR9GuVxhZJHtBhjVYUOWFLZUkgelOs7+ 7X7aoqgkFtgYdEmPtbP0DoVVlHLCzHcWchPAg9wn+jJIsIa50B+9ofC7yq7GCxW+Z5 l85HVsTpWjRSw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7495B628BF for ; Tue, 20 Jun 2023 16:29:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="klnvIo2r"; dkim-atps=neutral Received: from uno.lan (unknown [IPv6:2001:b07:5d2e:52c9:72c3:346:a663:c82d]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6FE0D2B3; Tue, 20 Jun 2023 16:28:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1687271324; bh=ybjACyq3vQ4lsNMBp29sW93fC5ZrCQ/CfKJ76mC74aE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=klnvIo2rTOdzjmDs+tfHUj+hkXdx64iGoNoXANoCUy3jr82tooGWY68gQVEcWYRMu Jd77C36aUGZrzYWZwa8cVdc9/3p79/YzyQYgNPkLVZ7n9YaLdZJTVC57aUcc1oYPwG ExlvauFPIoI1dYrYwjSajYhE3DZGhlSfYY/2tUUA= To: libcamera-devel@lists.libcamera.org Date: Tue, 20 Jun 2023 16:29:03 +0200 Message-Id: <20230620142904.72600-4-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230620142904.72600-1-jacopo.mondi@ideasonboard.com> References: <20230620142904.72600-1-jacopo.mondi@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 3/4] libcamera: Move to use CameraConfiguration::orientation 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: , X-Patchwork-Original-From: Jacopo Mondi via libcamera-devel From: Jacopo Mondi Reply-To: Jacopo Mondi Cc: Jacopo Mondi Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Replace the usage of CameraConfiguration::transform with the newly introduced CameraConfiguration::orientation. Rework the CameraSensor::validateTransform(transform) to CameraSensor::computeTransform(orientation), that given the desired image orientation computes the Transform that pipeline handlers should apply to the sensor to obtain it. Introduce in transform.cpp two functions to convert from Transform to Orientation and vice-versa. Port all pipeline handlers to use the newly introduced function. This commit breaks existing applications as it removes the public CameraConfiguration::transform in faviour of CameraConfiguration::orientation. Signed-off-by: Jacopo Mondi --- include/libcamera/camera.h | 2 - include/libcamera/internal/camera_sensor.h | 3 +- include/libcamera/transform.h | 4 + src/libcamera/camera.cpp | 15 +-- src/libcamera/camera_sensor.cpp | 95 +++++++++---------- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +- .../pipeline/rpi/common/pipeline_base.cpp | 9 +- src/libcamera/pipeline/simple/simple.cpp | 6 +- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +- src/libcamera/pipeline/vimc/vimc.cpp | 4 +- src/libcamera/transform.cpp | 58 +++++++++++ 12 files changed, 129 insertions(+), 85 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 608774ce7768..bffac6bc8922 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -20,7 +20,6 @@ #include #include #include -#include namespace libcamera { @@ -76,7 +75,6 @@ public: bool empty() const; std::size_t size() const; - Transform transform; Orientation orientation; protected: diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index 02b4b4d25e6d..1d47a2b1a500 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -71,7 +72,7 @@ public: CameraLens *focusLens() { return focusLens_.get(); } - Transform validateTransform(Transform *transform) const; + Transform computeTransform(CameraConfiguration::Orientation *orientation) const; protected: std::string logPrefix() const override; diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h index 2a6838c78369..573adf18715d 100644 --- a/include/libcamera/transform.h +++ b/include/libcamera/transform.h @@ -9,6 +9,8 @@ #include +#include + namespace libcamera { enum class Transform : int { @@ -69,6 +71,8 @@ constexpr Transform operator~(Transform t) } Transform transformFromRotation(int angle, bool *success = nullptr); +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation); +CameraConfiguration::Orientation transformToOrientation(const Transform &transform); const char *transformToString(Transform t); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 5ca05719ebfc..bc1409b5c960 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -197,7 +197,7 @@ LOG_DECLARE_CATEGORY(Camera) * \brief Create an empty camera configuration */ CameraConfiguration::CameraConfiguration() - : transform(Transform::Identity), orientation(rotate0), config_({}) + : orientation(rotate0), config_({}) { } @@ -428,19 +428,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF return status; } -/** - * \var CameraConfiguration::transform - * \brief User-specified transform to be applied to the image - * - * The transform is a user-specified 2D plane transform that will be applied - * to the camera images by the processing pipeline before being handed to - * the application. - * - * The usual 2D plane transforms are allowed here (horizontal/vertical - * flips, multiple of 90-degree rotations etc.), but the validate() function - * may adjust this field at its discretion if the selection is not supported. - */ - /** * \var CameraConfiguration::orientation * \brief The desired orientation of the video streams produced by the camera diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index a15a6c89c5c8..5c9f30a62527 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -465,7 +465,7 @@ int CameraSensor::initProperties() /* * Cache the Transform associated with the camera mounting - * rotation for later use in validateTransform(). + * rotation for later use in computeTransform(). */ bool success; rotationTransform_ = transformFromRotation(propertyValue, &success); @@ -477,7 +477,7 @@ int CameraSensor::initProperties() } /* - * Adjust property::Rotation as validateTransform() compensates + * Adjust property::Rotation as computeTransform() compensates * for the mounting rotation. However, as a camera sensor can * only compensate rotations by applying H/VFlips, only rotation * of 180 degrees are automatically compensated. The other valid @@ -1033,69 +1033,66 @@ void CameraSensor::updateControlInfo() */ /** - * \brief Validate a transform request against the sensor capabilities - * \param[inout] transform The requested transformation, updated to match - * the sensor capabilities + * \brief Compute the Transform that gives the requested \a orientation + * \param[inout] orientation The desired image orientation * - * The input \a transform is the transform that the caller wants, and it is - * adjusted according to the capabilities of the sensor to represent the - * "nearest" transform that can actually be delivered. + * This function computes the Transform that the pipeline handler should apply + * to the CameraSensor to obtain the requested \a orientation. * - * The returned Transform is the transform applied to the sensor in order to - * produce the input \a transform, It is also validated against the sensor's - * ability to perform horizontal and vertical flips. + * The intended caller of this function is the validate() implementation of + * pipeline handlers, that pass in the application requested + * CameraConfiguration::orientation and obtain a Transform to apply to the + * camera sensor, likely at configure() time. * - * For example, if the requested \a transform is Transform::Identity and the - * sensor rotation is 180 degrees, the output transform will be - * Transform::Rot180 to correct the images so that they appear to have - * Transform::Identity, but only if the sensor can apply horizontal and vertical - * flips. + * If the requested \a orientation cannot be obtained, the \a orientation + * parameter is adjusted to report what the current image orientation is and + * Transform::Identity is returned. * - * \return A Transform instance that represents which transformation has been - * applied to the camera sensor + * If the requested \a orientation can be obtained, the function computes a + * Transform and does not adjust \a orientation. + * + * Pipeline handlers are expected to verify if \a orientation has been + * adjusted by this function and set the CameraConfiguration::status to + * Adjusted accordingly. + * + * \return A Transform instance that applied to the CameraSensor produces images + * with \a orientation */ -Transform CameraSensor::validateTransform(Transform *transform) const +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const { /* - * Combine the requested transform to compensate the sensor mounting - * rotation. + * We cannot do any flips, we cannot change the native camera mounting + * orientation. */ - Transform combined = *transform * rotationTransform_; + if (!supportFlips_) { + *orientation = transformToOrientation(rotationTransform_); + return Transform::Identity; + } /* - * We combine the platform and user transform, but must "adjust away" - * any combined result that includes a transposition, as we can't do - * those. In this case, flipping only the transpose bit is helpful to - * applications - they either get the transform they requested, or have - * to do a simple transpose themselves (they don't have to worry about - * the other possible cases). + * If the camera is mounted 90 or 270 degrees rotated, there is no + * way we can correct it and there's no point in continuing as the + * user request cannot be satisfied in full. */ - if (!!(combined & Transform::Transpose)) { - /* - * Flipping the transpose bit in "transform" flips it in the - * combined result too (as it's the last thing that happens), - * which is of course clearing it. - */ - *transform ^= Transform::Transpose; - combined &= ~Transform::Transpose; + if (!!(rotationTransform_ & Transform::Transpose)) { + *orientation = transformToOrientation(rotationTransform_); + return Transform::Identity; } /* - * We also check if the sensor doesn't do h/vflips at all, in which - * case we clear them, and the application will have to do everything. + * If the user request contains a transform there's no way we can + * satisfy it, default it to Identity. We cannot return early as the + * camera mounting rotation has to be corrected, and if we get here we + * know we can do that (we adjusted property::Rotation already because + * of this). */ - if (!supportFlips_ && !!combined) { - /* - * If the sensor can do no transforms, then combined must be - * changed to the identity. The only user transform that gives - * rise to this is the inverse of the rotation. (Recall that - * combined = transform * rotationTransform.) - */ - *transform = -rotationTransform_; - combined = Transform::Identity; - } + Transform request = transformFromOrientation(*orientation); + if (!!(request & Transform::Transpose)) + request = Transform::Identity; + + *orientation = transformToOrientation(request); - return combined; + return request * rotationTransform_; } std::string CameraSensor::logPrefix() const diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 355cb0cb76b8..ded41e011be2 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() * rotation and store the final combined transform that configure() will * need to apply to the sensor to save us working it out again. */ - Transform requestedTransform = transform; - combinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform); - if (transform != requestedTransform) + Orientation requestedOrientation = orientation; + combinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation); + if (orientation != requestedOrientation) status = Adjusted; /* Cap the number of entries to the available streams. */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 91a3c60757e1..81ae84b13a62 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() status = Adjusted; } - Transform requestedTransform = transform; - Transform combined = sensor->validateTransform(&transform); - if (transform != requestedTransform) + Orientation requestedOrientation = orientation; + combinedTransform_ = data_->sensor_->computeTransform(&orientation); + if (orientation != requestedOrientation) status = Adjusted; /* @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (sensorFormat_.size.isNull()) sensorFormat_.size = sensor->resolution(); - combinedTransform_ = combined; - return status; } diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index df7482920e75..9d6d816f637a 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() * rotation and store the final combined transform that configure() will * need to apply to the sensor to save us working it out again. */ - Transform requestedTransform = transform; - combinedTransform_ = data_->sensor_->validateTransform(&transform); - if (transform != requestedTransform) + Orientation requestedOrientation = orientation; + combinedTransform_ = data_->sensor_->computeTransform(&orientation); + if (orientation != requestedOrientation) status = Adjusted; std::vector rawStreams, outStreams; @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config } /* Always send the user transform to the IPA. */ - params.transform = static_cast(config->transform); + params.transform = + static_cast(transformFromOrientation(config->orientation)); /* Ready the IPA - it must know about the sensor resolution. */ ret = ipa_->configure(sensorInfo_, params, result); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 050285fd389e..cae67c90bd20 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -888,9 +888,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() if (config_.empty()) return Invalid; - Transform requestedTransform = transform; - combinedTransform_ = sensor->validateTransform(&transform); - if (transform != requestedTransform) + Orientation requestedOrientation = orientation; + combinedTransform_ = sensor->computeTransform(&orientation); + if (orientation != requestedOrientation) status = Adjusted; /* Cap the number of entries to the available streams. */ diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 277465b72164..e1f215f06db2 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -111,8 +111,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() if (config_.empty()) return Invalid; - if (transform != Transform::Identity) { - transform = Transform::Identity; + if (orientation != CameraConfiguration::rotate0) { + orientation = CameraConfiguration::rotate0; status = Adjusted; } diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 204f5ad73f6d..33c165d0cee2 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() if (config_.empty()) return Invalid; - if (transform != Transform::Identity) { - transform = Transform::Identity; + if (orientation != CameraConfiguration::rotate0) { + orientation = CameraConfiguration::rotate0; status = Adjusted; } diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp index 4668303d0676..930155b60ff4 100644 --- a/src/libcamera/transform.cpp +++ b/src/libcamera/transform.cpp @@ -299,6 +299,64 @@ Transform transformFromRotation(int angle, bool *success) return Transform::Identity; } +/** + * \brief Return the transform representing \a orientation + * \param[in] orientation The orientation to convert + * \return The transform corresponding to \a orientation + */ +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation) +{ + switch (orientation) { + case CameraConfiguration::rotate0: + return Transform::Identity; + case CameraConfiguration::flipRotate0: + return Transform::HFlip; + case CameraConfiguration::rotate180: + return Transform::Rot180; + case CameraConfiguration::flipRotate180: + return Transform::VFlip; + case CameraConfiguration::flipRotate270: + return Transform::Transpose; + case CameraConfiguration::rotate90: + return Transform::Rot90; + case CameraConfiguration::flipRotate90: + return Transform::Rot180Transpose; + case CameraConfiguration::rotate270: + return Transform::Rot270; + } + + return Transform::Identity; +} + +/** + * \brief Return the CameraConfiguration::Orientation representing \a transform + * \param[in] transform The transform to convert + * \return The Orientation corresponding to \a transform + */ +CameraConfiguration::Orientation transformToOrientation(const Transform &transform) +{ + switch (transform) { + case Transform::Identity: + return CameraConfiguration::rotate0; + case Transform::HFlip: + return CameraConfiguration::flipRotate0; + case Transform::VFlip: + return CameraConfiguration::flipRotate180; + case Transform::Rot180: + return CameraConfiguration::rotate180; + case Transform::Transpose: + return CameraConfiguration::flipRotate270; + case Transform::Rot270: + return CameraConfiguration::rotate270; + case Transform::Rot90: + return CameraConfiguration::rotate90; + case Transform::Rot180Transpose: + return CameraConfiguration::flipRotate90; + } + + return CameraConfiguration::rotate0; +} + /** * \brief Return a character string describing the transform * \param[in] t The transform to be described.