{"id":8533,"url":"https://patchwork.libcamera.org/api/patches/8533/?format=json","web_url":"https://patchwork.libcamera.org/patch/8533/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200701123036.51922-7-jacopo@jmondi.org>","date":"2020-07-01T12:30:27","name":"[libcamera-devel,06/15] libcamera: ipu3: Rework stream validation","commit_ref":null,"pull_url":null,"state":"superseded","archived":true,"hash":"1c1d8a814cacef55ca230da9b92bda3b83d72ee0","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/?format=json","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"delegate":{"id":15,"url":"https://patchwork.libcamera.org/api/users/15/?format=json","username":"jmondi","first_name":"Jacopo","last_name":"Mondi","email":"jacopo@jmondi.org"},"mbox":"https://patchwork.libcamera.org/patch/8533/mbox/","series":[{"id":1066,"url":"https://patchwork.libcamera.org/api/series/1066/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1066","date":"2020-07-01T12:30:21","name":"libcamera: ipu3: Rework streams configuration","version":1,"mbox":"https://patchwork.libcamera.org/series/1066/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/8533/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/8533/checks/","tags":{},"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 2C8A2C2E69\n\tfor ;\n\tWed, 1 Jul 2020 12:27:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE39960CBC;\n\tWed, 1 Jul 2020 14:27:24 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C976F60C5B\n\tfor ;\n\tWed, 1 Jul 2020 14:27:22 +0200 (CEST)","from uno.lan (93-34-118-233.ip49.fastwebnet.it [93.34.118.233])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 6A7A51C0007;\n\tWed, 1 Jul 2020 12:27:22 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","From":"Jacopo Mondi ","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 1 Jul 2020 14:30:27 +0200","Message-Id":"<20200701123036.51922-7-jacopo@jmondi.org>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20200701123036.51922-1-jacopo@jmondi.org>","References":"<20200701123036.51922-1-jacopo@jmondi.org>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH 06/15] libcamera: ipu3: Rework stream\n\tvalidation","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"With the goal of moving stream assignement to configuration from\nvalidate() to configure(), rework the validate() function to not\ninspect the stream assigned to a configuration to identify it.\n\nRe-work the validation logic to follow this simpler flow:\n- if a stream is a raw stream use the sensor configuration\n- if a stream is a processed one, make sure it respects the ImgU\n output size and alignment constraints.\n\nRemove the 'adjustStream()' function as it depends on stream assignment\nand was built on the assumption the main output should always work at\nthe maximum available resolution, and it addressed the case where no\nwidth or height where supplied for a viewfinder stream, which should\nonly be validated against the ImgU output constraints, not defaulted to a\nsane value.\n\nRetain the call to assignStreams() in validate() for the moment in order\nnot to break capture operations, but while at it clean up the code a bit\nby removing a rogue empty line and make a conditional check fit on a\nsingle line.\n\nSigned-off-by: Jacopo Mondi \n---\n src/libcamera/pipeline/ipu3/ipu3.cpp | 85 ++++++++++------------------\n 1 file changed, 31 insertions(+), 54 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex ed2360347fb4..daa6d71dae72 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -68,7 +68,6 @@ public:\n \n private:\n \tvoid assignStreams();\n-\tvoid adjustStream(StreamConfiguration &cfg, bool scale);\n \n \t/*\n \t * The IPU3CameraData instance is guaranteed to be valid as long as the\n@@ -178,52 +177,6 @@ void IPU3CameraConfiguration::assignStreams()\n \t}\n }\n \n-void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)\n-{\n-\t/* The only pixel format the driver supports is NV12. */\n-\tcfg.pixelFormat = formats::NV12;\n-\n-\tif (scale) {\n-\t\t/*\n-\t\t * Provide a suitable default that matches the sensor aspect\n-\t\t * ratio.\n-\t\t */\n-\t\tif (!cfg.size.width || !cfg.size.height) {\n-\t\t\tcfg.size.width = 1280;\n-\t\t\tcfg.size.height = 1280 * cio2Configuration_.size.height\n-\t\t\t\t\t/ cio2Configuration_.size.width;\n-\t\t}\n-\n-\t\t/*\n-\t\t * \\todo: Clamp the size to the hardware bounds when we will\n-\t\t * figure them out.\n-\t\t *\n-\t\t * \\todo: Handle the scaler (BDS) restrictions. The BDS can\n-\t\t * only scale with the same factor in both directions, and the\n-\t\t * scaling factor is limited to a multiple of 1/32. At the\n-\t\t * moment the ImgU driver hides these constraints by applying\n-\t\t * additional cropping, this should be fixed on the driver\n-\t\t * side, and cropping should be exposed to us.\n-\t\t */\n-\t} else {\n-\t\t/*\n-\t\t * \\todo: Properly support cropping when the ImgU driver\n-\t\t * interface will be cleaned up.\n-\t\t */\n-\t\tcfg.size = cio2Configuration_.size;\n-\t}\n-\n-\t/*\n-\t * Clamp the size to match the ImgU alignment constraints. The width\n-\t * shall be a multiple of 8 pixels and the height a multiple of 4\n-\t * pixels.\n-\t */\n-\tif (cfg.size.width % 8 || cfg.size.height % 4) {\n-\t\tcfg.size.width &= ~7;\n-\t\tcfg.size.height &= ~3;\n-\t}\n-}\n-\n CameraConfiguration::Status IPU3CameraConfiguration::validate()\n {\n \tStatus status = Valid;\n@@ -244,7 +197,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n \t * resolution is large enough, pick the largest one.\n \t */\n \tSize size = {};\n-\n \tfor (const StreamConfiguration &cfg : config_) {\n \t\tif (cfg.size.width > size.width)\n \t\t\tsize.width = cfg.size.width;\n@@ -264,20 +216,45 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n \t\tStreamConfiguration &cfg = config_[i];\n \t\tconst StreamConfiguration oldCfg = cfg;\n-\t\tconst Stream *stream = streams_[i];\n+\t\tconst PixelFormatInfo &info =\n+\t\t\tPixelFormatInfo::info(cfg.pixelFormat);\n \n-\t\tif (stream == &data_->rawStream_) {\n+\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n \t\t\tcfg.size = cio2Configuration_.size;\n \t\t\tcfg.pixelFormat = cio2Configuration_.pixelFormat;\n \t\t\tcfg.bufferCount = cio2Configuration_.bufferCount;\n \t\t} else {\n-\t\t\tbool scale = stream == &data_->vfStream_;\n-\t\t\tadjustStream(config_[i], scale);\n+\t\t\t/*\n+\t\t\t * Clamp the size to match the ImgU alignment\n+\t\t\t * constraints.\n+\t\t\t */\n+\t\t\tsize.width = std::min(cfg.size.width,\n+\t\t\t\t\t IPU3_OUTPUT_MAX_WIDTH);\n+\t\t\tsize.height = std::min(cfg.size.height,\n+\t\t\t\t\t IPU3_OUTPUT_MAX_HEIGHT);\n+\t\t\tsize.width = std::max(cfg.size.width,\n+\t\t\t\t\t minIPU3OutputSize.width);\n+\t\t\tsize.height = std::max(cfg.size.height,\n+\t\t\t\t\t minIPU3OutputSize.height);\n+\t\t\tif (cfg.size.width % 8 || cfg.size.height % 4) {\n+\t\t\t\tcfg.size.width &= ~7;\n+\t\t\t\tcfg.size.height &= ~3;\n+\t\t\t}\n+\t\t\tcfg.pixelFormat = formats::NV12;\n \t\t\tcfg.bufferCount = IPU3_BUFFER_COUNT;\n+\n+\t\t\t/*\n+\t\t\t * \\todo: Handle the scaler (BDS) restrictions. The BDS\n+\t\t\t * can only scale with the same factor in both\n+\t\t\t * directions, and the scaling factor is limited to a\n+\t\t\t * multiple of 1/32. At the moment the ImgU driver hides\n+\t\t\t * these constraints by applying additional cropping,\n+\t\t\t * this should be fixed on the driver side, and cropping\n+\t\t\t * should be exposed to us.\n+\t\t\t */\n \t\t}\n \n-\t\tif (cfg.pixelFormat != oldCfg.pixelFormat ||\n-\t\t cfg.size != oldCfg.size) {\n+\t\tif (cfg.pixelFormat != oldCfg.pixelFormat || cfg.size != oldCfg.size) {\n \t\t\tLOG(IPU3, Debug)\n \t\t\t\t<< \"Stream \" << i << \" configuration adjusted to \"\n \t\t\t\t<< cfg.toString();\n","prefixes":["libcamera-devel","06/15"]}