[{"id":11179,"web_url":"https://patchwork.libcamera.org/comment/11179/","msgid":"<20200704214540.GF10773@pendragon.ideasonboard.com>","date":"2020-07-04T21:45:40","subject":"Re: [libcamera-devel] [PATCH v3 16/22] libcamera: ipu3: Fill stride\n\tand frameSize at config validation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, Jul 04, 2020 at 10:31:34PM +0900, Paul Elder wrote:\n> Fill the stride and frameSize fields of the StreamConfiguration at\n> configuration validation time instead of at camera configuration time.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> New in v3\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++--------\n> 1 file changed, 4 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 00559ce..f116c93 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -278,6 +278,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \t\t\t\t<< cfg.toString();\n> \t\t\tstatus = Adjusted;\n> \t\t}\n> +\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> +\t\tcfg.stride = info.stride(cfg.size.width, 0);\n\nI think you need more than this. For raw formats, the CIO2 has a 64\nbytes alignment requirement. For other formats, it seems that there's no\nother special alignment, but the kernel code is difficult to follow, so\nI'm not 100% sure.\n\n> +\t\tcfg.frameSize = info.frameSize(cfg.size);\n\nAnd here, we can't compute the frameSize based on cfg.size, it has to be\ncomputed based on the stride that takes the extra alignment into\naccount. I'm tempted to to propose dropping\nPixelFormatInfo::frameSize(), as all we need is\n\n\t\tcfg.frameSize = cfg.stride * cfg.size.height;\n\nhere, and designed a proper helper to calculate the frame size will need\nto take multi-planar formats into account, which we don't support yet.\nOn the other hand, having a PixelFormatInfo::frameSize() function, even\nif not perfect (for instance taking stride and height of the first\nplane and considering that the stride of the other planes can be deduced\nby the stride of the first plane), and enforcing its usage in pipeline\nhandlers, will mean that when it will be time to support multi-planar\nformats properly all the code that needs to be changed will be easily\nidentifiable.\n\n> \t}\n> \n> \treturn status;\n> @@ -495,21 +499,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \n> -\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> \t\t\toutActive = true;\n> \t\t} else if (stream == vfStream) {\n> \t\t\tret = imgu->configureViewfinder(cfg, &outputFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \n> -\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> \t\t\tvfActive = true;\n> -\t\t} else {\n> -\t\t\t/*\n> -\t\t\t * The RAW stream is configured as part of the CIO2 and\n> -\t\t\t * no configuration is needed for the ImgU.\n> -\t\t\t */\n> -\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> \t\t}\n> \t}\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 3836BBD792\n\tfor ;\n\tSat, 4 Jul 2020 21:45:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A7F960E09;\n\tSat, 4 Jul 2020 23:45:46 +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 2B0C4609C7\n\tfor ;\n\tSat, 4 Jul 2020 23:45:45 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98D01296;\n\tSat, 4 Jul 2020 23:45:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Y+5Y8wew\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593899144;\n\tbh=dxeFFEWarJpgWaICt/TKo3CwgtLr76zD0JVkLNMkAII=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Y+5Y8wewTtWqqzcMoCnqxwWOYPIK+x3NgBkcIDjsWUaKBStkjTqVVs8QU5P+L7mcP\n\toj7u1fgvGp8QzrRuqWUH2jv2wo/3NcFdFyyO3I3nS239fFENpmRsrKNlZ28wb3+BYQ\n\tUGL3FngtKCqgHu2tjmcxlsU1kQhPiTnGpZddB5PY=","Date":"Sun, 5 Jul 2020 00:45:40 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200704214540.GF10773@pendragon.ideasonboard.com>","References":"<20200704133140.1738660-1-paul.elder@ideasonboard.com>\n\t<20200704133140.1738660-17-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200704133140.1738660-17-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 16/22] libcamera: ipu3: Fill stride\n\tand frameSize at config validation","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]