[{"id":11289,"web_url":"https://patchwork.libcamera.org/comment/11289/","msgid":"<20200709140019.GQ3875643@oden.dyn.berto.se>","date":"2020-07-09T14:00:19","subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for this Herculean effort.\n\nOn 2020-07-09 10:41:25 +0200, Jacopo Mondi wrote:\n> Instrument the ImgU component to dynamically calculate the image\n> manipulation pipeline intermediate sizes.\n> \n> To correctly configure the ImgU it is necessary to program the IF, BDS\n> and GDC sizes, which are currently fixed to the input frame size.\n> \n> The procedure used to calculate the intermediate sizes has been ported\n> from the pipe_config.py python script, available at:\n> https://github.com/intel/intel-ipu3-pipecfg\n> at revision:\n> 61e83f2f7606 (\"Add more information into README\")\n> \n> Define two structures (ImgUDevice::Pipe and ImgUdevice::PipeConfig)\n> to allow the pipeline handler to supply and retrieve configuration\n> parameters from the ImgU unit.\n> \n> Finally, add a new operation to the ImgUDdevice that calculates\n> the pipe configuration parameters based on the requested input and\n> output sizes.\n> \n> Signed-off-by: Jacopo Mondi \n\nI will not review all the calculations from the python script to C++ and \nwill therefore trust you on those :-) One small typo in a comment bellow \nother then that.\n\nAcked-by: Niklas Söderlund \n\n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 365 +++++++++++++++++++++++++++\n> src/libcamera/pipeline/ipu3/imgu.h | 20 ++\n> 2 files changed, 385 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index d7f4173d3607..d1addb7d84d1 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -7,6 +7,9 @@\n> \n> #include \"imgu.h\"\n> \n> +#include \n> +#include \n> +\n> #include \n> \n> #include \n> @@ -19,6 +22,311 @@ namespace libcamera {\n> \n> LOG_DECLARE_CATEGORY(IPU3)\n> \n> +namespace {\n> +\n> +static constexpr unsigned int FILTER_H = 4;\n> +\n> +static constexpr unsigned int IF_ALIGN_W = 2;\n> +static constexpr unsigned int IF_ALIGN_H = 4;\n> +\n> +static constexpr unsigned int BDS_ALIGN_W = 2;\n> +static constexpr unsigned int BDS_ALIGN_H = 4;\n> +\n> +static constexpr unsigned int IF_CROP_MAX_W = 40;\n> +static constexpr unsigned int IF_CROP_MAX_H = 540;\n> +\n> +static constexpr float BDS_SF_MAX = 2.5;\n> +static constexpr float BDS_SF_MIN = 1.0;\n> +static constexpr float BDS_SF_STEP = 0.03125;\n> +\n> +/* BSD scaling factors: min=1, max=2.5, step=1/32 */\n> +const std::vector bdsScalingFactors = {\n> +\t1, 1.03125, 1.0625, 1.09375, 1.125, 1.15625, 1.1875, 1.21875, 1.25,\n> +\t1.28125, 1.3125, 1.34375, 1.375, 1.40625, 1.4375, 1.46875, 1.5, 1.53125,\n> +\t1.5625, 1.59375, 1.625, 1.65625, 1.6875, 1.71875, 1.75, 1.78125, 1.8125,\n> +\t1.84375, 1.875, 1.90625, 1.9375, 1.96875, 2, 2.03125, 2.0625, 2.09375,\n> +\t2.125, 2.15625, 2.1875, 2.21875, 2.25, 2.28125, 2.3125, 2.34375, 2.375,\n> +\t2.40625, 2.4375, 2.46875, 2.5\n> +};\n> +\n> +/* GDC scaling factors: min=1, max=16, step=1/4 */\n> +const std::vector gdcScalingFactors = {\n> +\t1, 1.25, 1.5, 1.75, 2, 2.25, 2.5, 2.75, 3, 3.25, 3.5, 3.75, 4, 4.25,\n> +\t4.5, 4.75, 5, 5.25, 5.5, 5.75, 6, 6.25, 6.5, 6.75, 7, 7.25, 7.5, 7.75,\n> +\t8, 8.25, 8.5, 8.75, 9, 9.25, 9.5, 9.75, 10, 10.25, 10.5, 10.75, 11,\n> +\t11.25, 11.5, 11.75, 12, 12.25, 12.5, 12.75, 13, 13.25, 13.5, 13.75, 14,\n> +\t14.25, 14.5, 14.75, 15, 15.25, 15.5, 15.75, 16,\n> +};\n> +\n> +std::vector pipeConfigs;\n> +\n> +struct FOV {\n> +\tfloat w;\n> +\tfloat h;\n> +\n> +\tbool isLarger(const FOV &other)\n> +\t{\n> +\t\tif (w > other.w)\n> +\t\t\treturn true;\n> +\t\tif (w == other.w && h > other.h)\n> +\t\t\treturn true;\n> +\t\treturn false;\n> +\t}\n> +};\n> +\n> +/* Approximate a scaling factor sf to the closest one available in a range. */\n> +float findScaleFactor(float sf, const std::vector &range,\n> +\t\t bool roundDown = false)\n> +{\n> +\tif (sf <= range[0])\n> +\t\treturn range[0];\n> +\tif (sf >= range[range.size() - 1])\n> +\t\treturn range[range.size() - 1];\n> +\n> +\n> +\tfloat bestDiff = std::numeric_limits::max();\n> +\tunsigned int index = 0;\n> +\tfor (unsigned int i = 0; i < range.size(); ++i) {\n> +\t\tfloat diff = std::abs(sf - range[i]);\n> +\t\tif (diff < bestDiff) {\n> +\t\t\tbestDiff = diff;\n> +\t\t\tindex = i;\n> +\t\t}\n> +\t}\n> +\n> +\tif (roundDown && index > 0 && sf < range[index])\n> +\t\tindex--;\n> +\n> +\treturn range[index];\n> +}\n> +\n> +bool isSameRatio(const Size &in, const Size &out)\n> +{\n> +\tfloat inRatio = static_cast(in.width) /\n> +\t\t\tstatic_cast(in.height);\n> +\tfloat outRatio = static_cast(out.width) /\n> +\t\t\t static_cast(out.height);\n> +\n> +\tif (std::abs(inRatio - outRatio) > 0.1)\n> +\t\treturn false;\n> +\n> +\treturn true;\n> +}\n> +\n> +unsigned int alignIncrease(unsigned int len, unsigned int align)\n> +{\n> +\tif (len % align)\n> +\t\treturn (std::floor(static_cast(len) /\n> +\t\t\t\t static_cast(align)) + 1) * align;\n> +\treturn len;\n> +}\n> +\n> +void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> +\t\t\tunsigned int bdsWidth, float bdsSF)\n> +{\n> +\tunsigned int minIFHeight = iif.height - IF_CROP_MAX_H;\n> +\tunsigned int minBDSHeight = gdc.height + FILTER_H * 2;\n> +\tfloat bdsHeight;\n> +\n> +\tif (!isSameRatio(pipe->input, gdc)) {\n> +\t\tbool found = false;\n> +\t\tfloat estIFHeight = static_cast(iif.width) *\n> +\t\t\t\t static_cast(gdc.height) /\n> +\t\t\t\t static_cast(gdc.width);\n> +\t\testIFHeight = utils::clamp(estIFHeight,\n> +\t\t\t\t\t\t static_cast(minIFHeight),\n> +\t\t\t\t\t\t static_cast(iif.height));\n> +\t\tfloat ifHeight =\n> +\t\t\tstatic_cast(alignIncrease(static_cast(estIFHeight),\n> +\t\t\t\t\t\t\t IF_ALIGN_H));\n> +\t\twhile (ifHeight >= minIFHeight && (ifHeight / bdsSF >= minBDSHeight)) {\n> +\t\t\tbdsHeight = ifHeight / bdsSF;\n> +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> +\t\t\t\tunsigned int bdsIntHeight =\n> +\t\t\t\t\tstatic_cast(bdsHeight);\n> +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tifHeight -= IF_ALIGN_H;\n> +\t\t}\n> +\n> +\t\tifHeight = static_cast(\n> +\t\t\talignIncrease(static_cast(estIFHeight),\n> +\t\t\t\t IF_ALIGN_H));\n> +\n> +\t\twhile (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) {\n> +\t\t\tbdsHeight = ifHeight / bdsSF;\n> +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> +\t\t\t\tunsigned int bdsIntHeight =\n> +\t\t\t\t\tstatic_cast(bdsHeight);\n> +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tifHeight += IF_ALIGN_H;\n> +\t\t}\n> +\n> +\t\tif (found) {\n> +\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> +\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> +\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> +\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> +\t\t\treturn;\n> +\t\t}\n> +\t} else {\n> +\t\tfloat ifHeight = static_cast(alignIncrease(iif.height, IF_ALIGN_H));\n> +\t\twhile (ifHeight > minIFHeight && (ifHeight / bdsSF > minBDSHeight)) {\n> +\t\t\tbdsHeight = ifHeight / bdsSF;\n> +\t\t\tif (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {\n> +\t\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> +\t\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> +\n> +\t\t\t\tif (!(ifIntHeight % IF_ALIGN_H) &&\n> +\t\t\t\t !(bdsIntHeight % BDS_ALIGN_H)) {\n> +\t\t\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> +\t\t\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tifHeight -= IF_ALIGN_H;\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> +\t\t float bdsSF)\n> +{\n> +\tunsigned int minBDSWidth = gdc.width + FILTER_H * 2;\n> +\n> +\tfloat sf = bdsSF;\n> +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> +\n> +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> +\t\t}\n> +\n> +\t\tsf += BDS_SF_STEP;\n> +\t}\n> +\n> +\tsf = bdsSF;\n> +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> +\n> +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> +\t\t}\n> +\n> +\t\tsf -= BDS_SF_STEP;\n> +\t}\n> +}\n> +\n> +Size calculateGDC(ImgUDevice::Pipe *pipe)\n> +{\n> +\tconst Size &in = pipe->input;\n> +\tconst Size &main = pipe->output;\n> +\tconst Size &vf = pipe->viewfinder;\n> +\tSize gdc;\n> +\n> +\tif (!vf.isNull()) {\n> +\t\tgdc.width = main.width;\n> +\n> +\t\tfloat ratio = static_cast(main.width) *\n> +\t\t\t static_cast(vf.height) /\n> +\t\t\t static_cast(vf.width);\n> +\t\tgdc.height = std::max(static_cast(main.height), ratio);\n> +\n> +\t\treturn gdc;\n> +\t}\n> +\n> +\tif (!isSameRatio(in, main)) {\n> +\t\tgdc = main;\n> +\t\treturn gdc;\n> +\t}\n> +\n> +\tfloat totalSF = static_cast(in.width) /\n> +\t\t\tstatic_cast(main.width);\n> +\tfloat bdsSF = totalSF > 2 ? 2 : 1;\n> +\tfloat yuvSF = totalSF / bdsSF;\n> +\tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n> +\n> +\tgdc.width = static_cast(main.width) * sf;\n> +\tgdc.height = static_cast(main.height) * sf;\n> +\n> +\treturn gdc;\n> +}\n> +\n> +FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)\n> +{\n> +\tFOV fov{};\n> +\n> +\tfloat inW = static_cast(in.width);\n> +\tfloat inH = static_cast(in.height);\n> +\tfloat ifCropW = static_cast(in.width) - static_cast(pipe.iif.width);\n> +\tfloat ifCropH = static_cast(in.height) - static_cast(pipe.iif.height);\n> +\tfloat gdcCropW = static_cast(pipe.bds.width - pipe.gdc.width) * pipe.bds_sf;\n> +\tfloat gdcCropH = static_cast(pipe.bds.height - pipe.gdc.height) * pipe.bds_sf;\n> +\tfov.w = (inW - (ifCropW + gdcCropW)) / inW;\n> +\tfov.h = (inH - (ifCropH + gdcCropH)) / inH;\n> +\n> +\treturn fov;\n> +}\n> +\n> +} /* namespace */\n> +\n> +/**\n> + * \\struct PipeConfig\n> + * \\brief The ImgU pipe configuration parameters\n> + *\n> + * The ImgU image pipeline is composed of several hardware blocks that crop\n> + * and scale the input image to obtain the desired output sizes. The\n> + * scaling/cropping operations of those components is configured though the\n> + * V4L2 selection API and the V4L2 subdev API applied to the ImgU media entity.\n> + *\n> + * The configurable components in the pipeline are:\n> + * - IF: image feeder\n> + * - BDS: bayer downscaler\n> + * = GDC: geometric distorsion correction\n\ns/=/-/\n\n> + *\n> + * The IF crop rectangle is controlled by the V4L2_SEL_TGT_CROP selection target\n> + * applied to the ImgU media entity sink pad number 0. The BDS scaler is\n> + * controlled by the V4L2_SEL_TGT_COMPOSE target on the same pad, while the GDC\n> + * output size is configured with the VIDIOC_SUBDEV_S_FMT IOCTL, again on pad\n> + * number 0.\n> + *\n> + * The PipeConfig structure collects the sizes of each of those components\n> + * plus the BDS scaling factor used to calculate the field of view\n> + * of the final images.\n> + */\n> +\n> +/**\n> + * \\struct Pipe\n> + * \\brief Describe the ImgU requested configuration\n> + *\n> + * The ImgU unit processes images through several components, which have\n> + * to be properly configured inspecting the input image size and the desired\n> + * output sizes. This structure collects the ImgU input configuration and the\n> + * requested main output and viewfinder configurations.\n> + *\n> + * \\var Pipe::input\n> + * \\brief The input image size\n> + *\n> + * \\var Pipe::output\n> + * \\brief The requested main output size\n> + *\n> + * \\var Pipe::viewfinder\n> + * \\brief The requested viewfinder size\n> + */\n> +\n> /**\n> * \\brief Initialize components of the ImgU instance\n> * \\param[in] mediaDevice The ImgU instance media device\n> @@ -74,6 +382,63 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Calculate the ImgU pipe configuration parameters\n> + * \\param[in] pipe The requested ImgU configuration\n> + * \\return An ImgUDevice::PipeConfig instance on success, an empty configuration\n> + * otherwise\n> + */\n> +ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> +{\n> +\tpipeConfigs.clear();\n> +\n> +\tLOG(IPU3, Debug) << \"Calculating pipe configuration for: \";\n> +\tLOG(IPU3, Debug) << \"input: \" << pipe->input.toString();\n> +\tLOG(IPU3, Debug) << \"main: \" << pipe->output.toString();\n> +\tLOG(IPU3, Debug) << \"vf: \" << pipe->viewfinder.toString();\n> +\n> +\tconst Size &in = pipe->input;\n> +\tSize gdc = calculateGDC(pipe);\n> +\n> +\tunsigned int ifWidth = alignIncrease(in.width, IF_ALIGN_W);\n> +\tunsigned int ifHeight = in.height;\n> +\tunsigned int minIfWidth = in.width - IF_CROP_MAX_W;\n> +\tfloat bdsSF = static_cast(in.width) /\n> +\t\t static_cast(gdc.width);\n> +\tfloat sf = findScaleFactor(bdsSF, bdsScalingFactors, true);\n> +\twhile (ifWidth >= minIfWidth) {\n> +\t\tSize iif{ ifWidth, ifHeight };\n> +\t\tcalculateBDS(pipe, iif, gdc, sf);\n> +\n> +\t\tifWidth -= IF_ALIGN_W;\n> +\t}\n> +\n> +\tif (pipeConfigs.size() == 0) {\n> +\t\tLOG(IPU3, Error) << \"Failed to calculate pipe configuration\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tFOV bestFov = calcFOV(pipe->input, pipeConfigs[0]);\n> +\tunsigned int bestIndex = 0;\n> +\tunsigned int p = 0;\n> +\tfor (auto pipeConfig : pipeConfigs) {\n> +\t\tFOV fov = calcFOV(pipe->input, pipeConfig);\n> +\t\tif (fov.isLarger(bestFov)) {\n> +\t\t\tbestFov = fov;\n> +\t\t\tbestIndex = p;\n> +\t\t}\n> +\n> +\t\t++p;\n> +\t}\n> +\n> +\tLOG(IPU3, Debug) << \"Computed pipe configuration: \";\n> +\tLOG(IPU3, Debug) << \"IF: \" << pipeConfigs[bestIndex].iif.toString();\n> +\tLOG(IPU3, Debug) << \"BDS: \" << pipeConfigs[bestIndex].bds.toString();\n> +\tLOG(IPU3, Debug) << \"GDC: \" << pipeConfigs[bestIndex].gdc.toString();\n> +\n> +\treturn pipeConfigs[bestIndex];\n> +}\n> +\n> /**\n> * \\brief Configure the ImgU unit input\n> * \\param[in] size The ImgU input frame size\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> index 5c124af2e9fe..15ee9a7f5698 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.h\n> +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> @@ -23,8 +23,28 @@ struct StreamConfiguration;\n> class ImgUDevice\n> {\n> public:\n> +\tstruct PipeConfig {\n> +\t\tfloat bds_sf;\n> +\t\tSize iif;\n> +\t\tSize bds;\n> +\t\tSize gdc;\n> +\n> +\t\tbool isNull() const\n> +\t\t{\n> +\t\t\treturn iif.isNull() || bds.isNull() || gdc.isNull();\n> +\t\t}\n> +\t};\n> +\n> +\tstruct Pipe {\n> +\t\tSize input;\n> +\t\tSize output;\n> +\t\tSize viewfinder;\n> +\t};\n> +\n> \tint init(MediaDevice *media, unsigned int index);\n> \n> +\tPipeConfig calculatePipeConfig(Pipe *pipe);\n> +\n> \tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> \n> \tint configureOutput(const StreamConfiguration &cfg,\n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 CCDD4BD790\n\tfor ;\n\tThu, 9 Jul 2020 14:00:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FFB3612E6;\n\tThu, 9 Jul 2020 16:00:24 +0200 (CEST)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 58987611B2\n\tfor ;\n\tThu, 9 Jul 2020 16:00:22 +0200 (CEST)","by mail-lj1-x22c.google.com with SMTP id d17so2520392ljl.3\n\tfor ;\n\tThu, 09 Jul 2020 07:00:22 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tr13sm834571ljg.101.2020.07.09.07.00.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Jul 2020 07:00:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"R7HSHud6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=xD7xmuwLoWHZDgDhSK5Sq1s/QzQbtJfkJvidLw0gT7E=;\n\tb=R7HSHud6ZflwCtKrCdwYk4CT+PXwE8WnEWwnPniaf0DmHqJ/HHUnMVQZo9bDhhNJEy\n\tDJPXPPik59qBMufcPQMAX7BA9OnfJzmD7ADYZo5CUTfB9JjbMms31Dg4X9GUXXU2xRUt\n\tdSI06+53Pdzw4it7x4Wzgt4Y6P528u8YGCBz1vQ2EZm1Vcd2Fi0IPeKDVzb+kbbR1v0y\n\tFStYvDnj7HdOfEzeyg709KWW2ayVlLLDFD4AOA/DbtMW+1nizS3wz2StSfercPVgBgL3\n\tE/OpaP6rR1toHb4fHgXe1ezTA5nPCY5q315dc1LfrOs7H3MPajHfSerBkYUFU1NZPRuj\n\tIvDg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=xD7xmuwLoWHZDgDhSK5Sq1s/QzQbtJfkJvidLw0gT7E=;\n\tb=bTeaNldlcPfBsuUU5FP3mZ7165I51zVobcHy6y6eNDiiuWx7d4T/wFT0YXOA5Z9niH\n\tKTYm208dJN7mYc0nlD6b0Hd4tk/9fdUm8Xrgz4G7axWg/bJjAJ8p3xVRF0S9dQamXZ7g\n\tOJVzQBCPnppb05HOtNikZ8WMURp2FEoaAwS0CXtSfD1Dcf3ohE7JYWQj75SrMJJb48M2\n\tg2dMD3JHliwF8p2Qs44zFS5FZMpTLPHQw0cVvRNRq5NLpXW+KVFCKs6Hp4MEZuNhI7WE\n\tehRimKFaEThR9OpGi66Y9dcRyYJ2fbAmUVn6cxBOKEsSbTcvYP7o6nOtAWAnTH8a6aWz\n\thE7Q==","X-Gm-Message-State":"AOAM533Zhefub5rGWnwTQs66qn8zIVUwEpapVXBw4EdAcVjGFRpe2tjO\n\tsYPAYij0c2yKqOyG/uZTPFcPGA==","X-Google-Smtp-Source":"ABdhPJyPwhvckz2qYaoLTuqk3QN+Gxuo1QHzJ/ZlhsVLz4XTewX8kTXpwRjSGQFGCoWU2EOCey9meA==","X-Received":"by 2002:a2e:b889:: with SMTP id r9mr37066092ljp.92.1594303220798;\n\tThu, 09 Jul 2020 07:00:20 -0700 (PDT)","Date":"Thu, 9 Jul 2020 16:00:19 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200709140019.GQ3875643@oden.dyn.berto.se>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-18-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709084128.5316-18-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11330,"web_url":"https://patchwork.libcamera.org/comment/11330/","msgid":"<20200710123928.GT5964@pendragon.ideasonboard.com>","date":"2020-07-10T12:39:28","subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","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, Jul 09, 2020 at 10:41:25AM +0200, Jacopo Mondi wrote:\n> Instrument the ImgU component to dynamically calculate the image\n> manipulation pipeline intermediate sizes.\n> \n> To correctly configure the ImgU it is necessary to program the IF, BDS\n> and GDC sizes, which are currently fixed to the input frame size.\n> \n> The procedure used to calculate the intermediate sizes has been ported\n> from the pipe_config.py python script, available at:\n> https://github.com/intel/intel-ipu3-pipecfg\n> at revision:\n> 61e83f2f7606 (\"Add more information into README\")\n> \n> Define two structures (ImgUDevice::Pipe and ImgUdevice::PipeConfig)\n\ns/ImgUdevice/ImgUDevice/\n\n> to allow the pipeline handler to supply and retrieve configuration\n> parameters from the ImgU unit.\n\ns/ImgU unit/ImgU/\n\nImgU stands for imaging unit :-)\n\n> Finally, add a new operation to the ImgUDdevice that calculates\n\ns/ImgUDdevice/ImgUDevice/\n\n(I wouldn't be opposed to renaming that simple ImgU)\n\n> the pipe configuration parameters based on the requested input and\n> output sizes.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/imgu.cpp | 365 +++++++++++++++++++++++++++\n> src/libcamera/pipeline/ipu3/imgu.h | 20 ++\n> 2 files changed, 385 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index d7f4173d3607..d1addb7d84d1 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -7,6 +7,9 @@\n> \n> #include \"imgu.h\"\n> \n> +#include \n> +#include \n> +\n> #include \n> \n> #include \n> @@ -19,6 +22,311 @@ namespace libcamera {\n> \n> LOG_DECLARE_CATEGORY(IPU3)\n> \n> +namespace {\n> +\n> +static constexpr unsigned int FILTER_H = 4;\n> +\n> +static constexpr unsigned int IF_ALIGN_W = 2;\n> +static constexpr unsigned int IF_ALIGN_H = 4;\n> +\n> +static constexpr unsigned int BDS_ALIGN_W = 2;\n> +static constexpr unsigned int BDS_ALIGN_H = 4;\n> +\n> +static constexpr unsigned int IF_CROP_MAX_W = 40;\n> +static constexpr unsigned int IF_CROP_MAX_H = 540;\n> +\n> +static constexpr float BDS_SF_MAX = 2.5;\n> +static constexpr float BDS_SF_MIN = 1.0;\n> +static constexpr float BDS_SF_STEP = 0.03125;\n> +\n> +/* BSD scaling factors: min=1, max=2.5, step=1/32 */\n> +const std::vector bdsScalingFactors = {\n> +\t1, 1.03125, 1.0625, 1.09375, 1.125, 1.15625, 1.1875, 1.21875, 1.25,\n> +\t1.28125, 1.3125, 1.34375, 1.375, 1.40625, 1.4375, 1.46875, 1.5, 1.53125,\n> +\t1.5625, 1.59375, 1.625, 1.65625, 1.6875, 1.71875, 1.75, 1.78125, 1.8125,\n> +\t1.84375, 1.875, 1.90625, 1.9375, 1.96875, 2, 2.03125, 2.0625, 2.09375,\n> +\t2.125, 2.15625, 2.1875, 2.21875, 2.25, 2.28125, 2.3125, 2.34375, 2.375,\n> +\t2.40625, 2.4375, 2.46875, 2.5\n> +};\n> +\n> +/* GDC scaling factors: min=1, max=16, step=1/4 */\n> +const std::vector gdcScalingFactors = {\n> +\t1, 1.25, 1.5, 1.75, 2, 2.25, 2.5, 2.75, 3, 3.25, 3.5, 3.75, 4, 4.25,\n> +\t4.5, 4.75, 5, 5.25, 5.5, 5.75, 6, 6.25, 6.5, 6.75, 7, 7.25, 7.5, 7.75,\n> +\t8, 8.25, 8.5, 8.75, 9, 9.25, 9.5, 9.75, 10, 10.25, 10.5, 10.75, 11,\n> +\t11.25, 11.5, 11.75, 12, 12.25, 12.5, 12.75, 13, 13.25, 13.5, 13.75, 14,\n> +\t14.25, 14.5, 14.75, 15, 15.25, 15.5, 15.75, 16,\n> +};\n> +\n> +std::vector pipeConfigs;\n> +\n> +struct FOV {\n> +\tfloat w;\n> +\tfloat h;\n> +\n> +\tbool isLarger(const FOV &other)\n> +\t{\n> +\t\tif (w > other.w)\n> +\t\t\treturn true;\n> +\t\tif (w == other.w && h > other.h)\n> +\t\t\treturn true;\n> +\t\treturn false;\n> +\t}\n> +};\n\nCould we use the Size class instead ?\n\n> +\n> +/* Approximate a scaling factor sf to the closest one available in a range. */\n> +float findScaleFactor(float sf, const std::vector &range,\n> +\t\t bool roundDown = false)\n> +{\n> +\tif (sf <= range[0])\n> +\t\treturn range[0];\n> +\tif (sf >= range[range.size() - 1])\n> +\t\treturn range[range.size() - 1];\n> +\n> +\n> +\tfloat bestDiff = std::numeric_limits::max();\n> +\tunsigned int index = 0;\n> +\tfor (unsigned int i = 0; i < range.size(); ++i) {\n> +\t\tfloat diff = std::abs(sf - range[i]);\n> +\t\tif (diff < bestDiff) {\n> +\t\t\tbestDiff = diff;\n> +\t\t\tindex = i;\n> +\t\t}\n> +\t}\n> +\n> +\tif (roundDown && index > 0 && sf < range[index])\n> +\t\tindex--;\n> +\n> +\treturn range[index];\n> +}\n\nGiven that the bdsScalingFactors factor is equal, in Python form, to\n\n[1 + i / 32 for i in range(49)]\n\nand gdcScalingFactors to\n\n[ 1 + i / 4 for i in range(61)]\n\ndoesn't this function really round sf to a multiple of 1/32 or 1/4 with\nbound cheking ? You could drop and it do the computation in the caller,\nfor instance\n\n\tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n\nwould become\n\n\tfloat sf = utils::clamp(roundf(yuvSF * 4) / 4, 1.0, 16.0);\n\nand something similar with floorf for the BDS.\n\n> +\n> +bool isSameRatio(const Size &in, const Size &out)\n> +{\n> +\tfloat inRatio = static_cast(in.width) /\n> +\t\t\tstatic_cast(in.height);\n> +\tfloat outRatio = static_cast(out.width) /\n> +\t\t\t static_cast(out.height);\n> +\n> +\tif (std::abs(inRatio - outRatio) > 0.1)\n> +\t\treturn false;\n> +\n> +\treturn true;\n> +}\n> +\n> +unsigned int alignIncrease(unsigned int len, unsigned int align)\n> +{\n> +\tif (len % align)\n> +\t\treturn (std::floor(static_cast(len) /\n> +\t\t\t\t static_cast(align)) + 1) * align;\n\nI find it quite peculiar to go through a double for this :-)\n\nIf a helper function is needed to align, I think it should go to\nutils.h.\n\n> +\treturn len;\n> +}\n> +\n> +void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> +\t\t\tunsigned int bdsWidth, float bdsSF)\n> +{\n> +\tunsigned int minIFHeight = iif.height - IF_CROP_MAX_H;\n> +\tunsigned int minBDSHeight = gdc.height + FILTER_H * 2;\n> +\tfloat bdsHeight;\n> +\n> +\tif (!isSameRatio(pipe->input, gdc)) {\n> +\t\tbool found = false;\n> +\t\tfloat estIFHeight = static_cast(iif.width) *\n> +\t\t\t\t static_cast(gdc.height) /\n> +\t\t\t\t static_cast(gdc.width);\n> +\t\testIFHeight = utils::clamp(estIFHeight,\n> +\t\t\t\t\t\t static_cast(minIFHeight),\n> +\t\t\t\t\t\t static_cast(iif.height));\n\nAs your force the utils::clamp type to float, is the static cast needed\nfor these parameters ?\n\n> +\t\tfloat ifHeight =\n\nUnless I'm mistaken, ifHeight only takes integer values. Why is it a\nfloat ?\n\nCan I let you go through the rest of the code to simplify the\nmathematical operations for the next version ?\n\n> +\t\t\tstatic_cast(alignIncrease(static_cast(estIFHeight),\n> +\t\t\t\t\t\t\t IF_ALIGN_H));\n> +\t\twhile (ifHeight >= minIFHeight && (ifHeight / bdsSF >= minBDSHeight)) {\n> +\t\t\tbdsHeight = ifHeight / bdsSF;\n> +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> +\t\t\t\tunsigned int bdsIntHeight =\n> +\t\t\t\t\tstatic_cast(bdsHeight);\n> +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tifHeight -= IF_ALIGN_H;\n> +\t\t}\n> +\n> +\t\tifHeight = static_cast(\n> +\t\t\talignIncrease(static_cast(estIFHeight),\n> +\t\t\t\t IF_ALIGN_H));\n> +\n> +\t\twhile (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) {\n> +\t\t\tbdsHeight = ifHeight / bdsSF;\n> +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> +\t\t\t\tunsigned int bdsIntHeight =\n> +\t\t\t\t\tstatic_cast(bdsHeight);\n> +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tifHeight += IF_ALIGN_H;\n> +\t\t}\n> +\n> +\t\tif (found) {\n> +\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> +\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> +\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> +\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> +\t\t\treturn;\n> +\t\t}\n> +\t} else {\n> +\t\tfloat ifHeight = static_cast(alignIncrease(iif.height, IF_ALIGN_H));\n> +\t\twhile (ifHeight > minIFHeight && (ifHeight / bdsSF > minBDSHeight)) {\n> +\t\t\tbdsHeight = ifHeight / bdsSF;\n> +\t\t\tif (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {\n> +\t\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> +\t\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> +\n> +\t\t\t\tif (!(ifIntHeight % IF_ALIGN_H) &&\n> +\t\t\t\t !(bdsIntHeight % BDS_ALIGN_H)) {\n> +\t\t\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> +\t\t\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tifHeight -= IF_ALIGN_H;\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> +\t\t float bdsSF)\n> +{\n> +\tunsigned int minBDSWidth = gdc.width + FILTER_H * 2;\n> +\n> +\tfloat sf = bdsSF;\n> +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> +\n> +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> +\t\t}\n> +\n> +\t\tsf += BDS_SF_STEP;\n> +\t}\n> +\n> +\tsf = bdsSF;\n> +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> +\n> +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> +\t\t}\n> +\n> +\t\tsf -= BDS_SF_STEP;\n> +\t}\n> +}\n> +\n> +Size calculateGDC(ImgUDevice::Pipe *pipe)\n> +{\n> +\tconst Size &in = pipe->input;\n> +\tconst Size &main = pipe->output;\n> +\tconst Size &vf = pipe->viewfinder;\n> +\tSize gdc;\n> +\n> +\tif (!vf.isNull()) {\n> +\t\tgdc.width = main.width;\n> +\n> +\t\tfloat ratio = static_cast(main.width) *\n> +\t\t\t static_cast(vf.height) /\n> +\t\t\t static_cast(vf.width);\n> +\t\tgdc.height = std::max(static_cast(main.height), ratio);\n> +\n> +\t\treturn gdc;\n> +\t}\n> +\n> +\tif (!isSameRatio(in, main)) {\n> +\t\tgdc = main;\n> +\t\treturn gdc;\n> +\t}\n> +\n> +\tfloat totalSF = static_cast(in.width) /\n> +\t\t\tstatic_cast(main.width);\n> +\tfloat bdsSF = totalSF > 2 ? 2 : 1;\n> +\tfloat yuvSF = totalSF / bdsSF;\n> +\tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n> +\n> +\tgdc.width = static_cast(main.width) * sf;\n> +\tgdc.height = static_cast(main.height) * sf;\n> +\n> +\treturn gdc;\n> +}\n> +\n> +FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)\n> +{\n> +\tFOV fov{};\n> +\n> +\tfloat inW = static_cast(in.width);\n> +\tfloat inH = static_cast(in.height);\n> +\tfloat ifCropW = static_cast(in.width) - static_cast(pipe.iif.width);\n> +\tfloat ifCropH = static_cast(in.height) - static_cast(pipe.iif.height);\n> +\tfloat gdcCropW = static_cast(pipe.bds.width - pipe.gdc.width) * pipe.bds_sf;\n> +\tfloat gdcCropH = static_cast(pipe.bds.height - pipe.gdc.height) * pipe.bds_sf;\n> +\tfov.w = (inW - (ifCropW + gdcCropW)) / inW;\n> +\tfov.h = (inH - (ifCropH + gdcCropH)) / inH;\n> +\n> +\treturn fov;\n> +}\n> +\n> +} /* namespace */\n> +\n> +/**\n> + * \\struct PipeConfig\n> + * \\brief The ImgU pipe configuration parameters\n> + *\n> + * The ImgU image pipeline is composed of several hardware blocks that crop\n> + * and scale the input image to obtain the desired output sizes. The\n> + * scaling/cropping operations of those components is configured though the\n> + * V4L2 selection API and the V4L2 subdev API applied to the ImgU media entity.\n> + *\n> + * The configurable components in the pipeline are:\n> + * - IF: image feeder\n> + * - BDS: bayer downscaler\n> + * = GDC: geometric distorsion correction\n> + *\n> + * The IF crop rectangle is controlled by the V4L2_SEL_TGT_CROP selection target\n> + * applied to the ImgU media entity sink pad number 0. The BDS scaler is\n> + * controlled by the V4L2_SEL_TGT_COMPOSE target on the same pad, while the GDC\n> + * output size is configured with the VIDIOC_SUBDEV_S_FMT IOCTL, again on pad\n> + * number 0.\n> + *\n> + * The PipeConfig structure collects the sizes of each of those components\n> + * plus the BDS scaling factor used to calculate the field of view\n> + * of the final images.\n> + */\n> +\n> +/**\n> + * \\struct Pipe\n> + * \\brief Describe the ImgU requested configuration\n> + *\n> + * The ImgU unit processes images through several components, which have\n> + * to be properly configured inspecting the input image size and the desired\n> + * output sizes. This structure collects the ImgU input configuration and the\n> + * requested main output and viewfinder configurations.\n> + *\n> + * \\var Pipe::input\n> + * \\brief The input image size\n> + *\n> + * \\var Pipe::output\n> + * \\brief The requested main output size\n> + *\n> + * \\var Pipe::viewfinder\n> + * \\brief The requested viewfinder size\n> + */\n> +\n> /**\n> * \\brief Initialize components of the ImgU instance\n> * \\param[in] mediaDevice The ImgU instance media device\n> @@ -74,6 +382,63 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Calculate the ImgU pipe configuration parameters\n> + * \\param[in] pipe The requested ImgU configuration\n> + * \\return An ImgUDevice::PipeConfig instance on success, an empty configuration\n> + * otherwise\n> + */\n> +ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> +{\n> +\tpipeConfigs.clear();\n> +\n> +\tLOG(IPU3, Debug) << \"Calculating pipe configuration for: \";\n> +\tLOG(IPU3, Debug) << \"input: \" << pipe->input.toString();\n> +\tLOG(IPU3, Debug) << \"main: \" << pipe->output.toString();\n> +\tLOG(IPU3, Debug) << \"vf: \" << pipe->viewfinder.toString();\n> +\n> +\tconst Size &in = pipe->input;\n> +\tSize gdc = calculateGDC(pipe);\n> +\n> +\tunsigned int ifWidth = alignIncrease(in.width, IF_ALIGN_W);\n> +\tunsigned int ifHeight = in.height;\n> +\tunsigned int minIfWidth = in.width - IF_CROP_MAX_W;\n> +\tfloat bdsSF = static_cast(in.width) /\n> +\t\t static_cast(gdc.width);\n> +\tfloat sf = findScaleFactor(bdsSF, bdsScalingFactors, true);\n> +\twhile (ifWidth >= minIfWidth) {\n> +\t\tSize iif{ ifWidth, ifHeight };\n> +\t\tcalculateBDS(pipe, iif, gdc, sf);\n> +\n> +\t\tifWidth -= IF_ALIGN_W;\n> +\t}\n> +\n> +\tif (pipeConfigs.size() == 0) {\n> +\t\tLOG(IPU3, Error) << \"Failed to calculate pipe configuration\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tFOV bestFov = calcFOV(pipe->input, pipeConfigs[0]);\n> +\tunsigned int bestIndex = 0;\n> +\tunsigned int p = 0;\n> +\tfor (auto pipeConfig : pipeConfigs) {\n> +\t\tFOV fov = calcFOV(pipe->input, pipeConfig);\n> +\t\tif (fov.isLarger(bestFov)) {\n> +\t\t\tbestFov = fov;\n> +\t\t\tbestIndex = p;\n> +\t\t}\n> +\n> +\t\t++p;\n> +\t}\n> +\n> +\tLOG(IPU3, Debug) << \"Computed pipe configuration: \";\n> +\tLOG(IPU3, Debug) << \"IF: \" << pipeConfigs[bestIndex].iif.toString();\n> +\tLOG(IPU3, Debug) << \"BDS: \" << pipeConfigs[bestIndex].bds.toString();\n> +\tLOG(IPU3, Debug) << \"GDC: \" << pipeConfigs[bestIndex].gdc.toString();\n> +\n> +\treturn pipeConfigs[bestIndex];\n> +}\n> +\n> /**\n> * \\brief Configure the ImgU unit input\n> * \\param[in] size The ImgU input frame size\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> index 5c124af2e9fe..15ee9a7f5698 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.h\n> +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> @@ -23,8 +23,28 @@ struct StreamConfiguration;\n> class ImgUDevice\n> {\n> public:\n> +\tstruct PipeConfig {\n> +\t\tfloat bds_sf;\n> +\t\tSize iif;\n> +\t\tSize bds;\n> +\t\tSize gdc;\n> +\n> +\t\tbool isNull() const\n> +\t\t{\n> +\t\t\treturn iif.isNull() || bds.isNull() || gdc.isNull();\n> +\t\t}\n> +\t};\n> +\n> +\tstruct Pipe {\n> +\t\tSize input;\n> +\t\tSize output;\n> +\t\tSize viewfinder;\n> +\t};\n> +\n> \tint init(MediaDevice *media, unsigned int index);\n> \n> +\tPipeConfig calculatePipeConfig(Pipe *pipe);\n> +\n> \tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> \n> \tint configureOutput(const StreamConfiguration &cfg,","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 3A4E9BD792\n\tfor ;\n\tFri, 10 Jul 2020 12:39:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C524B613CF;\n\tFri, 10 Jul 2020 14:39:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E0C4611BA\n\tfor ;\n\tFri, 10 Jul 2020 14:39:35 +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 E59C72C0;\n\tFri, 10 Jul 2020 14:39:34 +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=\"ZHDXEUF3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594384775;\n\tbh=39dud3eV1/6vketFwkWsAXe1+oSy3cxY/ne4qWt4k9A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZHDXEUF3+sfjAscvywKxWaInrXa+OpT2qHZ2uROO0IUxBtdqV1hSYzxSFxAM/Tz1A\n\tRFgxW+oYahcIt13dmKS20GJBBPSeR1UzggBPKeKZVLceIqh9+X5YE1bKOjxMrq+qN6\n\tozhNC6ssFRCcgmXdqd6/JJhjtAcLAPJV/os+1EUk=","Date":"Fri, 10 Jul 2020 15:39:28 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200710123928.GT5964@pendragon.ideasonboard.com>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-18-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709084128.5316-18-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","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\" "}},{"id":11357,"web_url":"https://patchwork.libcamera.org/comment/11357/","msgid":"<20200713092516.k3iqwvua6iklpygk@uno.localdomain>","date":"2020-07-13T09:25:16","subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Jul 10, 2020 at 03:39:28PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Jul 09, 2020 at 10:41:25AM +0200, Jacopo Mondi wrote:\n> > Instrument the ImgU component to dynamically calculate the image\n> > manipulation pipeline intermediate sizes.\n> >\n> > To correctly configure the ImgU it is necessary to program the IF, BDS\n> > and GDC sizes, which are currently fixed to the input frame size.\n> >\n> > The procedure used to calculate the intermediate sizes has been ported\n> > from the pipe_config.py python script, available at:\n> > https://github.com/intel/intel-ipu3-pipecfg\n> > at revision:\n> > 61e83f2f7606 (\"Add more information into README\")\n> >\n> > Define two structures (ImgUDevice::Pipe and ImgUdevice::PipeConfig)\n>\n> s/ImgUdevice/ImgUDevice/\n>\n> > to allow the pipeline handler to supply and retrieve configuration\n> > parameters from the ImgU unit.\n>\n> s/ImgU unit/ImgU/\n>\n> ImgU stands for imaging unit :-)\n>\n> > Finally, add a new operation to the ImgUDdevice that calculates\n>\n> s/ImgUDdevice/ImgUDevice/\n>\n> (I wouldn't be opposed to renaming that simple ImgU)\n>\n> > the pipe configuration parameters based on the requested input and\n> > output sizes.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/imgu.cpp | 365 +++++++++++++++++++++++++++\n> > src/libcamera/pipeline/ipu3/imgu.h | 20 ++\n> > 2 files changed, 385 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > index d7f4173d3607..d1addb7d84d1 100644\n> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > @@ -7,6 +7,9 @@\n> >\n> > #include \"imgu.h\"\n> >\n> > +#include \n> > +#include \n> > +\n> > #include \n> >\n> > #include \n> > @@ -19,6 +22,311 @@ namespace libcamera {\n> >\n> > LOG_DECLARE_CATEGORY(IPU3)\n> >\n> > +namespace {\n> > +\n> > +static constexpr unsigned int FILTER_H = 4;\n> > +\n> > +static constexpr unsigned int IF_ALIGN_W = 2;\n> > +static constexpr unsigned int IF_ALIGN_H = 4;\n> > +\n> > +static constexpr unsigned int BDS_ALIGN_W = 2;\n> > +static constexpr unsigned int BDS_ALIGN_H = 4;\n> > +\n> > +static constexpr unsigned int IF_CROP_MAX_W = 40;\n> > +static constexpr unsigned int IF_CROP_MAX_H = 540;\n> > +\n> > +static constexpr float BDS_SF_MAX = 2.5;\n> > +static constexpr float BDS_SF_MIN = 1.0;\n> > +static constexpr float BDS_SF_STEP = 0.03125;\n> > +\n> > +/* BSD scaling factors: min=1, max=2.5, step=1/32 */\n> > +const std::vector bdsScalingFactors = {\n> > +\t1, 1.03125, 1.0625, 1.09375, 1.125, 1.15625, 1.1875, 1.21875, 1.25,\n> > +\t1.28125, 1.3125, 1.34375, 1.375, 1.40625, 1.4375, 1.46875, 1.5, 1.53125,\n> > +\t1.5625, 1.59375, 1.625, 1.65625, 1.6875, 1.71875, 1.75, 1.78125, 1.8125,\n> > +\t1.84375, 1.875, 1.90625, 1.9375, 1.96875, 2, 2.03125, 2.0625, 2.09375,\n> > +\t2.125, 2.15625, 2.1875, 2.21875, 2.25, 2.28125, 2.3125, 2.34375, 2.375,\n> > +\t2.40625, 2.4375, 2.46875, 2.5\n> > +};\n> > +\n> > +/* GDC scaling factors: min=1, max=16, step=1/4 */\n> > +const std::vector gdcScalingFactors = {\n> > +\t1, 1.25, 1.5, 1.75, 2, 2.25, 2.5, 2.75, 3, 3.25, 3.5, 3.75, 4, 4.25,\n> > +\t4.5, 4.75, 5, 5.25, 5.5, 5.75, 6, 6.25, 6.5, 6.75, 7, 7.25, 7.5, 7.75,\n> > +\t8, 8.25, 8.5, 8.75, 9, 9.25, 9.5, 9.75, 10, 10.25, 10.5, 10.75, 11,\n> > +\t11.25, 11.5, 11.75, 12, 12.25, 12.5, 12.75, 13, 13.25, 13.5, 13.75, 14,\n> > +\t14.25, 14.5, 14.75, 15, 15.25, 15.5, 15.75, 16,\n> > +};\n> > +\n> > +std::vector pipeConfigs;\n> > +\n> > +struct FOV {\n> > +\tfloat w;\n> > +\tfloat h;\n> > +\n> > +\tbool isLarger(const FOV &other)\n> > +\t{\n> > +\t\tif (w > other.w)\n> > +\t\t\treturn true;\n> > +\t\tif (w == other.w && h > other.h)\n> > +\t\t\treturn true;\n> > +\t\treturn false;\n> > +\t}\n> > +};\n>\n> Could we use the Size class instead ?\n>\n\nNot sure as the operator< of that class does not match the above\ndefinition.\n\n * - A size with smaller width and smaller height is smaller.\n * - A size with smaller area is smaller.\n * - A size with smaller width is smaller.\n\nI would keep this.\n\n> > +\n> > +/* Approximate a scaling factor sf to the closest one available in a range. */\n> > +float findScaleFactor(float sf, const std::vector &range,\n> > +\t\t bool roundDown = false)\n> > +{\n> > +\tif (sf <= range[0])\n> > +\t\treturn range[0];\n> > +\tif (sf >= range[range.size() - 1])\n> > +\t\treturn range[range.size() - 1];\n> > +\n> > +\n> > +\tfloat bestDiff = std::numeric_limits::max();\n> > +\tunsigned int index = 0;\n> > +\tfor (unsigned int i = 0; i < range.size(); ++i) {\n> > +\t\tfloat diff = std::abs(sf - range[i]);\n> > +\t\tif (diff < bestDiff) {\n> > +\t\t\tbestDiff = diff;\n> > +\t\t\tindex = i;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tif (roundDown && index > 0 && sf < range[index])\n> > +\t\tindex--;\n> > +\n> > +\treturn range[index];\n> > +}\n>\n> Given that the bdsScalingFactors factor is equal, in Python form, to\n>\n> [1 + i / 32 for i in range(49)]\n>\n> and gdcScalingFactors to\n>\n> [ 1 + i / 4 for i in range(61)]\n>\n> doesn't this function really round sf to a multiple of 1/32 or 1/4 with\n> bound cheking ? You could drop and it do the computation in the caller,\n> for instance\n>\n> \tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n>\n> would become\n>\n> \tfloat sf = utils::clamp(roundf(yuvSF * 4) / 4, 1.0, 16.0);\n>\n> and something similar with floorf for the BDS.\n>\n\nI'm afraid of deviating from the tool implementation as updates we\nreceive to the script would be harder to port here, and testing the\nseveral corner cases is not trivial and incredibily time consuming for\na limited gain imho.\n\n> > +\n> > +bool isSameRatio(const Size &in, const Size &out)\n> > +{\n> > +\tfloat inRatio = static_cast(in.width) /\n> > +\t\t\tstatic_cast(in.height);\n> > +\tfloat outRatio = static_cast(out.width) /\n> > +\t\t\t static_cast(out.height);\n> > +\n> > +\tif (std::abs(inRatio - outRatio) > 0.1)\n> > +\t\treturn false;\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +unsigned int alignIncrease(unsigned int len, unsigned int align)\n> > +{\n> > +\tif (len % align)\n> > +\t\treturn (std::floor(static_cast(len) /\n> > +\t\t\t\t static_cast(align)) + 1) * align;\n>\n> I find it quite peculiar to go through a double for this :-)\n\nYes, I don't know why I though I had to use:\n double floor ( double arg );\n>\n> If a helper function is needed to align, I think it should go to\n> utils.h.\n>\n\nThis should be simple to add.\n\n> > +\treturn len;\n> > +}\n> > +\n> > +void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> > +\t\t\tunsigned int bdsWidth, float bdsSF)\n> > +{\n> > +\tunsigned int minIFHeight = iif.height - IF_CROP_MAX_H;\n> > +\tunsigned int minBDSHeight = gdc.height + FILTER_H * 2;\n> > +\tfloat bdsHeight;\n> > +\n> > +\tif (!isSameRatio(pipe->input, gdc)) {\n> > +\t\tbool found = false;\n> > +\t\tfloat estIFHeight = static_cast(iif.width) *\n> > +\t\t\t\t static_cast(gdc.height) /\n> > +\t\t\t\t static_cast(gdc.width);\n> > +\t\testIFHeight = utils::clamp(estIFHeight,\n> > +\t\t\t\t\t\t static_cast(minIFHeight),\n> > +\t\t\t\t\t\t static_cast(iif.height));\n>\n> As your force the utils::clamp type to float, is the static cast needed\n> for these parameters ?\n\nDoes the compiler implicitly cast thos integers to float ?\nI see no error messages, so I assume so. I'll fix.\n\n>\n> > +\t\tfloat ifHeight =\n>\n> Unless I'm mistaken, ifHeight only takes integer values. Why is it a\n> float ?\n\nBecause of\n\n (ifHeight / bdsSF >= minBDSHeight)) {\n\nAnd I've been multiple time bitten by roudings due to divisions by\nintegers having a different result than division by integers while\nporting the computations from python to C++.\n\n\n> Can I let you go through the rest of the code to simplify the\n> mathematical operations for the next version ?\n>\n\nI'll see what I can do, but I won't push it too far, as subtle changes\ncould lead to different results, I would like to have this as close as\npossible to the script to ease back-porting updates, and testing the\nseveral corner cases and comparing it with the script result (as I\ncan't use the .xml file) is incredibly time consuming.\n\n> > +\t\t\tstatic_cast(alignIncrease(static_cast(estIFHeight),\n> > +\t\t\t\t\t\t\t IF_ALIGN_H));\n> > +\t\twhile (ifHeight >= minIFHeight && (ifHeight / bdsSF >= minBDSHeight)) {\n> > +\t\t\tbdsHeight = ifHeight / bdsSF;\n> > +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> > +\t\t\t\tunsigned int bdsIntHeight =\n> > +\t\t\t\t\tstatic_cast(bdsHeight);\n> > +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> > +\t\t\t\t\tfound = true;\n> > +\t\t\t\t\tbreak;\n> > +\t\t\t\t}\n> > +\t\t\t}\n> > +\n> > +\t\t\tifHeight -= IF_ALIGN_H;\n> > +\t\t}\n> > +\n> > +\t\tifHeight = static_cast(\n> > +\t\t\talignIncrease(static_cast(estIFHeight),\n> > +\t\t\t\t IF_ALIGN_H));\n> > +\n> > +\t\twhile (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) {\n> > +\t\t\tbdsHeight = ifHeight / bdsSF;\n> > +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> > +\t\t\t\tunsigned int bdsIntHeight =\n> > +\t\t\t\t\tstatic_cast(bdsHeight);\n> > +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> > +\t\t\t\t\tfound = true;\n> > +\t\t\t\t\tbreak;\n> > +\t\t\t\t}\n> > +\t\t\t}\n> > +\n> > +\t\t\tifHeight += IF_ALIGN_H;\n> > +\t\t}\n> > +\n> > +\t\tif (found) {\n> > +\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> > +\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> > +\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> > +\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\t} else {\n> > +\t\tfloat ifHeight = static_cast(alignIncrease(iif.height, IF_ALIGN_H));\n> > +\t\twhile (ifHeight > minIFHeight && (ifHeight / bdsSF > minBDSHeight)) {\n> > +\t\t\tbdsHeight = ifHeight / bdsSF;\n> > +\t\t\tif (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {\n> > +\t\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> > +\t\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> > +\n> > +\t\t\t\tif (!(ifIntHeight % IF_ALIGN_H) &&\n> > +\t\t\t\t !(bdsIntHeight % BDS_ALIGN_H)) {\n> > +\t\t\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> > +\t\t\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> > +\t\t\t\t}\n> > +\t\t\t}\n> > +\n> > +\t\t\tifHeight -= IF_ALIGN_H;\n> > +\t\t}\n> > +\t}\n> > +}\n> > +\n> > +void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> > +\t\t float bdsSF)\n> > +{\n> > +\tunsigned int minBDSWidth = gdc.width + FILTER_H * 2;\n> > +\n> > +\tfloat sf = bdsSF;\n> > +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> > +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> > +\n> > +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> > +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> > +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> > +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> > +\t\t}\n> > +\n> > +\t\tsf += BDS_SF_STEP;\n> > +\t}\n> > +\n> > +\tsf = bdsSF;\n> > +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> > +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> > +\n> > +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> > +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> > +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> > +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> > +\t\t}\n> > +\n> > +\t\tsf -= BDS_SF_STEP;\n> > +\t}\n> > +}\n> > +\n> > +Size calculateGDC(ImgUDevice::Pipe *pipe)\n> > +{\n> > +\tconst Size &in = pipe->input;\n> > +\tconst Size &main = pipe->output;\n> > +\tconst Size &vf = pipe->viewfinder;\n> > +\tSize gdc;\n> > +\n> > +\tif (!vf.isNull()) {\n> > +\t\tgdc.width = main.width;\n> > +\n> > +\t\tfloat ratio = static_cast(main.width) *\n> > +\t\t\t static_cast(vf.height) /\n> > +\t\t\t static_cast(vf.width);\n> > +\t\tgdc.height = std::max(static_cast(main.height), ratio);\n> > +\n> > +\t\treturn gdc;\n> > +\t}\n> > +\n> > +\tif (!isSameRatio(in, main)) {\n> > +\t\tgdc = main;\n> > +\t\treturn gdc;\n> > +\t}\n> > +\n> > +\tfloat totalSF = static_cast(in.width) /\n> > +\t\t\tstatic_cast(main.width);\n> > +\tfloat bdsSF = totalSF > 2 ? 2 : 1;\n> > +\tfloat yuvSF = totalSF / bdsSF;\n> > +\tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n> > +\n> > +\tgdc.width = static_cast(main.width) * sf;\n> > +\tgdc.height = static_cast(main.height) * sf;\n> > +\n> > +\treturn gdc;\n> > +}\n> > +\n> > +FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)\n> > +{\n> > +\tFOV fov{};\n> > +\n> > +\tfloat inW = static_cast(in.width);\n> > +\tfloat inH = static_cast(in.height);\n> > +\tfloat ifCropW = static_cast(in.width) - static_cast(pipe.iif.width);\n> > +\tfloat ifCropH = static_cast(in.height) - static_cast(pipe.iif.height);\n> > +\tfloat gdcCropW = static_cast(pipe.bds.width - pipe.gdc.width) * pipe.bds_sf;\n> > +\tfloat gdcCropH = static_cast(pipe.bds.height - pipe.gdc.height) * pipe.bds_sf;\n> > +\tfov.w = (inW - (ifCropW + gdcCropW)) / inW;\n> > +\tfov.h = (inH - (ifCropH + gdcCropH)) / inH;\n> > +\n> > +\treturn fov;\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> > +/**\n> > + * \\struct PipeConfig\n> > + * \\brief The ImgU pipe configuration parameters\n> > + *\n> > + * The ImgU image pipeline is composed of several hardware blocks that crop\n> > + * and scale the input image to obtain the desired output sizes. The\n> > + * scaling/cropping operations of those components is configured though the\n> > + * V4L2 selection API and the V4L2 subdev API applied to the ImgU media entity.\n> > + *\n> > + * The configurable components in the pipeline are:\n> > + * - IF: image feeder\n> > + * - BDS: bayer downscaler\n> > + * = GDC: geometric distorsion correction\n> > + *\n> > + * The IF crop rectangle is controlled by the V4L2_SEL_TGT_CROP selection target\n> > + * applied to the ImgU media entity sink pad number 0. The BDS scaler is\n> > + * controlled by the V4L2_SEL_TGT_COMPOSE target on the same pad, while the GDC\n> > + * output size is configured with the VIDIOC_SUBDEV_S_FMT IOCTL, again on pad\n> > + * number 0.\n> > + *\n> > + * The PipeConfig structure collects the sizes of each of those components\n> > + * plus the BDS scaling factor used to calculate the field of view\n> > + * of the final images.\n> > + */\n> > +\n> > +/**\n> > + * \\struct Pipe\n> > + * \\brief Describe the ImgU requested configuration\n> > + *\n> > + * The ImgU unit processes images through several components, which have\n> > + * to be properly configured inspecting the input image size and the desired\n> > + * output sizes. This structure collects the ImgU input configuration and the\n> > + * requested main output and viewfinder configurations.\n> > + *\n> > + * \\var Pipe::input\n> > + * \\brief The input image size\n> > + *\n> > + * \\var Pipe::output\n> > + * \\brief The requested main output size\n> > + *\n> > + * \\var Pipe::viewfinder\n> > + * \\brief The requested viewfinder size\n> > + */\n> > +\n> > /**\n> > * \\brief Initialize components of the ImgU instance\n> > * \\param[in] mediaDevice The ImgU instance media device\n> > @@ -74,6 +382,63 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> > \treturn 0;\n> > }\n> >\n> > +/**\n> > + * \\brief Calculate the ImgU pipe configuration parameters\n> > + * \\param[in] pipe The requested ImgU configuration\n> > + * \\return An ImgUDevice::PipeConfig instance on success, an empty configuration\n> > + * otherwise\n> > + */\n> > +ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> > +{\n> > +\tpipeConfigs.clear();\n> > +\n> > +\tLOG(IPU3, Debug) << \"Calculating pipe configuration for: \";\n> > +\tLOG(IPU3, Debug) << \"input: \" << pipe->input.toString();\n> > +\tLOG(IPU3, Debug) << \"main: \" << pipe->output.toString();\n> > +\tLOG(IPU3, Debug) << \"vf: \" << pipe->viewfinder.toString();\n> > +\n> > +\tconst Size &in = pipe->input;\n> > +\tSize gdc = calculateGDC(pipe);\n> > +\n> > +\tunsigned int ifWidth = alignIncrease(in.width, IF_ALIGN_W);\n> > +\tunsigned int ifHeight = in.height;\n> > +\tunsigned int minIfWidth = in.width - IF_CROP_MAX_W;\n> > +\tfloat bdsSF = static_cast(in.width) /\n> > +\t\t static_cast(gdc.width);\n> > +\tfloat sf = findScaleFactor(bdsSF, bdsScalingFactors, true);\n> > +\twhile (ifWidth >= minIfWidth) {\n> > +\t\tSize iif{ ifWidth, ifHeight };\n> > +\t\tcalculateBDS(pipe, iif, gdc, sf);\n> > +\n> > +\t\tifWidth -= IF_ALIGN_W;\n> > +\t}\n> > +\n> > +\tif (pipeConfigs.size() == 0) {\n> > +\t\tLOG(IPU3, Error) << \"Failed to calculate pipe configuration\";\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\tFOV bestFov = calcFOV(pipe->input, pipeConfigs[0]);\n> > +\tunsigned int bestIndex = 0;\n> > +\tunsigned int p = 0;\n> > +\tfor (auto pipeConfig : pipeConfigs) {\n> > +\t\tFOV fov = calcFOV(pipe->input, pipeConfig);\n> > +\t\tif (fov.isLarger(bestFov)) {\n> > +\t\t\tbestFov = fov;\n> > +\t\t\tbestIndex = p;\n> > +\t\t}\n> > +\n> > +\t\t++p;\n> > +\t}\n> > +\n> > +\tLOG(IPU3, Debug) << \"Computed pipe configuration: \";\n> > +\tLOG(IPU3, Debug) << \"IF: \" << pipeConfigs[bestIndex].iif.toString();\n> > +\tLOG(IPU3, Debug) << \"BDS: \" << pipeConfigs[bestIndex].bds.toString();\n> > +\tLOG(IPU3, Debug) << \"GDC: \" << pipeConfigs[bestIndex].gdc.toString();\n> > +\n> > +\treturn pipeConfigs[bestIndex];\n> > +}\n> > +\n> > /**\n> > * \\brief Configure the ImgU unit input\n> > * \\param[in] size The ImgU input frame size\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> > index 5c124af2e9fe..15ee9a7f5698 100644\n> > --- a/src/libcamera/pipeline/ipu3/imgu.h\n> > +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> > @@ -23,8 +23,28 @@ struct StreamConfiguration;\n> > class ImgUDevice\n> > {\n> > public:\n> > +\tstruct PipeConfig {\n> > +\t\tfloat bds_sf;\n> > +\t\tSize iif;\n> > +\t\tSize bds;\n> > +\t\tSize gdc;\n> > +\n> > +\t\tbool isNull() const\n> > +\t\t{\n> > +\t\t\treturn iif.isNull() || bds.isNull() || gdc.isNull();\n> > +\t\t}\n> > +\t};\n> > +\n> > +\tstruct Pipe {\n> > +\t\tSize input;\n> > +\t\tSize output;\n> > +\t\tSize viewfinder;\n> > +\t};\n> > +\n> > \tint init(MediaDevice *media, unsigned int index);\n> >\n> > +\tPipeConfig calculatePipeConfig(Pipe *pipe);\n> > +\n> > \tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> >\n> > \tint configureOutput(const StreamConfiguration &cfg,\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 06910BD792\n\tfor ;\n\tMon, 13 Jul 2020 09:21:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F9C4605BC;\n\tMon, 13 Jul 2020 11:21:45 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E76F605B1\n\tfor ;\n\tMon, 13 Jul 2020 11:21:44 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id C5B69100004;\n\tMon, 13 Jul 2020 09:21:43 +0000 (UTC)"],"Date":"Mon, 13 Jul 2020 11:25:16 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200713092516.k3iqwvua6iklpygk@uno.localdomain>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-18-jacopo@jmondi.org>\n\t<20200710123928.GT5964@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200710123928.GT5964@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","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\" "}},{"id":11377,"web_url":"https://patchwork.libcamera.org/comment/11377/","msgid":"<20200713224335.GD25374@pendragon.ideasonboard.com>","date":"2020-07-13T22:43:35","subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jul 13, 2020 at 11:25:16AM +0200, Jacopo Mondi wrote:\n> On Fri, Jul 10, 2020 at 03:39:28PM +0300, Laurent Pinchart wrote:\n> > On Thu, Jul 09, 2020 at 10:41:25AM +0200, Jacopo Mondi wrote:\n> > > Instrument the ImgU component to dynamically calculate the image\n> > > manipulation pipeline intermediate sizes.\n> > >\n> > > To correctly configure the ImgU it is necessary to program the IF, BDS\n> > > and GDC sizes, which are currently fixed to the input frame size.\n> > >\n> > > The procedure used to calculate the intermediate sizes has been ported\n> > > from the pipe_config.py python script, available at:\n> > > https://github.com/intel/intel-ipu3-pipecfg\n> > > at revision:\n> > > 61e83f2f7606 (\"Add more information into README\")\n> > >\n> > > Define two structures (ImgUDevice::Pipe and ImgUdevice::PipeConfig)\n> >\n> > s/ImgUdevice/ImgUDevice/\n> >\n> > > to allow the pipeline handler to supply and retrieve configuration\n> > > parameters from the ImgU unit.\n> >\n> > s/ImgU unit/ImgU/\n> >\n> > ImgU stands for imaging unit :-)\n> >\n> > > Finally, add a new operation to the ImgUDdevice that calculates\n> >\n> > s/ImgUDdevice/ImgUDevice/\n> >\n> > (I wouldn't be opposed to renaming that simple ImgU)\n> >\n> > > the pipe configuration parameters based on the requested input and\n> > > output sizes.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline/ipu3/imgu.cpp | 365 +++++++++++++++++++++++++++\n> > > src/libcamera/pipeline/ipu3/imgu.h | 20 ++\n> > > 2 files changed, 385 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > > index d7f4173d3607..d1addb7d84d1 100644\n> > > --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > > @@ -7,6 +7,9 @@\n> > >\n> > > #include \"imgu.h\"\n> > >\n> > > +#include \n> > > +#include \n> > > +\n> > > #include \n> > >\n> > > #include \n> > > @@ -19,6 +22,311 @@ namespace libcamera {\n> > >\n> > > LOG_DECLARE_CATEGORY(IPU3)\n> > >\n> > > +namespace {\n> > > +\n> > > +static constexpr unsigned int FILTER_H = 4;\n> > > +\n> > > +static constexpr unsigned int IF_ALIGN_W = 2;\n> > > +static constexpr unsigned int IF_ALIGN_H = 4;\n> > > +\n> > > +static constexpr unsigned int BDS_ALIGN_W = 2;\n> > > +static constexpr unsigned int BDS_ALIGN_H = 4;\n> > > +\n> > > +static constexpr unsigned int IF_CROP_MAX_W = 40;\n> > > +static constexpr unsigned int IF_CROP_MAX_H = 540;\n> > > +\n> > > +static constexpr float BDS_SF_MAX = 2.5;\n> > > +static constexpr float BDS_SF_MIN = 1.0;\n> > > +static constexpr float BDS_SF_STEP = 0.03125;\n> > > +\n> > > +/* BSD scaling factors: min=1, max=2.5, step=1/32 */\n> > > +const std::vector bdsScalingFactors = {\n> > > +\t1, 1.03125, 1.0625, 1.09375, 1.125, 1.15625, 1.1875, 1.21875, 1.25,\n> > > +\t1.28125, 1.3125, 1.34375, 1.375, 1.40625, 1.4375, 1.46875, 1.5, 1.53125,\n> > > +\t1.5625, 1.59375, 1.625, 1.65625, 1.6875, 1.71875, 1.75, 1.78125, 1.8125,\n> > > +\t1.84375, 1.875, 1.90625, 1.9375, 1.96875, 2, 2.03125, 2.0625, 2.09375,\n> > > +\t2.125, 2.15625, 2.1875, 2.21875, 2.25, 2.28125, 2.3125, 2.34375, 2.375,\n> > > +\t2.40625, 2.4375, 2.46875, 2.5\n> > > +};\n> > > +\n> > > +/* GDC scaling factors: min=1, max=16, step=1/4 */\n> > > +const std::vector gdcScalingFactors = {\n> > > +\t1, 1.25, 1.5, 1.75, 2, 2.25, 2.5, 2.75, 3, 3.25, 3.5, 3.75, 4, 4.25,\n> > > +\t4.5, 4.75, 5, 5.25, 5.5, 5.75, 6, 6.25, 6.5, 6.75, 7, 7.25, 7.5, 7.75,\n> > > +\t8, 8.25, 8.5, 8.75, 9, 9.25, 9.5, 9.75, 10, 10.25, 10.5, 10.75, 11,\n> > > +\t11.25, 11.5, 11.75, 12, 12.25, 12.5, 12.75, 13, 13.25, 13.5, 13.75, 14,\n> > > +\t14.25, 14.5, 14.75, 15, 15.25, 15.5, 15.75, 16,\n> > > +};\n> > > +\n> > > +std::vector pipeConfigs;\n> > > +\n> > > +struct FOV {\n> > > +\tfloat w;\n> > > +\tfloat h;\n> > > +\n> > > +\tbool isLarger(const FOV &other)\n> > > +\t{\n> > > +\t\tif (w > other.w)\n> > > +\t\t\treturn true;\n> > > +\t\tif (w == other.w && h > other.h)\n> > > +\t\t\treturn true;\n> > > +\t\treturn false;\n> > > +\t}\n> > > +};\n> >\n> > Could we use the Size class instead ?\n> \n> Not sure as the operator< of that class does not match the above\n> definition.\n> \n> * - A size with smaller width and smaller height is smaller.\n> * - A size with smaller area is smaller.\n> * - A size with smaller width is smaller.\n> \n> I would keep this.\n\nWe could use Size and have a custom comparison function though. If you\nprefer keeping this structure, I would recommend renaming it to\nFieldOfView, renaming w/h to width and height, and replacing isLarger()\nwith comparison operators. Could be also use integers instead of floats\n?\n\n> > > +\n> > > +/* Approximate a scaling factor sf to the closest one available in a range. */\n> > > +float findScaleFactor(float sf, const std::vector &range,\n> > > +\t\t bool roundDown = false)\n> > > +{\n> > > +\tif (sf <= range[0])\n> > > +\t\treturn range[0];\n> > > +\tif (sf >= range[range.size() - 1])\n> > > +\t\treturn range[range.size() - 1];\n> > > +\n> > > +\n> > > +\tfloat bestDiff = std::numeric_limits::max();\n> > > +\tunsigned int index = 0;\n> > > +\tfor (unsigned int i = 0; i < range.size(); ++i) {\n> > > +\t\tfloat diff = std::abs(sf - range[i]);\n> > > +\t\tif (diff < bestDiff) {\n> > > +\t\t\tbestDiff = diff;\n> > > +\t\t\tindex = i;\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > > +\tif (roundDown && index > 0 && sf < range[index])\n> > > +\t\tindex--;\n> > > +\n> > > +\treturn range[index];\n> > > +}\n> >\n> > Given that the bdsScalingFactors factor is equal, in Python form, to\n> >\n> > [1 + i / 32 for i in range(49)]\n> >\n> > and gdcScalingFactors to\n> >\n> > [ 1 + i / 4 for i in range(61)]\n> >\n> > doesn't this function really round sf to a multiple of 1/32 or 1/4 with\n> > bound cheking ? You could drop and it do the computation in the caller,\n> > for instance\n> >\n> > \tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n> >\n> > would become\n> >\n> > \tfloat sf = utils::clamp(roundf(yuvSF * 4) / 4, 1.0, 16.0);\n> >\n> > and something similar with floorf for the BDS.\n> \n> I'm afraid of deviating from the tool implementation as updates we\n> receive to the script would be harder to port here, and testing the\n> several corner cases is not trivial and incredibily time consuming for\n> a limited gain imho.\n\nI wonder if we should expect updates :-) The python tool is really a\nstop-gap measure to overcome the lack of documentation. I don't think\nit's meant as a tool that will be maintained, and I believe the\nauthoritative open-source implementation of the ImgU pipeline\nconfiguration will be ours, in the pipeline handler. I would thus rather\ntry to make it as clean as possible, to improve readability, and give us\na better understanding of the code.\n\n> > > +\n> > > +bool isSameRatio(const Size &in, const Size &out)\n> > > +{\n> > > +\tfloat inRatio = static_cast(in.width) /\n> > > +\t\t\tstatic_cast(in.height);\n> > > +\tfloat outRatio = static_cast(out.width) /\n> > > +\t\t\t static_cast(out.height);\n> > > +\n> > > +\tif (std::abs(inRatio - outRatio) > 0.1)\n> > > +\t\treturn false;\n> > > +\n> > > +\treturn true;\n> > > +}\n> > > +\n> > > +unsigned int alignIncrease(unsigned int len, unsigned int align)\n> > > +{\n> > > +\tif (len % align)\n> > > +\t\treturn (std::floor(static_cast(len) /\n> > > +\t\t\t\t static_cast(align)) + 1) * align;\n> >\n> > I find it quite peculiar to go through a double for this :-)\n> \n> Yes, I don't know why I though I had to use:\n> double floor ( double arg );\n>\n> > If a helper function is needed to align, I think it should go to\n> > utils.h.\n> \n> This should be simple to add.\n\nName bikeshedding aside of course :-) But yes, it shouldn't be hard.\n\n> > > +\treturn len;\n> > > +}\n> > > +\n> > > +void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> > > +\t\t\tunsigned int bdsWidth, float bdsSF)\n> > > +{\n> > > +\tunsigned int minIFHeight = iif.height - IF_CROP_MAX_H;\n> > > +\tunsigned int minBDSHeight = gdc.height + FILTER_H * 2;\n> > > +\tfloat bdsHeight;\n> > > +\n> > > +\tif (!isSameRatio(pipe->input, gdc)) {\n> > > +\t\tbool found = false;\n> > > +\t\tfloat estIFHeight = static_cast(iif.width) *\n> > > +\t\t\t\t static_cast(gdc.height) /\n> > > +\t\t\t\t static_cast(gdc.width);\n> > > +\t\testIFHeight = utils::clamp(estIFHeight,\n> > > +\t\t\t\t\t\t static_cast(minIFHeight),\n> > > +\t\t\t\t\t\t static_cast(iif.height));\n> >\n> > As your force the utils::clamp type to float, is the static cast needed\n> > for these parameters ?\n> \n> Does the compiler implicitly cast thos integers to float ?\n> I see no error messages, so I assume so. I'll fix.\n\nWhen you force T = float with clamp, all arguments to the\nfunction are floats. The compiler thus implicitly converts the arguments\nto floats, I don't think a cast is needed. If you didn't force the type,\nand if the arguments were of different types, the compiler would likely\ncomplain that it wouldn't know which type to pick.\n\n> >\n> > > +\t\tfloat ifHeight =\n> >\n> > Unless I'm mistaken, ifHeight only takes integer values. Why is it a\n> > float ?\n> \n> Because of\n> \n> (ifHeight / bdsSF >= minBDSHeight)) {\n> \n> And I've been multiple time bitten by roudings due to divisions by\n> integers having a different result than division by integers while\n> porting the computations from python to C++.\n\nIsn't it equivalent in this case though ? The comparison uses >=, and\nthe round down caused by integer division would thus be covered by the =\ncase. Furthermore, bdsSF is a float, so the result of ifHeight / bdsSF\nis also a float\n(https://en.cppreference.com/w/cpp/language/operator_arithmetic#Conversions).\nUp to you.\n\n> > Can I let you go through the rest of the code to simplify the\n> > mathematical operations for the next version ?\n> \n> I'll see what I can do, but I won't push it too far, as subtle changes\n> could lead to different results, I would like to have this as close as\n> possible to the script to ease back-porting updates, and testing the\n> several corner cases and comparing it with the script result (as I\n> can't use the .xml file) is incredibly time consuming.\n\nAs stated above, I think that comparing the results of this initial\nimplementation with the output of the script is useful, but backporting\nwon't be the usual case. We'll have better result from code that we\nunderstand.\n\n> > > +\t\t\tstatic_cast(alignIncrease(static_cast(estIFHeight),\n> > > +\t\t\t\t\t\t\t IF_ALIGN_H));\n> > > +\t\twhile (ifHeight >= minIFHeight && (ifHeight / bdsSF >= minBDSHeight)) {\n> > > +\t\t\tbdsHeight = ifHeight / bdsSF;\n> > > +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> > > +\t\t\t\tunsigned int bdsIntHeight =\n> > > +\t\t\t\t\tstatic_cast(bdsHeight);\n> > > +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> > > +\t\t\t\t\tfound = true;\n> > > +\t\t\t\t\tbreak;\n> > > +\t\t\t\t}\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\tifHeight -= IF_ALIGN_H;\n> > > +\t\t}\n> > > +\n> > > +\t\tifHeight = static_cast(\n> > > +\t\t\talignIncrease(static_cast(estIFHeight),\n> > > +\t\t\t\t IF_ALIGN_H));\n> > > +\n> > > +\t\twhile (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) {\n> > > +\t\t\tbdsHeight = ifHeight / bdsSF;\n> > > +\t\t\tif (std::fmod(bdsHeight, 1.0) == 0) {\n> > > +\t\t\t\tunsigned int bdsIntHeight =\n> > > +\t\t\t\t\tstatic_cast(bdsHeight);\n> > > +\t\t\t\tif (!(bdsIntHeight % BDS_ALIGN_H)) {\n> > > +\t\t\t\t\tfound = true;\n> > > +\t\t\t\t\tbreak;\n> > > +\t\t\t\t}\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\tifHeight += IF_ALIGN_H;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (found) {\n> > > +\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> > > +\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> > > +\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> > > +\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> > > +\t\t\treturn;\n> > > +\t\t}\n> > > +\t} else {\n> > > +\t\tfloat ifHeight = static_cast(alignIncrease(iif.height, IF_ALIGN_H));\n> > > +\t\twhile (ifHeight > minIFHeight && (ifHeight / bdsSF > minBDSHeight)) {\n> > > +\t\t\tbdsHeight = ifHeight / bdsSF;\n> > > +\t\t\tif (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {\n> > > +\t\t\t\tunsigned int ifIntHeight = static_cast(ifHeight);\n> > > +\t\t\t\tunsigned int bdsIntHeight = static_cast(bdsHeight);\n> > > +\n> > > +\t\t\t\tif (!(ifIntHeight % IF_ALIGN_H) &&\n> > > +\t\t\t\t !(bdsIntHeight % BDS_ALIGN_H)) {\n> > > +\t\t\t\t\tpipeConfigs.push_back({ bdsSF, { iif.width, ifIntHeight },\n> > > +\t\t\t\t\t\t\t\t{ bdsWidth, bdsIntHeight }, gdc });\n> > > +\t\t\t\t}\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\tifHeight -= IF_ALIGN_H;\n> > > +\t\t}\n> > > +\t}\n> > > +}\n> > > +\n> > > +void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,\n> > > +\t\t float bdsSF)\n> > > +{\n> > > +\tunsigned int minBDSWidth = gdc.width + FILTER_H * 2;\n> > > +\n> > > +\tfloat sf = bdsSF;\n> > > +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> > > +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> > > +\n> > > +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> > > +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> > > +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> > > +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> > > +\t\t}\n> > > +\n> > > +\t\tsf += BDS_SF_STEP;\n> > > +\t}\n> > > +\n> > > +\tsf = bdsSF;\n> > > +\twhile (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {\n> > > +\t\tfloat bdsWidth = static_cast(iif.width) / sf;\n> > > +\n> > > +\t\tif (std::fmod(bdsWidth, 1.0) == 0) {\n> > > +\t\t\tunsigned int bdsIntWidth = static_cast(bdsWidth);\n> > > +\t\t\tif (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)\n> > > +\t\t\t\tcalculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);\n> > > +\t\t}\n> > > +\n> > > +\t\tsf -= BDS_SF_STEP;\n> > > +\t}\n> > > +}\n> > > +\n> > > +Size calculateGDC(ImgUDevice::Pipe *pipe)\n> > > +{\n> > > +\tconst Size &in = pipe->input;\n> > > +\tconst Size &main = pipe->output;\n> > > +\tconst Size &vf = pipe->viewfinder;\n> > > +\tSize gdc;\n> > > +\n> > > +\tif (!vf.isNull()) {\n> > > +\t\tgdc.width = main.width;\n> > > +\n> > > +\t\tfloat ratio = static_cast(main.width) *\n> > > +\t\t\t static_cast(vf.height) /\n> > > +\t\t\t static_cast(vf.width);\n> > > +\t\tgdc.height = std::max(static_cast(main.height), ratio);\n> > > +\n> > > +\t\treturn gdc;\n> > > +\t}\n> > > +\n> > > +\tif (!isSameRatio(in, main)) {\n> > > +\t\tgdc = main;\n> > > +\t\treturn gdc;\n> > > +\t}\n> > > +\n> > > +\tfloat totalSF = static_cast(in.width) /\n> > > +\t\t\tstatic_cast(main.width);\n> > > +\tfloat bdsSF = totalSF > 2 ? 2 : 1;\n> > > +\tfloat yuvSF = totalSF / bdsSF;\n> > > +\tfloat sf = findScaleFactor(yuvSF, gdcScalingFactors);\n> > > +\n> > > +\tgdc.width = static_cast(main.width) * sf;\n> > > +\tgdc.height = static_cast(main.height) * sf;\n> > > +\n> > > +\treturn gdc;\n> > > +}\n> > > +\n> > > +FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)\n> > > +{\n> > > +\tFOV fov{};\n> > > +\n> > > +\tfloat inW = static_cast(in.width);\n> > > +\tfloat inH = static_cast(in.height);\n> > > +\tfloat ifCropW = static_cast(in.width) - static_cast(pipe.iif.width);\n> > > +\tfloat ifCropH = static_cast(in.height) - static_cast(pipe.iif.height);\n> > > +\tfloat gdcCropW = static_cast(pipe.bds.width - pipe.gdc.width) * pipe.bds_sf;\n> > > +\tfloat gdcCropH = static_cast(pipe.bds.height - pipe.gdc.height) * pipe.bds_sf;\n> > > +\tfov.w = (inW - (ifCropW + gdcCropW)) / inW;\n> > > +\tfov.h = (inH - (ifCropH + gdcCropH)) / inH;\n> > > +\n> > > +\treturn fov;\n> > > +}\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > +/**\n> > > + * \\struct PipeConfig\n> > > + * \\brief The ImgU pipe configuration parameters\n> > > + *\n> > > + * The ImgU image pipeline is composed of several hardware blocks that crop\n> > > + * and scale the input image to obtain the desired output sizes. The\n> > > + * scaling/cropping operations of those components is configured though the\n> > > + * V4L2 selection API and the V4L2 subdev API applied to the ImgU media entity.\n> > > + *\n> > > + * The configurable components in the pipeline are:\n> > > + * - IF: image feeder\n> > > + * - BDS: bayer downscaler\n> > > + * = GDC: geometric distorsion correction\n> > > + *\n> > > + * The IF crop rectangle is controlled by the V4L2_SEL_TGT_CROP selection target\n> > > + * applied to the ImgU media entity sink pad number 0. The BDS scaler is\n> > > + * controlled by the V4L2_SEL_TGT_COMPOSE target on the same pad, while the GDC\n> > > + * output size is configured with the VIDIOC_SUBDEV_S_FMT IOCTL, again on pad\n> > > + * number 0.\n> > > + *\n> > > + * The PipeConfig structure collects the sizes of each of those components\n> > > + * plus the BDS scaling factor used to calculate the field of view\n> > > + * of the final images.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\struct Pipe\n> > > + * \\brief Describe the ImgU requested configuration\n> > > + *\n> > > + * The ImgU unit processes images through several components, which have\n> > > + * to be properly configured inspecting the input image size and the desired\n> > > + * output sizes. This structure collects the ImgU input configuration and the\n> > > + * requested main output and viewfinder configurations.\n> > > + *\n> > > + * \\var Pipe::input\n> > > + * \\brief The input image size\n> > > + *\n> > > + * \\var Pipe::output\n> > > + * \\brief The requested main output size\n> > > + *\n> > > + * \\var Pipe::viewfinder\n> > > + * \\brief The requested viewfinder size\n> > > + */\n> > > +\n> > > /**\n> > > * \\brief Initialize components of the ImgU instance\n> > > * \\param[in] mediaDevice The ImgU instance media device\n> > > @@ -74,6 +382,63 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> > > \treturn 0;\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Calculate the ImgU pipe configuration parameters\n> > > + * \\param[in] pipe The requested ImgU configuration\n> > > + * \\return An ImgUDevice::PipeConfig instance on success, an empty configuration\n> > > + * otherwise\n> > > + */\n> > > +ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> > > +{\n> > > +\tpipeConfigs.clear();\n> > > +\n> > > +\tLOG(IPU3, Debug) << \"Calculating pipe configuration for: \";\n> > > +\tLOG(IPU3, Debug) << \"input: \" << pipe->input.toString();\n> > > +\tLOG(IPU3, Debug) << \"main: \" << pipe->output.toString();\n> > > +\tLOG(IPU3, Debug) << \"vf: \" << pipe->viewfinder.toString();\n> > > +\n> > > +\tconst Size &in = pipe->input;\n> > > +\tSize gdc = calculateGDC(pipe);\n> > > +\n> > > +\tunsigned int ifWidth = alignIncrease(in.width, IF_ALIGN_W);\n> > > +\tunsigned int ifHeight = in.height;\n> > > +\tunsigned int minIfWidth = in.width - IF_CROP_MAX_W;\n> > > +\tfloat bdsSF = static_cast(in.width) /\n> > > +\t\t static_cast(gdc.width);\n> > > +\tfloat sf = findScaleFactor(bdsSF, bdsScalingFactors, true);\n> > > +\twhile (ifWidth >= minIfWidth) {\n> > > +\t\tSize iif{ ifWidth, ifHeight };\n> > > +\t\tcalculateBDS(pipe, iif, gdc, sf);\n> > > +\n> > > +\t\tifWidth -= IF_ALIGN_W;\n> > > +\t}\n> > > +\n> > > +\tif (pipeConfigs.size() == 0) {\n> > > +\t\tLOG(IPU3, Error) << \"Failed to calculate pipe configuration\";\n> > > +\t\treturn {};\n> > > +\t}\n> > > +\n> > > +\tFOV bestFov = calcFOV(pipe->input, pipeConfigs[0]);\n> > > +\tunsigned int bestIndex = 0;\n> > > +\tunsigned int p = 0;\n> > > +\tfor (auto pipeConfig : pipeConfigs) {\n> > > +\t\tFOV fov = calcFOV(pipe->input, pipeConfig);\n> > > +\t\tif (fov.isLarger(bestFov)) {\n> > > +\t\t\tbestFov = fov;\n> > > +\t\t\tbestIndex = p;\n> > > +\t\t}\n> > > +\n> > > +\t\t++p;\n> > > +\t}\n> > > +\n> > > +\tLOG(IPU3, Debug) << \"Computed pipe configuration: \";\n> > > +\tLOG(IPU3, Debug) << \"IF: \" << pipeConfigs[bestIndex].iif.toString();\n> > > +\tLOG(IPU3, Debug) << \"BDS: \" << pipeConfigs[bestIndex].bds.toString();\n> > > +\tLOG(IPU3, Debug) << \"GDC: \" << pipeConfigs[bestIndex].gdc.toString();\n> > > +\n> > > +\treturn pipeConfigs[bestIndex];\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Configure the ImgU unit input\n> > > * \\param[in] size The ImgU input frame size\n> > > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> > > index 5c124af2e9fe..15ee9a7f5698 100644\n> > > --- a/src/libcamera/pipeline/ipu3/imgu.h\n> > > +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> > > @@ -23,8 +23,28 @@ struct StreamConfiguration;\n> > > class ImgUDevice\n> > > {\n> > > public:\n> > > +\tstruct PipeConfig {\n> > > +\t\tfloat bds_sf;\n> > > +\t\tSize iif;\n> > > +\t\tSize bds;\n> > > +\t\tSize gdc;\n> > > +\n> > > +\t\tbool isNull() const\n> > > +\t\t{\n> > > +\t\t\treturn iif.isNull() || bds.isNull() || gdc.isNull();\n> > > +\t\t}\n> > > +\t};\n> > > +\n> > > +\tstruct Pipe {\n> > > +\t\tSize input;\n> > > +\t\tSize output;\n> > > +\t\tSize viewfinder;\n> > > +\t};\n> > > +\n> > > \tint init(MediaDevice *media, unsigned int index);\n> > >\n> > > +\tPipeConfig calculatePipeConfig(Pipe *pipe);\n> > > +\n> > > \tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> > >\n> > > \tint configureOutput(const StreamConfiguration &cfg,","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 9821EBD790\n\tfor ;\n\tMon, 13 Jul 2020 22:43:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1999E60553;\n\tTue, 14 Jul 2020 00:43:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0AC126048D\n\tfor ;\n\tTue, 14 Jul 2020 00:43:43 +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 52549C9;\n\tTue, 14 Jul 2020 00:43:42 +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=\"ZQ+Api7B\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594680222;\n\tbh=/Azjjy2oGVpiokp0l7EjhXjh7Pj17jUJMXUxLcU92tM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZQ+Api7B8GGNz/H4NxmeAwLgiBqr2V8NVEtr+jsCfGwoAynqChiHZuHhZVExRZ2q9\n\tQgBs4kJd0nupfGmJqSOR2Vp7PPXJjCa59x2nbiEs1Ffhb5aCXC+4xafPLdnWaMB4dA\n\tuxyf9TscytuXyB0+yzXwG+41dzBC1y1YFEfxWMoY=","Date":"Tue, 14 Jul 2020 01:43:35 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200713224335.GD25374@pendragon.ideasonboard.com>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-18-jacopo@jmondi.org>\n\t<20200710123928.GT5964@pendragon.ideasonboard.com>\n\t<20200713092516.k3iqwvua6iklpygk@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200713092516.k3iqwvua6iklpygk@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 17/20] libcamera: ipu3: imgu:\n\tCalculate ImgU pipe configuration","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\" "}}]