[{"id":25813,"web_url":"https://patchwork.libcamera.org/comment/25813/","msgid":"<20221117103341.hoy33kgbutsijbcv@uno.localdomain>","date":"2022-11-17T10:33:41","subject":"Re: [libcamera-devel] [PATCH 2/6] libcamera: camera_sensor: Do not\n\tclear camera flips when listing formats","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Dave\n\nOn Thu, Nov 10, 2022 at 02:45:52PM +0000, David Plowman via libcamera-devel wrote:\n> Previously the code used to clear the camnera's h and v flip bits when\n> enumerating the supported formats so as to obtain any Bayer formats in\n> the sensor's native (untransformed) orientation. However this fails\n> when the camera is already in use elsewhere.\n>\n> Instead, we query the current state of the flip bits and transform the\n> formats - which we obtain in their flipped orientation - back into\n> their native orientation to be stored.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n\nAs discussed on the RFC, this seems correct to me!\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n> ---\n>  src/libcamera/camera_sensor.cpp | 49 ++++++++++++++++++++++++++-------\n>  1 file changed, 39 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 572a313a..cbac9e78 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -19,6 +19,8 @@\n>\n>  #include <libcamera/base/utils.h>\n>\n> +#include <libcamera/transform.h>\n> +\n>  #include \"libcamera/internal/bayer_format.h\"\n>  #include \"libcamera/internal/camera_lens.h\"\n>  #include \"libcamera/internal/camera_sensor_properties.h\"\n> @@ -108,18 +110,45 @@ int CameraSensor::init()\n>  \t\treturn ret;\n>\n>  \t/*\n> -\t * Clear any flips to be sure we get the \"native\" Bayer order. This is\n> -\t * harmless for sensors where the flips don't affect the Bayer order.\n> +\t * We want to get the native mbus codes for the sensor, without any flips.\n> +\t * We can't clear any flips here, so we have to read the current values\n> +\t * (if the flip controls exist), decide whether they actually modify any\n> +\t * output Bayer pattern, and finally undo their effect on the formats.\n> +\t *\n> +\t * First, check if the flip controls exist and if so read them.\n>  \t */\n> -\tControlList ctrls(subdev_->controls());\n> -\tif (subdev_->controls().find(V4L2_CID_HFLIP) != subdev_->controls().end())\n> -\t\tctrls.set(V4L2_CID_HFLIP, 0);\n> -\tif (subdev_->controls().find(V4L2_CID_VFLIP) != subdev_->controls().end())\n> -\t\tctrls.set(V4L2_CID_VFLIP, 0);\n> -\tsubdev_->setControls(&ctrls);\n> +\tstd::vector<uint32_t> flipCtrlIds;\n> +\tconst struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP);\n> +\tconst struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);\n> +\tif (hflipInfo)\n> +\t\tflipCtrlIds.push_back(V4L2_CID_HFLIP);\n> +\tif (vflipInfo)\n> +\t\tflipCtrlIds.push_back(V4L2_CID_VFLIP);\n> +\tControlList flipCtrls = subdev_->getControls(flipCtrlIds);\n> +\n> +\t/* Now construct a transform that would undo any flips. */\n> +\tTransform transform = Transform::Identity;\n> +\tif (hflipInfo && flipCtrls.get(V4L2_CID_HFLIP).get<int>() &&\n> +\t    (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT))\n> +\t\ttransform |= Transform::HFlip;\n> +\tif (vflipInfo && flipCtrls.get(V4L2_CID_VFLIP).get<int>() &&\n> +\t    (vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT))\n> +\t\ttransform |= Transform::VFlip;\n> +\n> +\t/* Finally get the formats, and apply the transform to the mbus codes. */\n> +\tauto formats = subdev_->formats(pad_);\n> +\tfor (const auto &format : formats) {\n> +\t\tunsigned int mbusCode = format.first;\n> +\t\tBayerFormat bayerFormat = BayerFormat::fromMbusCode(mbusCode);\n> +\n> +\t\tif (bayerFormat.isValid())\n> +\t\t\tmbusCode = bayerFormat.transform(transform).toMbusCode();\n> +\n> +\t\tif (mbusCode)\n> +\t\t\tformats_[mbusCode] = std::move(format.second);\n> +\t}\n>\n>  \t/* Enumerate, sort and cache media bus codes and sizes. */\n> -\tformats_ = subdev_->formats(pad_);\n>  \tif (formats_.empty()) {\n>  \t\tLOG(CameraSensor, Error) << \"No image format found\";\n>  \t\treturn -EINVAL;\n> @@ -189,7 +218,7 @@ int CameraSensor::init()\n>  \t * \\todo The control API ought to have a flag to specify if a control\n>  \t * is read-only which could be used below.\n>  \t */\n> -\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> +\tconst ControlInfo hblank = subdev_->controls().at(V4L2_CID_HBLANK);\n>  \tconst int32_t hblankMin = hblank.min().get<int32_t>();\n>  \tconst int32_t hblankMax = hblank.max().get<int32_t>();\n>\n> --\n> 2.30.2\n>","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 2520BBE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 17 Nov 2022 10:33:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E4A776309B;\n\tThu, 17 Nov 2022 11:33:44 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6652D61F34\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Nov 2022 11:33:43 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id AD244C0012;\n\tThu, 17 Nov 2022 10:33:42 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668681224;\n\tbh=JcVVxwumfVMfxNGo8NY3qml3c9oizEHkW3V9JxV85Vo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=iV8t7ZPYofXzUZUUDRrGAh8n1wqC5tsGLpcRT1j8KVbLrSfiXs6nlvRH8CQGtzYHP\n\tMjcqZQNazDXNKEPUvt4ihB5K1JLYPcxp+zspE7wpic4n8F0vXNGwKEVW5j2/xfvS0r\n\tfzjIyff5+xi93W725V3EJnmyI5MZTH+o6lTuE5mItB9NX7eIZSJfx7n2gNQCBEwDhy\n\tfORdZbwalNp0ukUGGW1WA8KAH3UydKGQ3/RS9aIbgw3lUGkAHpeFqIL39xIommNYNz\n\tr/A6ikvunTtJ8DG9AUrUi9E3iuT3CwTs3qrjgsPDenFnjETqyBnTb017fvBPKXPdZV\n\tLEMCkmtf/vQ4A==","Date":"Thu, 17 Nov 2022 11:33:41 +0100","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20221117103341.hoy33kgbutsijbcv@uno.localdomain>","References":"<20221110144556.7858-1-david.plowman@raspberrypi.com>\n\t<20221110144556.7858-3-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221110144556.7858-3-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 2/6] libcamera: camera_sensor: Do not\n\tclear camera flips when listing formats","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]