[{"id":20156,"web_url":"https://patchwork.libcamera.org/comment/20156/","msgid":"","date":"2021-10-13T00:42:54","subject":"Re: [libcamera-devel] [PATCH v3 01/16] libcamera: ipu3: Use the\n\toptimal sensor size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Oct 11, 2021 at 05:11:39PM +0200, Jacopo Mondi wrote:\n> As reported by commit 7208e70211a6 (\"libcamera: ipu3: Always use sensor\n> full frame size\") the current implementation of the IPU3 pipeline\n> handler always uses the sensor resolution as the ImgU input frame size in\n> order to work around an issue with the ImgU configuration procedure.\n> \n> Now that the frame selection policy has been modified in the CIO2Device\n> class implementation to comply with the requirements of the ImgU\n> configuration script we can remove the workaround and select the most\n> opportune sensor size to feed the ImgU with.\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Paul Elder \n> Reviewed-by: Umang Jain \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 41 ++++++++++++++++------------\n> 1 file changed, 23 insertions(+), 18 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 92e869257e53..5b2ab2ee6825 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -229,7 +229,16 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \t\tstatus = Adjusted;\n> \t}\n> \n> -\t/* Validate the requested stream configuration */\n> +\t/*\n> +\t * Validate the requested stream configuration and select the sensor\n> +\t * format by collecting the maximum RAW stream width and height and\n> +\t * picking the closest larger match.\n> +\t *\n> +\t * If no RAW stream is requested use the one of the largest YUV stream,\n> +\t * plus margin pixels for the IF and BDS rectangle to downscale.\n> +\t *\n> +\t * \\todo Clarify the IF and BDS margins requirements.\n> +\t */\n> \tunsigned int rawCount = 0;\n> \tunsigned int yuvCount = 0;\n> \tSize maxYuvSize;\n> @@ -240,7 +249,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \n> \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> \t\t\trawCount++;\n> -\t\t\trawSize = cfg.size;\n> +\t\t\trawSize.expandTo(cfg.size);\n> \t\t} else {\n> \t\t\tyuvCount++;\n> \t\t\tmaxYuvSize.expandTo(cfg.size);\n> @@ -269,24 +278,20 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \t/*\n> \t * Generate raw configuration from CIO2.\n> \t *\n> -\t * \\todo The image sensor frame size should be selected to optimize\n> -\t * operations based on the sizes of the requested streams. However such\n> -\t * a selection makes the pipeline configuration procedure fail for small\n> -\t * resolutions (for example: 640x480 with OV5670) and causes the capture\n> -\t * operations to stall for some stream size combinations (see the\n> -\t * commit message of the patch that introduced this comment for more\n> -\t * failure examples).\n> +\t * The output YUV streams will be limited in size to the maximum frame\n> +\t * size requested for the RAW stream, if present.\n> \t *\n> -\t * Until the sensor frame size calculation criteria are clarified, when\n> -\t * capturing from ImgU always use the largest possible size which\n> -\t * guarantees better results at the expense of the frame rate and CSI-2\n> -\t * bus bandwidth. When only a raw stream is requested use the requested\n> -\t * size instead, as the ImgU is not involved.\n> +\t * If no raw stream is requested generate a size as large as the maximum\n> +\t * requested YUV size aligned to the ImgU constraints and bound by the\n> +\t * sensor's maximum resolution. See\n> +\t * https://bugs.libcamera.org/show_bug.cgi?id=32\n> \t */\n> -\tif (!yuvCount)\n> -\t\tcio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);\n> -\telse\n> -\t\tcio2Configuration_ = data_->cio2_.generateConfiguration({});\n> +\tif (rawSize.isNull())\n> +\t\trawSize = maxYuvSize.expandedTo({40, 540})\n> +\t\t\t\t .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> +\t\t\t\t\t\t IMGU_OUTPUT_HEIGHT_MARGIN)\n\nI still feel that we should add the margins, not align to them, but have\nno evidence to support this at this point (and to support this, it seems\nwe're missing a Size::growBy() function to add margins).\n\nFeel free to ignore this comment and merge the patch if you think it's\nright.\n\n> +\t\t\t\t .boundedTo(data_->cio2_.sensor()->resolution());\n> +\tcio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);\n> \tif (!cio2Configuration_.pixelFormat.isValid())\n> \t\treturn Invalid;\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 C92EDC323E\n\tfor ;\n\tWed, 13 Oct 2021 00:43:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68D5868F51;\n\tWed, 13 Oct 2021 02:43:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 721ED6012B\n\tfor ;\n\tWed, 13 Oct 2021 02:43:10 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DFAB9E7;\n\tWed, 13 Oct 2021 02:43:09 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kbtV23xG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634085790;\n\tbh=7qkak+O2mEvQRX0vWWolHOiXMt3X/pDbfmYMICG0m3Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kbtV23xGONox7dQAvPhUuHQ+gewByDVP4rnqNxNDv15Vjm+fG8D3liPorTymdVZa5\n\t3Fe2D8svQ2iBzpytg1ZfRWmdHdLJKHFwdLho4YtzZ5szOQnO8LhQ1Oz2N0WUj4jk5g\n\tOCUioKc5wcgaguExEXO61MaifzavDVMRVleiVVJQ=","Date":"Wed, 13 Oct 2021 03:42:54 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20211011151154.72856-1-jacopo@jmondi.org>\n\t<20211011151154.72856-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211011151154.72856-2-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 01/16] libcamera: ipu3: Use the\n\toptimal sensor size","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]