[{"id":16969,"web_url":"https://patchwork.libcamera.org/comment/16969/","msgid":"","date":"2021-05-17T03:57:16","subject":"Re: [libcamera-devel] [PATCH v3 2/8] libcamera: ipu3: imgu: Filter\n\tresolutions < IF_CROP_MAX","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thank you for the patch.\n\nOn Fri, May 14, 2021 at 12:20 AM Jacopo Mondi wrote:\n\n> As reported in https://bugs.libcamera.org/show_bug.cgi?id=32\n> allowing resolutions < IF_CROP_MAX leads to a not manageable number\n> configurations to be tested, slowing down the ImgU pipe configuration\n> to a point which is not acceptable for production devices.\n>\n> Filter all resolutions < IF_CROP_MAX to maintain the run-time complexity\n> acceptable and remove the safety check that was meant to avoid overflows\n> when computing the IF rectangle sizes.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=32\n> Signed-off-by: Jacopo Mondi \n>\n\nReviewed-by: Hirokazu Honda \n\n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 21 +++++++++++++++------\n> 1 file changed, 15 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp\n> b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index acc625ab0fac..f25f14a71e9e 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -392,6 +392,17 @@ ImgUDevice::PipeConfig\n> ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> LOG(IPU3, Debug) << \"vf: \" << pipe->viewfinder.toString();\n>\n> const Size &in = pipe->input;\n> +\n> + /*\n> + * \\todo Filter out all resolutions < IF_CROP_MAX.\n> + * See https://bugs.libcamera.org/show_bug.cgi?id=32\n> + */\n> + if (in.width < IF_CROP_MAX_W || in.height < IF_CROP_MAX_H) {\n> + LOG(IPU3, Error) << \"Input resolution \" << in.toString()\n> + << \" not supported\";\n> + return {};\n> + }\n> +\n> Size gdc = calculateGDC(pipe);\n>\n> float bdsSF = static_cast(in.width) / gdc.width;\n> @@ -400,10 +411,8 @@ ImgUDevice::PipeConfig\n> ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> /* Populate the configurations vector by scaling width and height.\n> */\n> unsigned int ifWidth = utils::alignUp(in.width, IF_ALIGN_W);\n> unsigned int ifHeight = utils::alignUp(in.height, IF_ALIGN_H);\n> - unsigned int minIfWidth = std::min(IF_ALIGN_W,\n> - in.width - IF_CROP_MAX_W);\n> - unsigned int minIfHeight = std::min(IF_ALIGN_H,\n> - in.height - IF_CROP_MAX_H);\n> + unsigned int minIfWidth = in.width - IF_CROP_MAX_W;\n> + unsigned int minIfHeight = in.height - IF_CROP_MAX_H;\n> while (ifWidth >= minIfWidth) {\n> while (ifHeight >= minIfHeight) {\n> Size iif{ ifWidth, ifHeight };\n> @@ -417,8 +426,8 @@ ImgUDevice::PipeConfig\n> ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> /* Repeat search by scaling width first. */\n> ifWidth = utils::alignUp(in.width, IF_ALIGN_W);\n> ifHeight = utils::alignUp(in.height, IF_ALIGN_H);\n> - minIfWidth = std::min(IF_ALIGN_W, in.width - IF_CROP_MAX_W);\n> - minIfHeight = std::min(IF_ALIGN_H, in.height - IF_CROP_MAX_H);\n> + minIfWidth = in.width - IF_CROP_MAX_W;\n> + minIfHeight = in.height - IF_CROP_MAX_H;\n> while (ifHeight >= minIfHeight) {\n> /*\n> * \\todo This procedure is probably broken:\n> --\n> 2.31.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 B5D7CC31FB\n\tfor ;\n\tMon, 17 May 2021 03:57:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21F5C68920;\n\tMon, 17 May 2021 05:57:29 +0200 (CEST)","from mail-ed1-x534.google.com (mail-ed1-x534.google.com\n\t[IPv6:2a00:1450:4864:20::534])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73472602B1\n\tfor ;\n\tMon, 17 May 2021 05:57:27 +0200 (CEST)","by mail-ed1-x534.google.com with SMTP id a25so5106339edr.12\n\tfor ;\n\tSun, 16 May 2021 20:57:27 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"hHhEt1tM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=XBfoCrW/Gdg9Ra5DNP0HJF0++KjKvNTPvQ53Tt6NKD4=;\n\tb=hHhEt1tM2gv5lxrXxjI04Omx46dVOxa0YfHoJUFHBbu+9O3xu0A4Ew6NyzrnppgVBH\n\tknmFzs8vaPuENQSl8yT1xatgiwDOkv3UDo+0T1r5k2fE+42K3JZpS5zG15JMa0hVNJPe\n\tAE2qPqLHyNcCCT0ieBEZuKYvwetiHkn/n+pTk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=XBfoCrW/Gdg9Ra5DNP0HJF0++KjKvNTPvQ53Tt6NKD4=;\n\tb=ooN7yHuTuHZbPpgMjW6L7gf5AlvGHbEGvqsN9EJ407dtm5fhK4awodMOTc+vPj7tqW\n\t2/pNSqfqkar0bVPNw9aPJ++EuwtQZ6B3SLAPeovRGvYLmwDT9sps7YYezljFMdF8zuwk\n\tYXqJwdlrbODbYTcUROtPFw/7kiqoQZy3cqeZ1DkuN3MNsyqgJwB3l7glFPokrS8zlueD\n\tRgzLsU7WE3BqVM5Q1IwnCRjokzAGirbyjMcUMYndS/Er2oUGJfWalxEjGnlVTbmv5B3N\n\tHWqNSLymcWuTlaHtiLw+SrnmlAZlXd2rebB+KVIaoxlhxucS7vlECubEov41v1+/1K3l\n\tbqtg==","X-Gm-Message-State":"AOAM5305pnCsgB/kb0rGK+/iqyRogMIXZal3oc5G95M3kYDtW8fTImU8\n\tuadh84rsRUSJeENEUGBhtoiNwsgrIMWOnVCj8x0EzSBctgY=","X-Google-Smtp-Source":"ABdhPJyUZl/pKufXZCgjaM3Cx7/HjVPx/+pcX+f3auE/RuLMjVAM87CM6mXL9woW7fFktdEepjuHLe1wnFDrM+sYhyM=","X-Received":"by 2002:a50:bec1:: with SMTP id\n\te1mr69166909edk.116.1621223847110; \n\tSun, 16 May 2021 20:57:27 -0700 (PDT)","MIME-Version":"1.0","References":"<20210513152116.17666-1-jacopo@jmondi.org>\n\t<20210513152116.17666-3-jacopo@jmondi.org>","In-Reply-To":"<20210513152116.17666-3-jacopo@jmondi.org>","From":"Hirokazu Honda ","Date":"Mon, 17 May 2021 12:57:16 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000082438605c27e97e7\"","Subject":"Re: [libcamera-devel] [PATCH v3 2/8] libcamera: ipu3: imgu: Filter\n\tresolutions < IF_CROP_MAX","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17042,"web_url":"https://patchwork.libcamera.org/comment/17042/","msgid":"","date":"2021-05-20T08:43:48","subject":"Re: [libcamera-devel] [PATCH v3 2/8] libcamera: ipu3: imgu: Filter\n\tresolutions < IF_CROP_MAX","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 13/05/2021 17:21, Jacopo Mondi wrote:\n> As reported in https://bugs.libcamera.org/show_bug.cgi?id=32\n> allowing resolutions < IF_CROP_MAX leads to a not manageable number\n> configurations to be tested, slowing down the ImgU pipe configuration\n> to a point which is not acceptable for production devices.\n> \n> Filter all resolutions < IF_CROP_MAX to maintain the run-time complexity\n> acceptable and remove the safety check that was meant to avoid overflows\n> when computing the IF rectangle sizes.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=32\n> Signed-off-by: Jacopo Mondi \nReviewed-by: Jean-Michel Hautbois \n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 21 +++++++++++++++------\n> 1 file changed, 15 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index acc625ab0fac..f25f14a71e9e 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -392,6 +392,17 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> \tLOG(IPU3, Debug) << \"vf: \" << pipe->viewfinder.toString();\n> \n> \tconst Size &in = pipe->input;\n> +\n> +\t/*\n> +\t * \\todo Filter out all resolutions < IF_CROP_MAX.\n> +\t * See https://bugs.libcamera.org/show_bug.cgi?id=32\n> +\t */\n> +\tif (in.width < IF_CROP_MAX_W || in.height < IF_CROP_MAX_H) {\n> +\t\tLOG(IPU3, Error) << \"Input resolution \" << in.toString()\n> +\t\t\t\t << \" not supported\";\n> +\t\treturn {};\n> +\t}\n> +\n> \tSize gdc = calculateGDC(pipe);\n> \n> \tfloat bdsSF = static_cast(in.width) / gdc.width;\n> @@ -400,10 +411,8 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> \t/* Populate the configurations vector by scaling width and height. */\n> \tunsigned int ifWidth = utils::alignUp(in.width, IF_ALIGN_W);\n> \tunsigned int ifHeight = utils::alignUp(in.height, IF_ALIGN_H);\n> -\tunsigned int minIfWidth = std::min(IF_ALIGN_W,\n> -\t\t\t\t\t in.width - IF_CROP_MAX_W);\n> -\tunsigned int minIfHeight = std::min(IF_ALIGN_H,\n> -\t\t\t\t\t in.height - IF_CROP_MAX_H);\n> +\tunsigned int minIfWidth = in.width - IF_CROP_MAX_W;\n> +\tunsigned int minIfHeight = in.height - IF_CROP_MAX_H;\n> \twhile (ifWidth >= minIfWidth) {\n> \t\twhile (ifHeight >= minIfHeight) {\n> \t\t\tSize iif{ ifWidth, ifHeight };\n> @@ -417,8 +426,8 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> \t/* Repeat search by scaling width first. */\n> \tifWidth = utils::alignUp(in.width, IF_ALIGN_W);\n> \tifHeight = utils::alignUp(in.height, IF_ALIGN_H);\n> -\tminIfWidth = std::min(IF_ALIGN_W, in.width - IF_CROP_MAX_W);\n> -\tminIfHeight = std::min(IF_ALIGN_H, in.height - IF_CROP_MAX_H);\n> +\tminIfWidth = in.width - IF_CROP_MAX_W;\n> +\tminIfHeight = in.height - IF_CROP_MAX_H;\n> \twhile (ifHeight >= minIfHeight) {\n> \t\t/*\n> \t\t * \\todo This procedure is probably broken:\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 3DBEAC31FB\n\tfor ;\n\tThu, 20 May 2021 08:43:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 825EC6891E;\n\tThu, 20 May 2021 10:43:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B054068919\n\tfor ;\n\tThu, 20 May 2021 10:43:49 +0200 (CEST)","from [IPv6:2a01:e0a:169:7140:9d6a:69b6:9b62:2634] (unknown\n\t[IPv6:2a01:e0a:169:7140:9d6a:69b6:9b62:2634])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A137D41;\n\tThu, 20 May 2021 10:43:49 +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=\"lnARj2QC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621500229;\n\tbh=X5710CxCRMdkVTiWRdI/gPdw1d7v1fp86w4IqpXEiiI=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=lnARj2QCHJEWXNd0lneVI5rtzdgR/rAjObtDjarDlpXFJGGbg7OedXldkQVzRmViK\n\tC8+RaLelOqeL7na1D29klHsLKl/OcRvNvlcguZEylsBkmoEjYiTiWHrh1l9NkBuz8i\n\tdc+roZQ75XDTRniKJx/X+lzGJvwvC6fJ0UwQRyww=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20210513152116.17666-1-jacopo@jmondi.org>\n\t<20210513152116.17666-3-jacopo@jmondi.org>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Thu, 20 May 2021 10:43:48 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<20210513152116.17666-3-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 2/8] libcamera: ipu3: imgu: Filter\n\tresolutions < IF_CROP_MAX","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]