Patch Detail
Show a patch.
GET /api/patches/18114/?format=api
{ "id": 18114, "url": "https://patchwork.libcamera.org/api/patches/18114/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18114/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>", "date": "2023-01-14T19:47:10", "name": "[libcamera-devel,v2,3/5] libcamera: camera_sensor: Apply flips at setFormat()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "3f1ea4639b662eeeb7bc4365574ee1faada1caf9", "submitter": { "id": 143, "url": "https://patchwork.libcamera.org/api/people/143/?format=api", "name": "Jacopo Mondi", "email": "jacopo.mondi@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18114/mbox/", "series": [ { "id": 3711, "url": "https://patchwork.libcamera.org/api/series/3711/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3711", "date": "2023-01-14T19:47:07", "name": "libcamera: camera_sensor: Centralize flips handling", "version": 2, "mbox": "https://patchwork.libcamera.org/series/3711/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18114/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18114/checks/", "tags": {}, "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 A9758C3293\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 14 Jan 2023 19:47:31 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54567625EB;\n\tSat, 14 Jan 2023 20:47:29 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C6B4625D0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 14 Jan 2023 20:47:26 +0100 (CET)", "from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 755096CF;\n\tSat, 14 Jan 2023 20:47:25 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673725649;\n\tbh=8sBhE2PAmURrrlB3B3CqN6E5XAEjx6Y/yqzQIH5iXgQ=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=aJoj7wq2kQq6mPCSBeRchzeBR6tFmzg51ruFjT0Yg4HsGXK3JN3SE+zoE9SgR72jf\n\tRylMFoitfajcl12sjwB8Q/4mFcXEB60kLxsPI9AD9f8ydGDBSY+LqUhDkpgrs10EFR\n\tv65ETf/Fy65qBdiqCsmCqJvsBkRgO8w9HYiC7XOvy6fVRiXO1zbeLyuCFYmFbRKhWg\n\tZrLe2i6QH6pg3CQ43SxFAzAKsb7UGIfei0vqCyZ/Fl1QrqgX9269PtNxxFhe35m0uI\n\tgj0biYuQrlV6D+TvxARGmTpxRsVyQgm/wWmVtELKSPElBnf8M+7s7ZDdUN3/b1ydO6\n\tzhGlybl3PWJxg==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1673725645;\n\tbh=8sBhE2PAmURrrlB3B3CqN6E5XAEjx6Y/yqzQIH5iXgQ=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=g6lS0Tm+MeIpHinv4HwiYMKIYr/Txjd4yrgXVTgTbjQ5FqQAjYeAbXwcNVJ2rZgPa\n\tshGiijCc6tx2wLYe2VcvQXuvdJPsxet67hJxkKU7xznEzEniO5D4lEAPorib74Ajrd\n\tvMVO6lOqSTmJKYWUyEtgJ6TfgwTzgHQoLBZYbncc=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"g6lS0Tm+\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 14 Jan 2023 20:47:10 +0100", "Message-Id": "<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.0", "In-Reply-To": "<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>", "References": "<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor: Apply\n\tflips at setFormat()", "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.mondi@ideasonboard.com>", "Cc": "Jacopo Mondi <jacopo.mondi@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Augment the CameraSensor::setFormat() function to configure horizontal\nand vertical flips before applying the image format on the sensor.\n\nApplying flips before format is crucial as they might change the Bayer\npattern ordering.\n\nTo allow users of the CameraSensor class to specify a Transform,\nadd to the V4L2SubdeviceFormat class a 'transform' member, by\ndefault initialized to Transform::Identity.\n\nMoving the handling of H/V flips to the CameraSensor class allows to\nremove quite some boilerplate code from the IPU3 and RaspberryPi\npipeline handlers.\n\nNo functional changes intended.\n\nSigned-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n---\n include/libcamera/internal/v4l2_subdevice.h | 2 ++\n src/libcamera/camera_sensor.cpp | 23 +++++++++++++++\n src/libcamera/pipeline/ipu3/cio2.cpp | 6 +++-\n src/libcamera/pipeline/ipu3/cio2.h | 4 ++-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 28 ++-----------------\n .../pipeline/raspberrypi/raspberrypi.cpp | 27 +++++-------------\n src/libcamera/v4l2_subdevice.cpp | 7 +++++\n 7 files changed, 49 insertions(+), 48 deletions(-)", "diff": "diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\nindex 69862de0585a..576faf971a05 100644\n--- a/include/libcamera/internal/v4l2_subdevice.h\n+++ b/include/libcamera/internal/v4l2_subdevice.h\n@@ -20,6 +20,7 @@\n \n #include <libcamera/color_space.h>\n #include <libcamera/geometry.h>\n+#include <libcamera/transform.h>\n \n #include \"libcamera/internal/formats.h\"\n #include \"libcamera/internal/media_object.h\"\n@@ -44,6 +45,7 @@ struct V4L2SubdeviceFormat {\n \tuint32_t mbus_code;\n \tSize size;\n \tstd::optional<ColorSpace> colorSpace;\n+\tTransform transform = Transform::Identity;\n \n \tconst std::string toString() const;\n \tuint8_t bitsPerPixel() const;\ndiff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 3518d3e3b02a..6d5c2317e0fe 100644\n--- a/src/libcamera/camera_sensor.cpp\n+++ b/src/libcamera/camera_sensor.cpp\n@@ -750,6 +750,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n \t\t.mbus_code = bestCode,\n \t\t.size = *bestSize,\n \t\t.colorSpace = ColorSpace::Raw,\n+\t\t.transform = Transform::Identity,\n \t};\n \n \treturn format;\n@@ -759,12 +760,34 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n * \\brief Set the sensor output format\n * \\param[in] format The desired sensor output format\n *\n+ * If flips are writable they are configured according to the desired Transform.\n+ * Transform::Identity always corresponds to H/V flip being disabled if the\n+ * controls are writable. Flips are set before the new format is applied as\n+ * they can effectively change the Bayer pattern ordering.\n+ *\n * The ranges of any controls associated with the sensor are also updated.\n *\n * \\return 0 on success or a negative error code otherwise\n */\n int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n {\n+\t/* Configure flips if the sensor supports that. */\n+\tif (supportFlips_) {\n+\t\tControlList flipCtrls(subdev_->controls());\n+\n+\t\tflipCtrls.set(V4L2_CID_HFLIP,\n+\t\t\t static_cast<int32_t>(!!(format->transform &\n+\t\t\t\t\t\t Transform::HFlip)));\n+\t\tflipCtrls.set(V4L2_CID_VFLIP,\n+\t\t\t static_cast<int32_t>(!!(format->transform &\n+\t\t\t\t\t\t Transform::VFlip)));\n+\n+\t\tint ret = subdev_->setControls(&flipCtrls);\n+\t\tif (ret)\n+\t\t\treturn ret;\n+\t}\n+\n+\t/* Apply format on the subdev. */\n \tint ret = subdev_->setFormat(pad_, format);\n \tif (ret)\n \t\treturn ret;\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\nindex d4e523af24b4..a819884f762d 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.cpp\n+++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n@@ -15,6 +15,7 @@\n #include <libcamera/formats.h>\n #include <libcamera/geometry.h>\n #include <libcamera/stream.h>\n+#include <libcamera/transform.h>\n \n #include \"libcamera/internal/camera_sensor.h\"\n #include \"libcamera/internal/framebuffer.h\"\n@@ -177,10 +178,12 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n /**\n * \\brief Configure the CIO2 unit\n * \\param[in] size The requested CIO2 output frame size\n+ * \\param[in] transform The transformation to be applied on the image sensor\n * \\param[out] outputFormat The CIO2 unit output image format\n * \\return 0 on success or a negative error code otherwise\n */\n-int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n+int CIO2Device::configure(const Size &size, const Transform &transform,\n+\t\t\t V4L2DeviceFormat *outputFormat)\n {\n \tV4L2SubdeviceFormat sensorFormat;\n \tint ret;\n@@ -191,6 +194,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n \t */\n \tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToPixelFormat);\n \tsensorFormat = getSensorFormat(mbusCodes, size);\n+\tsensorFormat.transform = transform;\n \tret = sensor_->setFormat(&sensorFormat);\n \tif (ret)\n \t\treturn ret;\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\nindex 68504a2da89d..bbd87eb8ceb6 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.h\n+++ b/src/libcamera/pipeline/ipu3/cio2.h\n@@ -26,6 +26,7 @@ class Request;\n class Size;\n class SizeRange;\n struct StreamConfiguration;\n+enum class Transform;\n \n class CIO2Device\n {\n@@ -38,7 +39,8 @@ public:\n \tstd::vector<SizeRange> sizes(const PixelFormat &format) const;\n \n \tint init(const MediaDevice *media, unsigned int index);\n-\tint configure(const Size &size, V4L2DeviceFormat *outputFormat);\n+\tint configure(const Size &size, const Transform &transform,\n+\t\t V4L2DeviceFormat *outputFormat);\n \n \tStreamConfiguration generateConfiguration(Size size) const;\n \ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex a424ac914859..3a569c7e0031 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -51,7 +51,7 @@ class IPU3CameraData : public Camera::Private\n {\n public:\n \tIPU3CameraData(PipelineHandler *pipe)\n-\t\t: Camera::Private(pipe), supportsFlips_(false)\n+\t\t: Camera::Private(pipe)\n \t{\n \t}\n \n@@ -73,7 +73,6 @@ public:\n \tStream rawStream_;\n \n \tRectangle cropRegion_;\n-\tbool supportsFlips_;\n \tTransform rotationTransform_;\n \n \tstd::unique_ptr<DelayedControls> delayedCtrls_;\n@@ -539,7 +538,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n \t */\n \tconst Size &sensorSize = config->cio2Format().size;\n \tV4L2DeviceFormat cio2Format;\n-\tret = cio2->configure(sensorSize, &cio2Format);\n+\tret = cio2->configure(sensorSize, config->combinedTransform_, &cio2Format);\n \tif (ret)\n \t\treturn ret;\n \n@@ -547,24 +546,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n \tcio2->sensor()->sensorInfo(&sensorInfo);\n \tdata->cropRegion_ = sensorInfo.analogCrop;\n \n-\t/*\n-\t * Configure the H/V flip controls based on the combination of\n-\t * the sensor and user transform.\n-\t */\n-\tif (data->supportsFlips_) {\n-\t\tControlList sensorCtrls(cio2->sensor()->controls());\n-\t\tsensorCtrls.set(V4L2_CID_HFLIP,\n-\t\t\t\tstatic_cast<int32_t>(!!(config->combinedTransform_\n-\t\t\t\t\t\t\t& Transform::HFlip)));\n-\t\tsensorCtrls.set(V4L2_CID_VFLIP,\n-\t\t\t\tstatic_cast<int32_t>(!!(config->combinedTransform_\n-\t\t\t\t\t\t & Transform::VFlip)));\n-\n-\t\tret = cio2->sensor()->setControls(&sensorCtrls);\n-\t\tif (ret)\n-\t\t\treturn ret;\n-\t}\n-\n \t/*\n \t * If the ImgU gets configured, its driver seems to expect that\n \t * buffers will be queued to its outputs, as otherwise the next\n@@ -1127,11 +1108,6 @@ int PipelineHandlerIPU3::registerCameras()\n \t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotationValue\n \t\t\t\t\t << \" degrees: ignoring\";\n \n-\t\tControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });\n-\t\tif (!ctrls.empty())\n-\t\t\t/* We assume the sensor supports VFLIP too. */\n-\t\t\tdata->supportsFlips_ = true;\n-\n \t\t/**\n \t\t * \\todo Dynamically assign ImgU and output devices to each\n \t\t * stream and camera; as of now, limit support to two cameras\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex c086a69a913d..d8232ff8e065 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -691,24 +691,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\t}\n \t}\n \n-\t/*\n-\t * Configure the H/V flip controls based on the combination of\n-\t * the sensor and user transform.\n-\t */\n-\tif (data->supportsFlips_) {\n-\t\tconst RPiCameraConfiguration *rpiConfig =\n-\t\t\tstatic_cast<const RPiCameraConfiguration *>(config);\n-\t\tControlList controls;\n-\n-\t\tcontrols.set(V4L2_CID_HFLIP,\n-\t\t\t static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));\n-\t\tcontrols.set(V4L2_CID_VFLIP,\n-\t\t\t static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));\n-\t\tdata->setSensorControls(controls);\n-\t}\n-\n \t/* First calculate the best sensor mode we can use based on the user request. */\n \tV4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n+\t/* Apply any cached transform. */\n+\tconst RPiCameraConfiguration *rpiConfig = static_cast<const RPiCameraConfiguration *>(config);\n+\tsensorFormat.transform = rpiConfig->combinedTransform_;\n+\t/* Finally apply the format on the sensor. */\n \tret = data->sensor_->setFormat(&sensorFormat);\n \tif (ret)\n \t\treturn ret;\n@@ -1293,10 +1281,9 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n \t * We cache three things about the sensor in relation to transforms\n \t * (meaning horizontal and vertical flips).\n \t *\n-\t * Firstly, does it support them?\n-\t * Secondly, if you use them does it affect the Bayer ordering?\n-\t * Thirdly, what is the \"native\" Bayer order, when no transforms are\n-\t * applied?\n+\t * If flips are supported verify if they affect the Bayer ordering\n+\t * and what the \"native\" Bayer order is, when no transforms are\n+\t * applied.\n \t *\n \t * We note that the sensor's cached list of supported formats is\n \t * already in the \"native\" order, with any flips having been undone.\ndiff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\nindex 15e8206a915c..38ff8b0c605b 100644\n--- a/src/libcamera/v4l2_subdevice.cpp\n+++ b/src/libcamera/v4l2_subdevice.cpp\n@@ -216,6 +216,13 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {\n * resulting color space is acceptable.\n */\n \n+/**\n+ * \\var V4L2SubdeviceFormat::transform\n+ * \\brief The transform (vertical/horizontal flips) to be applied on the subdev\n+ *\n+ * Default initialized to Identity (no transform).\n+ */\n+\n /**\n * \\brief Assemble and return a string describing the format\n * \\return A string describing the V4L2SubdeviceFormat\n", "prefixes": [ "libcamera-devel", "v2", "3/5" ] }