[{"id":26235,"web_url":"https://patchwork.libcamera.org/comment/26235/","msgid":"","date":"2023-01-16T14:56:53","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor:\n\tApply flips at setFormat()","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the patch!\n\nOn Sat, 14 Jan 2023 at 19:47, Jacopo Mondi\n wrote:\n>\n> Augment the CameraSensor::setFormat() function to configure horizontal\n> and vertical flips before applying the image format on the sensor.\n>\n> Applying flips before format is crucial as they might change the Bayer\n> pattern ordering.\n>\n> To allow users of the CameraSensor class to specify a Transform,\n> add to the V4L2SubdeviceFormat class a 'transform' member, by\n> default initialized to Transform::Identity.\n>\n> Moving the handling of H/V flips to the CameraSensor class allows to\n> remove quite some boilerplate code from the IPU3 and RaspberryPi\n> pipeline handlers.\n>\n> No functional changes intended.\n>\n> Signed-off-by: Jacopo Mondi \n\nLGTM!\n\nReviewed-by: David Plowman \n\nNow all we need to do is solve the problem of how to get the native\nBayer order for the sensor (without resetting the flips). On the Pi we\nneed this so that validate() can figure out the correct Bayer order\nfor the format/transform being validated. Presumably IPU3/RKISP1 will\nhave this problem too - I think when I looked at that code a little\nwhile back there seemed to be no attempt to handle it.\n\nThanks!\nDavid\n\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(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 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 \n> #include \n> +#include \n>\n> #include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/media_object.h\"\n> @@ -44,6 +45,7 @@ struct V4L2SubdeviceFormat {\n> uint32_t mbus_code;\n> Size size;\n> std::optional colorSpace;\n> + Transform transform = Transform::Identity;\n>\n> const std::string toString() const;\n> uint8_t bitsPerPixel() const;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 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 &mbu\n> .mbus_code = bestCode,\n> .size = *bestSize,\n> .colorSpace = ColorSpace::Raw,\n> + .transform = Transform::Identity,\n> };\n>\n> return format;\n> @@ -759,12 +760,34 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector &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> + /* Configure flips if the sensor supports that. */\n> + if (supportFlips_) {\n> + ControlList flipCtrls(subdev_->controls());\n> +\n> + flipCtrls.set(V4L2_CID_HFLIP,\n> + static_cast(!!(format->transform &\n> + Transform::HFlip)));\n> + flipCtrls.set(V4L2_CID_VFLIP,\n> + static_cast(!!(format->transform &\n> + Transform::VFlip)));\n> +\n> + int ret = subdev_->setControls(&flipCtrls);\n> + if (ret)\n> + return ret;\n> + }\n> +\n> + /* Apply format on the subdev. */\n> int ret = subdev_->setFormat(pad_, format);\n> if (ret)\n> return ret;\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 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 \n> #include \n> #include \n> +#include \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> + V4L2DeviceFormat *outputFormat)\n> {\n> V4L2SubdeviceFormat sensorFormat;\n> int ret;\n> @@ -191,6 +194,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> */\n> std::vector mbusCodes = utils::map_keys(mbusCodesToPixelFormat);\n> sensorFormat = getSensorFormat(mbusCodes, size);\n> + sensorFormat.transform = transform;\n> ret = sensor_->setFormat(&sensorFormat);\n> if (ret)\n> return ret;\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> index 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> std::vector sizes(const PixelFormat &format) const;\n>\n> int init(const MediaDevice *media, unsigned int index);\n> - int configure(const Size &size, V4L2DeviceFormat *outputFormat);\n> + int configure(const Size &size, const Transform &transform,\n> + V4L2DeviceFormat *outputFormat);\n>\n> StreamConfiguration generateConfiguration(Size size) const;\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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> IPU3CameraData(PipelineHandler *pipe)\n> - : Camera::Private(pipe), supportsFlips_(false)\n> + : Camera::Private(pipe)\n> {\n> }\n>\n> @@ -73,7 +73,6 @@ public:\n> Stream rawStream_;\n>\n> Rectangle cropRegion_;\n> - bool supportsFlips_;\n> Transform rotationTransform_;\n>\n> std::unique_ptr delayedCtrls_;\n> @@ -539,7 +538,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> */\n> const Size &sensorSize = config->cio2Format().size;\n> V4L2DeviceFormat cio2Format;\n> - ret = cio2->configure(sensorSize, &cio2Format);\n> + ret = cio2->configure(sensorSize, config->combinedTransform_, &cio2Format);\n> if (ret)\n> return ret;\n>\n> @@ -547,24 +546,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> cio2->sensor()->sensorInfo(&sensorInfo);\n> data->cropRegion_ = sensorInfo.analogCrop;\n>\n> - /*\n> - * Configure the H/V flip controls based on the combination of\n> - * the sensor and user transform.\n> - */\n> - if (data->supportsFlips_) {\n> - ControlList sensorCtrls(cio2->sensor()->controls());\n> - sensorCtrls.set(V4L2_CID_HFLIP,\n> - static_cast(!!(config->combinedTransform_\n> - & Transform::HFlip)));\n> - sensorCtrls.set(V4L2_CID_VFLIP,\n> - static_cast(!!(config->combinedTransform_\n> - & Transform::VFlip)));\n> -\n> - ret = cio2->sensor()->setControls(&sensorCtrls);\n> - if (ret)\n> - return ret;\n> - }\n> -\n> /*\n> * If the ImgU gets configured, its driver seems to expect that\n> * buffers will be queued to its outputs, as otherwise the next\n> @@ -1127,11 +1108,6 @@ int PipelineHandlerIPU3::registerCameras()\n> LOG(IPU3, Warning) << \"Invalid rotation of \" << rotationValue\n> << \" degrees: ignoring\";\n>\n> - ControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });\n> - if (!ctrls.empty())\n> - /* We assume the sensor supports VFLIP too. */\n> - data->supportsFlips_ = true;\n> -\n> /**\n> * \\todo Dynamically assign ImgU and output devices to each\n> * stream and camera; as of now, limit support to two cameras\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 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> }\n> }\n>\n> - /*\n> - * Configure the H/V flip controls based on the combination of\n> - * the sensor and user transform.\n> - */\n> - if (data->supportsFlips_) {\n> - const RPiCameraConfiguration *rpiConfig =\n> - static_cast(config);\n> - ControlList controls;\n> -\n> - controls.set(V4L2_CID_HFLIP,\n> - static_cast(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));\n> - controls.set(V4L2_CID_VFLIP,\n> - static_cast(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));\n> - data->setSensorControls(controls);\n> - }\n> -\n> /* First calculate the best sensor mode we can use based on the user request. */\n> V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> + /* Apply any cached transform. */\n> + const RPiCameraConfiguration *rpiConfig = static_cast(config);\n> + sensorFormat.transform = rpiConfig->combinedTransform_;\n> + /* Finally apply the format on the sensor. */\n> ret = data->sensor_->setFormat(&sensorFormat);\n> if (ret)\n> return ret;\n> @@ -1293,10 +1281,9 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> * We cache three things about the sensor in relation to transforms\n> * (meaning horizontal and vertical flips).\n> *\n> - * Firstly, does it support them?\n> - * Secondly, if you use them does it affect the Bayer ordering?\n> - * Thirdly, what is the \"native\" Bayer order, when no transforms are\n> - * applied?\n> + * If flips are supported verify if they affect the Bayer ordering\n> + * and what the \"native\" Bayer order is, when no transforms are\n> + * applied.\n> *\n> * We note that the sensor's cached list of supported formats is\n> * already in the \"native\" order, with any flips having been undone.\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 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 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> --\n> 2.39.0\n>","headers":{"Return-Path":"","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 EFE01BD87C\n\tfor ;\n\tMon, 16 Jan 2023 14:57:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 563DC625D8;\n\tMon, 16 Jan 2023 15:57:07 +0100 (CET)","from mail-oo1-xc2c.google.com (mail-oo1-xc2c.google.com\n\t[IPv6:2607:f8b0:4864:20::c2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B020D61F01\n\tfor ;\n\tMon, 16 Jan 2023 15:57:05 +0100 (CET)","by mail-oo1-xc2c.google.com with SMTP id\n\tu13-20020a4aa34d000000b004f5219f9424so371420ool.5\n\tfor ;\n\tMon, 16 Jan 2023 06:57:05 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673881027;\n\tbh=p4JEF9VpoTbiQxXhAjn/ZBMfVgsZser24wvUUM1NaJ8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=uc655b0opqUPhXWdfNnT2tOrov5B3c+h7eUcCKN6F6g3fAOEsGE2vjdQPx4W3trzY\n\tSAqriirM0bvFxRNvmymhlzpOmazUdmOwAhG5N7yuO2wDG1tCnLrj0mBrmkwRp6ST+J\n\tWqzZEpmyX8OpkSh7bNHh+YzCj3g+Yc2NLPATqRTGUM+yJl0Lb10IWrZR/RkmI3OuL+\n\tNXAseALzxUJ9blA/Q1kRMX7ZA8dKjpbdT2oLJR4XOU1llwAyQdkfB67T4NA7a5HYiu\n\t+VWw9vxNevKG27aK4wnoMOonXQ/VWo4dI2wtZAmBmxWrj5sZJ7EW8dd25uYgV0EUWB\n\tIkvhP5TUhttOA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ss4Dp1aHwEvuthFafy2Ty56GxknyQzwBEJ/rzOUOLjs=;\n\tb=tqQzXAnVgV1rblOHjTgnG3hNVuoU7H2SbXwfN/2f2At/p8EPK++7kYFWsW4+NRPwba\n\t14DdZZ1VbzgU5QYjD7Hm2N1WKnaWX2to1Ss4nQYuoLWS487I7WGppWBd8kL1eI/je74v\n\tj1dtdyCj3K/kP8AdjLtFo4xsrWLW+bCv0u/yp11sBsvfbRensEu4rJANbn2ZFKDCVy07\n\tY4oJc8koYx6tG0UgKY9rPobcoaNLNmRBwXw3pgMp0bTqanfEk2bhdsaELfvAkiZEfkGW\n\tz/ioTXuJvL/iV8F04mjab1gbmiMXiCDGror5qfaDn/oHfEKYPHUWfKa1sNfCIbTXYIDF\n\t9CSg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"tqQzXAnV\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=ss4Dp1aHwEvuthFafy2Ty56GxknyQzwBEJ/rzOUOLjs=;\n\tb=XfEtfB4JsvI12ADfjcEokQDvznj8TpI9XDHFwF1xKbSiakqgoK/KkUsa5SSV06+bQh\n\tmX+S1PPsYHPvSQB6C6xY6dEqviRXIJecHqgQiEUb/gYMzBC3rPE3UwvTnagQpBUS2HQO\n\tB1XSsvtyjRs0drDn7/FQbyfQc44kb6NgT1JmR3RtaDYiqbba8EazU8/ga66rQyTKIi0m\n\tLDZJb8YookrzmZ2tCYzJT0EHKTJ+hsX01G2DIb6JKSlUXe5B36Bn1vdWotpGLvIb6veh\n\t6LRE3E279EU8GmsnYEZPzk0rCVSIMt30XTfdsIpHgesgPp/eSV3cNHqPH6Af37y8QmgT\n\tGvJw==","X-Gm-Message-State":"AFqh2kqGWD7xML0Fcekw9+NHQwaJ62s7gB93Hvm8+HZcz8xd2KhvtGIq\n\tLEYXHTSHZgCYGxqPq8yCnLwpzoe0xLhgy4QkBAa1dQ==","X-Google-Smtp-Source":"AMrXdXvMDyT1v9PA/2j1pMjSHdnoByQhvDciuovyswnqb09fMnnmOmCkUf89tt8aKLHP/bUKKm3qIcqRdriORYCvBS0=","X-Received":"by 2002:a4a:bb1a:0:b0:4f2:1946:159f with SMTP id\n\tf26-20020a4abb1a000000b004f21946159fmr975707oop.32.1673881024233;\n\tMon, 16 Jan 2023 06:57:04 -0800 (PST)","MIME-Version":"1.0","References":"<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>\n\t<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>","Date":"Mon, 16 Jan 2023 14:56:53 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor:\n\tApply flips at setFormat()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26237,"web_url":"https://patchwork.libcamera.org/comment/26237/","msgid":"<20230116154200.vm5dfo3z4q4vgetz@uno.localdomain>","date":"2023-01-16T15:42:00","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor:\n\tApply flips at setFormat()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi David,\n\nOn Mon, Jan 16, 2023 at 02:56:53PM +0000, David Plowman via libcamera-devel wrote:\n> Hi Jacopo\n>\n> Thanks for the patch!\n>\n> On Sat, 14 Jan 2023 at 19:47, Jacopo Mondi\n> wrote:\n> >\n> > Augment the CameraSensor::setFormat() function to configure horizontal\n> > and vertical flips before applying the image format on the sensor.\n> >\n> > Applying flips before format is crucial as they might change the Bayer\n> > pattern ordering.\n> >\n> > To allow users of the CameraSensor class to specify a Transform,\n> > add to the V4L2SubdeviceFormat class a 'transform' member, by\n> > default initialized to Transform::Identity.\n> >\n> > Moving the handling of H/V flips to the CameraSensor class allows to\n> > remove quite some boilerplate code from the IPU3 and RaspberryPi\n> > pipeline handlers.\n> >\n> > No functional changes intended.\n> >\n> > Signed-off-by: Jacopo Mondi \n>\n> LGTM!\n>\n> Reviewed-by: David Plowman \n>\n\nThanks\n\n> Now all we need to do is solve the problem of how to get the native\n> Bayer order for the sensor (without resetting the flips). On the Pi we\n> need this so that validate() can figure out the correct Bayer order\n> for the format/transform being validated. Presumably IPU3/RKISP1 will\n> have this problem too - I think when I looked at that code a little\n> while back there seemed to be no attempt to handle it.\n\nIt's a problem for all pipelines but you're the only one that tryed to\nhandle it.\n\nLet's have this series in soon and iterate on top !\n\n>\n> Thanks!\n> David\n>\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(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index 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 \n> > #include \n> > +#include \n> >\n> > #include \"libcamera/internal/formats.h\"\n> > #include \"libcamera/internal/media_object.h\"\n> > @@ -44,6 +45,7 @@ struct V4L2SubdeviceFormat {\n> > uint32_t mbus_code;\n> > Size size;\n> > std::optional colorSpace;\n> > + Transform transform = Transform::Identity;\n> >\n> > const std::string toString() const;\n> > uint8_t bitsPerPixel() const;\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 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 &mbu\n> > .mbus_code = bestCode,\n> > .size = *bestSize,\n> > .colorSpace = ColorSpace::Raw,\n> > + .transform = Transform::Identity,\n> > };\n> >\n> > return format;\n> > @@ -759,12 +760,34 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector &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> > + /* Configure flips if the sensor supports that. */\n> > + if (supportFlips_) {\n> > + ControlList flipCtrls(subdev_->controls());\n> > +\n> > + flipCtrls.set(V4L2_CID_HFLIP,\n> > + static_cast(!!(format->transform &\n> > + Transform::HFlip)));\n> > + flipCtrls.set(V4L2_CID_VFLIP,\n> > + static_cast(!!(format->transform &\n> > + Transform::VFlip)));\n> > +\n> > + int ret = subdev_->setControls(&flipCtrls);\n> > + if (ret)\n> > + return ret;\n> > + }\n> > +\n> > + /* Apply format on the subdev. */\n> > int ret = subdev_->setFormat(pad_, format);\n> > if (ret)\n> > return ret;\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index 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 \n> > #include \n> > #include \n> > +#include \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> > + V4L2DeviceFormat *outputFormat)\n> > {\n> > V4L2SubdeviceFormat sensorFormat;\n> > int ret;\n> > @@ -191,6 +194,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > */\n> > std::vector mbusCodes = utils::map_keys(mbusCodesToPixelFormat);\n> > sensorFormat = getSensorFormat(mbusCodes, size);\n> > + sensorFormat.transform = transform;\n> > ret = sensor_->setFormat(&sensorFormat);\n> > if (ret)\n> > return ret;\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> > index 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> > std::vector sizes(const PixelFormat &format) const;\n> >\n> > int init(const MediaDevice *media, unsigned int index);\n> > - int configure(const Size &size, V4L2DeviceFormat *outputFormat);\n> > + int configure(const Size &size, const Transform &transform,\n> > + V4L2DeviceFormat *outputFormat);\n> >\n> > StreamConfiguration generateConfiguration(Size size) const;\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 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> > IPU3CameraData(PipelineHandler *pipe)\n> > - : Camera::Private(pipe), supportsFlips_(false)\n> > + : Camera::Private(pipe)\n> > {\n> > }\n> >\n> > @@ -73,7 +73,6 @@ public:\n> > Stream rawStream_;\n> >\n> > Rectangle cropRegion_;\n> > - bool supportsFlips_;\n> > Transform rotationTransform_;\n> >\n> > std::unique_ptr delayedCtrls_;\n> > @@ -539,7 +538,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > */\n> > const Size &sensorSize = config->cio2Format().size;\n> > V4L2DeviceFormat cio2Format;\n> > - ret = cio2->configure(sensorSize, &cio2Format);\n> > + ret = cio2->configure(sensorSize, config->combinedTransform_, &cio2Format);\n> > if (ret)\n> > return ret;\n> >\n> > @@ -547,24 +546,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > cio2->sensor()->sensorInfo(&sensorInfo);\n> > data->cropRegion_ = sensorInfo.analogCrop;\n> >\n> > - /*\n> > - * Configure the H/V flip controls based on the combination of\n> > - * the sensor and user transform.\n> > - */\n> > - if (data->supportsFlips_) {\n> > - ControlList sensorCtrls(cio2->sensor()->controls());\n> > - sensorCtrls.set(V4L2_CID_HFLIP,\n> > - static_cast(!!(config->combinedTransform_\n> > - & Transform::HFlip)));\n> > - sensorCtrls.set(V4L2_CID_VFLIP,\n> > - static_cast(!!(config->combinedTransform_\n> > - & Transform::VFlip)));\n> > -\n> > - ret = cio2->sensor()->setControls(&sensorCtrls);\n> > - if (ret)\n> > - return ret;\n> > - }\n> > -\n> > /*\n> > * If the ImgU gets configured, its driver seems to expect that\n> > * buffers will be queued to its outputs, as otherwise the next\n> > @@ -1127,11 +1108,6 @@ int PipelineHandlerIPU3::registerCameras()\n> > LOG(IPU3, Warning) << \"Invalid rotation of \" << rotationValue\n> > << \" degrees: ignoring\";\n> >\n> > - ControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });\n> > - if (!ctrls.empty())\n> > - /* We assume the sensor supports VFLIP too. */\n> > - data->supportsFlips_ = true;\n> > -\n> > /**\n> > * \\todo Dynamically assign ImgU and output devices to each\n> > * stream and camera; as of now, limit support to two cameras\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 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> > }\n> > }\n> >\n> > - /*\n> > - * Configure the H/V flip controls based on the combination of\n> > - * the sensor and user transform.\n> > - */\n> > - if (data->supportsFlips_) {\n> > - const RPiCameraConfiguration *rpiConfig =\n> > - static_cast(config);\n> > - ControlList controls;\n> > -\n> > - controls.set(V4L2_CID_HFLIP,\n> > - static_cast(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));\n> > - controls.set(V4L2_CID_VFLIP,\n> > - static_cast(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));\n> > - data->setSensorControls(controls);\n> > - }\n> > -\n> > /* First calculate the best sensor mode we can use based on the user request. */\n> > V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> > + /* Apply any cached transform. */\n> > + const RPiCameraConfiguration *rpiConfig = static_cast(config);\n> > + sensorFormat.transform = rpiConfig->combinedTransform_;\n> > + /* Finally apply the format on the sensor. */\n> > ret = data->sensor_->setFormat(&sensorFormat);\n> > if (ret)\n> > return ret;\n> > @@ -1293,10 +1281,9 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> > * We cache three things about the sensor in relation to transforms\n> > * (meaning horizontal and vertical flips).\n> > *\n> > - * Firstly, does it support them?\n> > - * Secondly, if you use them does it affect the Bayer ordering?\n> > - * Thirdly, what is the \"native\" Bayer order, when no transforms are\n> > - * applied?\n> > + * If flips are supported verify if they affect the Bayer ordering\n> > + * and what the \"native\" Bayer order is, when no transforms are\n> > + * applied.\n> > *\n> > * We note that the sensor's cached list of supported formats is\n> > * already in the \"native\" order, with any flips having been undone.\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 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 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> > --\n> > 2.39.0\n> >","headers":{"Return-Path":"","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 ADD61C3240\n\tfor ;\n\tMon, 16 Jan 2023 15:42:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 022D161F01;\n\tMon, 16 Jan 2023 16:42:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B426061F01\n\tfor ;\n\tMon, 16 Jan 2023 16:42:04 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 21496802;\n\tMon, 16 Jan 2023 16:42:04 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673883727;\n\tbh=Bo3fAbseNAgqseXUVUxw7CLdzrzSEo0Nynf9VXrttHw=;\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=XoLMpea32yyYGyzl5XVdv0p5qp4u3L2U8kzhD9gMXrTnT49eIVJqbtiq6wKfxvgsC\n\tJ4ERkKn18xQ7byW1cZuQkIQ+tWur+8GRTU5fv3uXgjGWKl1LKwTucrZ6tAV5JoU5UT\n\tDCtN+Jzkk/e13a35xjWtzOtNB1PzMnOnjw78OycJFk0vWmocKWnxDS73l/cq73otC4\n\t6QJu1j50UnxSP6fdHVq+Px1UfyKyWsiF4NLRjUdODlHuWQekb5LYfAEy0dGHRihwMR\n\tAetYUkR5Jj1OgyoPnBNp944myLzMzoQMzv0lN5PzXFrHBeGOoxmCJBBj8/HBv3hHw/\n\tQm1DBwkzXObWw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1673883724;\n\tbh=Bo3fAbseNAgqseXUVUxw7CLdzrzSEo0Nynf9VXrttHw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WkJnnlKpxeCNYV300iy1vpdE2Hlv0A5FoK3BooI7o25gdx23n5RTYX8djsPq7QGGP\n\tZoCsmW9O+WHakZNmwc8K/4XZoq3hB2uXyp6mW2SIYAagL5SSojOr7C6Up/3PzudNQP\n\t3KYCoJQUUPWv6u5jL2HKoageu/DpY93UB8hdmEn4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WkJnnlKp\"; dkim-atps=neutral","Date":"Mon, 16 Jan 2023 16:42:00 +0100","To":"David Plowman ","Message-ID":"<20230116154200.vm5dfo3z4q4vgetz@uno.localdomain>","References":"<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>\n\t<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor:\n\tApply flips at setFormat()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26409,"web_url":"https://patchwork.libcamera.org/comment/26409/","msgid":"","date":"2023-02-06T16:53:02","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor:\n\tApply flips at setFormat()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sat, Jan 14, 2023 at 08:47:10PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> Augment the CameraSensor::setFormat() function to configure horizontal\n> and vertical flips before applying the image format on the sensor.\n\nGood idea :-)\n\n> Applying flips before format is crucial as they might change the Bayer\n> pattern ordering.\n> \n> To allow users of the CameraSensor class to specify a Transform,\n> add to the V4L2SubdeviceFormat class a 'transform' member, by\n> default initialized to Transform::Identity.\n\nNot so good idea :-( You're breaking abstraction, V4L2SubdeviceFormat\nshould match the kernel API.\n\nAs this has been merged already, we can fix it on top.\n\n> Moving the handling of H/V flips to the CameraSensor class allows to\n> remove quite some boilerplate code from the IPU3 and RaspberryPi\n> pipeline handlers.\n> \n> No functional changes intended.\n> \n> Signed-off-by: Jacopo Mondi \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(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 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 \n> #include \n> +#include \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;\n> +\tTransform transform = Transform::Identity;\n> \n> \tconst std::string toString() const;\n> \tuint8_t bitsPerPixel() const;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 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 &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 &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(!!(format->transform &\n> +\t\t\t\t\t\t Transform::HFlip)));\n> +\t\tflipCtrls.set(V4L2_CID_VFLIP,\n> +\t\t\t static_cast(!!(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;\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 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 \n> #include \n> #include \n> +#include \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 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;\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> index 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 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> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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 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(!!(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(!!(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\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 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(config);\n> -\t\tControlList controls;\n> -\n> -\t\tcontrols.set(V4L2_CID_HFLIP,\n> -\t\t\t static_cast(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));\n> -\t\tcontrols.set(V4L2_CID_VFLIP,\n> -\t\t\t static_cast(!!(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(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.\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 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 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","headers":{"Return-Path":"","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 7EE4EBDB1C\n\tfor ;\n\tMon, 6 Feb 2023 16:53:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA83B61EF4;\n\tMon, 6 Feb 2023 17:53:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94F89603B8\n\tfor ;\n\tMon, 6 Feb 2023 17:53:04 +0100 (CET)","from pendragon.ideasonboard.com (unknown [109.136.43.56])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0BBED4DA;\n\tMon, 6 Feb 2023 17:53:03 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675702386;\n\tbh=sd0+04/QcJm1RKxfqHfIiBomEnzBLq43HAHWcd1PmpU=;\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=1EM1XxCO13F2f60t/GWDI8oMKhkVdQ7ho8OR1HONdJMPur32hTm1NOX8i3+nzzuqT\n\tavWalsLo+7bviC9pNdFWaEvvwPSalbH/4aTAqBWNs/+fiWnglYCihRT7DeocE8fKJn\n\tEVtgFdHNot6rI9LPXFYJ872dshpnM3083dzC1/WVHlXNQAFMOOwt9Tpdkp0yvIcmbx\n\tiCXJ/h0o9fAkzdb4lC8HEpxKDjmG+3oJdoJohUJtLBaW/pEuB46bZPo4nzbfzYud+U\n\tWA0cVIVB/QzKc4rf3/zm8NFgrrsPlQIZyoPx9n13OJnbMwBTSqisqrkVT1i4Ft+zfe\n\t7+lAEKIfzOlmA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675702384;\n\tbh=sd0+04/QcJm1RKxfqHfIiBomEnzBLq43HAHWcd1PmpU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EhkV/ovs2Tt8Zz3qDw43jjtFswnOnDR9ZgL3jDKFCjdtzVcFz0QdQq8JbuHaxWIYy\n\tNy8531byZE8Wbj+LhW7Ub3RVt1zlY35iTliKIGKDwzxXgVcL2qT9UpmQ2rtOofvLyb\n\tqpSzyEmjXQ3WQcdEeneHg/xJtPlXEn9Cg0TYpna0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EhkV/ovs\"; dkim-atps=neutral","Date":"Mon, 6 Feb 2023 18:53:02 +0200","To":"Jacopo Mondi ","Message-ID":"","References":"<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>\n\t<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230114194712.23272-4-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: camera_sensor:\n\tApply flips at setFormat()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]