[{"id":20226,"web_url":"https://patchwork.libcamera.org/comment/20226/","msgid":"","date":"2021-10-15T02:34:25","subject":"Re: [libcamera-devel] [PATCH v4 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":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Oct 14, 2021 at 07:41:53PM +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\nReviewed-by: Laurent Pinchart \n\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 262b9a23703e..8aab5041b3ea 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 .grownBy({ IMGU_OUTPUT_WIDTH_MARGIN,\n> +\t\t\t\t\t IMGU_OUTPUT_HEIGHT_MARGIN })\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 C33C5C323E\n\tfor ;\n\tFri, 15 Oct 2021 02:34:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A80868F50;\n\tFri, 15 Oct 2021 04:34:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BCF260239\n\tfor ;\n\tFri, 15 Oct 2021 04:34:41 +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 F356D2E3;\n\tFri, 15 Oct 2021 04:34:40 +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=\"pzE/SxH5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634265281;\n\tbh=zB+mXApzUsgdW3Dgbj3HnUFNV+GfMl0Qrna0Okq7YC4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pzE/SxH5KjWqakoVoqu6WB/bZ709RYZbwB5H0T5d5HErsDFXrW8ml+/ypxLIgzXZ6\n\t5lJxDYt81JlRnFvgltirOigomoQT6MNZPyCZ7AYv1lk3cgKCY+qFqYgH+rh1K2A+u1\n\thrFaZwLV0nIbikjXGboJpkza36GEuUA119oCBjUw=","Date":"Fri, 15 Oct 2021 05:34:25 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20211014174208.50509-1-jacopo@jmondi.org>\n\t<20211014174208.50509-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211014174208.50509-2-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v4 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\" "}}]