[{"id":15856,"web_url":"https://patchwork.libcamera.org/comment/15856/","msgid":"","date":"2021-03-24T07:17:21","subject":"Re: [libcamera-devel] [PATCH 6/7] libcamera: ipu3: imgu: Add pipe\n\tcalculation debug","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Jacopo,\n\nThanks for the patch,\n\nOn 18/03/2021 11:39, Jacopo Mondi wrote:\n> Add pipe calculation debug with a new associated log category.\n> \n> This helps compare the pipe calculation with the one performed by the\n> python script.\n\nThat is useful indeed !\n\n> Signed-off-by: Jacopo Mondi \nTested-by: Jean-Michel Hautbois \nReviewed-by: Jean-Michel Hautbois \n\n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++++++++++++\n> 1 file changed, 24 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index ccc85864fc39..b6a1fe34494e 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -23,6 +23,7 @@\n> namespace libcamera {\n> \n> LOG_DECLARE_CATEGORY(IPU3)\n> +LOG_DEFINE_CATEGORY(IMGUPIPE)\n> \n> namespace {\n> \n> @@ -128,6 +129,8 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \tunsigned int ifHeight;\n> \tfloat bdsHeight;\n> \n> +\tLOG(IMGUPIPE, Debug) << \"BDS sf: \" << bdsSF << \" BDS width: \" << bdsWidth;\n> +\n> \tif (!isSameRatio(pipe->input, gdc)) {\n> \t\tunsigned int foundIfHeights[2] = { 0, 0 };\n> \t\tfloat estIFHeight = (iif.width * gdc.height) /\n> @@ -135,6 +138,9 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \t\testIFHeight = std::clamp(estIFHeight, minIFHeight, iif.height);\n> \n> \t\tifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);\n> +\t\tLOG(IMGUPIPE, Debug) << \"Estimate IF Height: \" << estIFHeight\n> +\t\t\t\t << \" IF Height: \" << ifHeight;\n> +\n> \t\twhile (ifHeight >= minIFHeight && ifHeight <= iif.height &&\n> \t\t ifHeight / bdsSF >= minBDSHeight) {\n> \n> @@ -176,6 +182,11 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \t\tif (foundIfHeights[0] || foundIfHeights[1]) {\n> \t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> \n> +\t\t\tLOG(IMGUPIPE, Debug)\n> +\t\t\t\t<< \"IF: \" << Size(iif.width, ifHeight).toString()\n> +\t\t\t\t<< \"BDS: \" << Size(bdsWidth, bdsIntHeight).toString()\n> +\t\t\t\t<< \"GDC: \" << gdc.toString();\n> +\n> \t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },\n> \t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> \t\t\treturn;\n> @@ -190,6 +201,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \n> \t\t\t\tif (!(ifHeight % IF_ALIGN_H) &&\n> \t\t\t\t !(bdsIntHeight % BDS_ALIGN_H)) {\n> +\n> +\t\t\t\t\tLOG(IMGUPIPE, Debug)\n> +\t\t\t\t\t\t<< \"IF: \" << Size(iif.width, ifHeight).toString()\n> +\t\t\t\t\t\t<< \"BDS: \" << Size(bdsWidth, bdsIntHeight).toString()\n> +\t\t\t\t\t\t<< \"GDC: \" << gdc.toString();\n> +\n> \t\t\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },\n> \t\t\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> \t\t\t\t}\n> @@ -269,6 +286,8 @@ Size calculateGDC(ImgUDevice::Pipe *pipe)\n> \tgdc.width = main.width * sf;\n> \tgdc.height = main.height * sf;\n> \n> +\tLOG(IMGUPIPE, Debug) << \"GDC: \" << gdc.toString();\n> +\n> \treturn gdc;\n> }\n> \n> @@ -286,6 +305,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)\n> \tfov.w = (inW - (ifCropW + gdcCropW)) / inW;\n> \tfov.h = (inH - (ifCropH + gdcCropH)) / inH;\n> \n> +\tLOG(IMGUPIPE, Debug)\n> +\t\t<< \"IF (\" << pipe.iif.toString() << \") - BDS (\"\n> +\t\t<< pipe.bds.toString() << \") - GDC (\" << pipe.gdc.toString()\n> +\t\t<< \") -> FOV: \" << fov.w << \"x\" << fov.h;\n> +\n> \treturn fov;\n> }\n> \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 9F052C32E7\n\tfor ;\n\tWed, 24 Mar 2021 07:17:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D42968D63;\n\tWed, 24 Mar 2021 08:17:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A8A968D47\n\tfor ;\n\tWed, 24 Mar 2021 08:17:22 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:21bd:98cc:c21e:1364] (unknown\n\t[IPv6:2a01:e0a:169:7140:21bd:98cc:c21e:1364])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E1A8B580;\n\tWed, 24 Mar 2021 08:17:21 +0100 (CET)"],"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=\"ZxeTCpgD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616570241;\n\tbh=2SJwnfCqq/oxOC8UJlxPsC8McHZDzC7sLNv0EaSMsT8=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ZxeTCpgDPys5LjWM+fMe3+sg0ypVXF9y07c4Wzg/DHzbutL5dN8tc1I+WHlYXdK/j\n\tHOKxobp/2pNRik4m+EOE4642EYPTKpkh/z2HbN1Z0/Y+3E3PQSnufBeR5JyInXyWeu\n\tp4e/IN3alWCQrouygMorz/Iwy9DSt2w7EuDqJGgE=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20210318103941.18837-1-jacopo@jmondi.org>\n\t<20210318103941.18837-7-jacopo@jmondi.org>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Wed, 24 Mar 2021 08:17:21 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210318103941.18837-7-jacopo@jmondi.org>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 6/7] libcamera: ipu3: imgu: Add pipe\n\tcalculation debug","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\" "}},{"id":16193,"web_url":"https://patchwork.libcamera.org/comment/16193/","msgid":"","date":"2021-04-12T23:58:27","subject":"Re: [libcamera-devel] [PATCH 6/7] libcamera: ipu3: imgu: Add pipe\n\tcalculation debug","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, Mar 18, 2021 at 11:39:40AM +0100, Jacopo Mondi wrote:\n> Add pipe calculation debug with a new associated log category.\n> \n> This helps compare the pipe calculation with the one performed by the\n> python script.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++++++++++++\n> 1 file changed, 24 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index ccc85864fc39..b6a1fe34494e 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -23,6 +23,7 @@\n> namespace libcamera {\n> \n> LOG_DECLARE_CATEGORY(IPU3)\n> +LOG_DEFINE_CATEGORY(IMGUPIPE)\n\nImgUPipe\n\n> \n> namespace {\n> \n> @@ -128,6 +129,8 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \tunsigned int ifHeight;\n> \tfloat bdsHeight;\n> \n> +\tLOG(IMGUPIPE, Debug) << \"BDS sf: \" << bdsSF << \" BDS width: \" << bdsWidth;\n\ns/ BDS width/, BDS width/\n\n> +\n> \tif (!isSameRatio(pipe->input, gdc)) {\n> \t\tunsigned int foundIfHeights[2] = { 0, 0 };\n> \t\tfloat estIFHeight = (iif.width * gdc.height) /\n> @@ -135,6 +138,9 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \t\testIFHeight = std::clamp(estIFHeight, minIFHeight, iif.height);\n> \n> \t\tifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);\n> +\t\tLOG(IMGUPIPE, Debug) << \"Estimate IF Height: \" << estIFHeight\n\ns/Estimate/Estimated/\n\n> +\t\t\t\t << \" IF Height: \" << ifHeight;\n\ns/ IF/, IF/\n\nand similarly below (the code is certainly readable when split across\nmultiple lines, but the debug message will be printed in the log on a\nsingle line, so commas help readability).\n\n> +\n> \t\twhile (ifHeight >= minIFHeight && ifHeight <= iif.height &&\n> \t\t ifHeight / bdsSF >= minBDSHeight) {\n> \n> @@ -176,6 +182,11 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \t\tif (foundIfHeights[0] || foundIfHeights[1]) {\n> \t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> \n> +\t\t\tLOG(IMGUPIPE, Debug)\n> +\t\t\t\t<< \"IF: \" << Size(iif.width, ifHeight).toString()\n> +\t\t\t\t<< \"BDS: \" << Size(bdsWidth, bdsIntHeight).toString()\n> +\t\t\t\t<< \"GDC: \" << gdc.toString();\n\nI'd create two variables to store the IF and BDS sizes, as they're used\non the next line.\n\n> +\n> \t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },\n> \t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> \t\t\treturn;\n> @@ -190,6 +201,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc\n> \n> \t\t\t\tif (!(ifHeight % IF_ALIGN_H) &&\n> \t\t\t\t !(bdsIntHeight % BDS_ALIGN_H)) {\n> +\n> +\t\t\t\t\tLOG(IMGUPIPE, Debug)\n> +\t\t\t\t\t\t<< \"IF: \" << Size(iif.width, ifHeight).toString()\n> +\t\t\t\t\t\t<< \"BDS: \" << Size(bdsWidth, bdsIntHeight).toString()\n> +\t\t\t\t\t\t<< \"GDC: \" << gdc.toString();\n\nSame here.\n\n> +\n> \t\t\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },\n> \t\t\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> \t\t\t\t}\n> @@ -269,6 +286,8 @@ Size calculateGDC(ImgUDevice::Pipe *pipe)\n> \tgdc.width = main.width * sf;\n> \tgdc.height = main.height * sf;\n> \n> +\tLOG(IMGUPIPE, Debug) << \"GDC: \" << gdc.toString();\n> +\n> \treturn gdc;\n> }\n> \n> @@ -286,6 +305,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)\n> \tfov.w = (inW - (ifCropW + gdcCropW)) / inW;\n> \tfov.h = (inH - (ifCropH + gdcCropH)) / inH;\n> \n> +\tLOG(IMGUPIPE, Debug)\n> +\t\t<< \"IF (\" << pipe.iif.toString() << \") - BDS (\"\n> +\t\t<< pipe.bds.toString() << \") - GDC (\" << pipe.gdc.toString()\n> +\t\t<< \") -> FOV: \" << fov.w << \"x\" << fov.h;\n> +\n> \treturn fov;\n\nOn a side note, we should create a SizeF class, but that's out of scope\nfor this series.\n\nReviewed-by: Laurent Pinchart \n\n> }\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 2767DBD224\n\tfor ;\n\tMon, 12 Apr 2021 23:59:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A0180687EC;\n\tTue, 13 Apr 2021 01:59:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA7E6605AE\n\tfor ;\n\tTue, 13 Apr 2021 01:59:17 +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 62FBB6F2;\n\tTue, 13 Apr 2021 01:59:17 +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=\"iHyIXrLw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618271957;\n\tbh=csfY576ZcRB+H1Rb2iKpuHVC5WOhmrOdiMUo/0xhC6E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iHyIXrLwrfVSUmW4GeJt5PKgIWlu1bn7CcSNmuQWmNqCC+s9fHVbfzO98e+ZqrzRl\n\td2qhdNgBTg0Y3QuHbUj41wNN1G2VSlfK3mmegMtcEPeZi4ArV/uYGB0uK+h2rbviEQ\n\tEHKZ1OcZm4amAsoLAW65BJJgnf6b8wiBngDrPAd4=","Date":"Tue, 13 Apr 2021 02:58:27 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210318103941.18837-1-jacopo@jmondi.org>\n\t<20210318103941.18837-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210318103941.18837-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 6/7] libcamera: ipu3: imgu: Add pipe\n\tcalculation debug","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\" "}}]