[{"id":14622,"web_url":"https://patchwork.libcamera.org/comment/14622/","msgid":"","date":"2021-01-20T09:53:08","subject":"Re: [libcamera-devel] [PATCH 2/5] pipeline: raspberrypi: Set the\n\tISP Output1 to 1/4 resolution if unused","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this patch. Just one small tidiness thing...\n\nOn Wed, 20 Jan 2021 at 08:34, Naushir Patuck wrote:\n>\n> In preparation for fast colour denoise, set the low resolution ISP\n> output stream (Output1) to a 1/4 resolution of the application requested\n> stream (Output0). This only happens if the application has not requested\n> an additional YUV or RGB stream.\n>\n> We also constrain this 1/4 resolution to at most 1200 pixels in the\n> largest dimension to avoid being too taxing on memory usage and system\n> bandwidth.\n>\n> Also switch the default StreamRole::VideoRecording to YUV420 to allow\n> fast colour denoise to run.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 42 ++++++++++++++++++-\n> 1 file changed, 41 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index e03bcb036f9f..282c4ba75d89 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -496,7 +496,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>\n> case StreamRole::VideoRecording:\n> fmts = data->isp_[Isp::Output0].dev()->formats();\n> - pixelFormat = formats::NV12;\n> + pixelFormat = formats::YUV420;\n> size = { 1920, 1080 };\n> bufferCount = 4;\n> outCount++;\n> @@ -608,6 +608,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> * StreamConfiguration appropriately.\n> */\n> V4L2DeviceFormat format;\n> + bool output1Set = false;\n> for (unsigned i = 0; i < config->size(); i++) {\n> StreamConfiguration &cfg = config->at(i);\n>\n> @@ -632,6 +633,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> format.size = cfg.size;\n> format.fourcc = fourcc;\n>\n> + LOG(RPI, Info) << \"Setting \" << stream->name() << \" to a resolution of \"\n> + << format.toString();\n> +\n> ret = stream->dev()->setFormat(&format);\n> if (ret)\n> return -EINVAL;\n> @@ -645,6 +649,42 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>\n> cfg.setStream(stream);\n> stream->setExternal(true);\n> +\n> + if (i != maxIndex)\n> + output1Set = true;\n> + }\n> +\n> + /*\n> + * If ISP::Output1 stream has not been requested by the application, we\n> + * set it up for internal use now. This second stream will be used for\n> + * fast colour denoise, and must be a quarter resolution of the ISP::Output0\n> + * stream. However, also limit the maximum size to 1200 pixels in the\n> + * larger dimension, just to avoid being wasteful with buffer allocations\n> + * and memory bandwidth.\n> + */\n> + if (!output1Set) {\n> + V4L2DeviceFormat output1Format = format;\n> + constexpr unsigned int maxDimensions = 1200;\n> +\n> + output1Format.size = format.size / 2;\n> + if (output1Format.size.width > maxDimensions) {\n> + output1Format.size.height = maxDimensions * output1Format.size.height /\n> + output1Format.size.width;\n> + output1Format.size.height &= ~1;\n> + output1Format.size.width = maxDimensions;\n> + } else if (output1Format.size.height > maxDimensions) {\n> + output1Format.size.width = maxDimensions * output1Format.size.width /\n> + output1Format.size.height;\n> + output1Format.size.width &= ~1;\n> + output1Format.size.height = maxDimensions;\n> + }\n\nI wonder if we can tidy this up with those new(ish) Size methods?\nMaybe along the lines of\n\nconst Size maxSize = Size(maxDimensions ,\nmaxDimensions).boundedToAspectRatio(format.size);\noutput1Format.size = (format.size / 2).boundedTo(maxSize).alignedDownTo(2, 2);\n\nThanks!\nDavid\n\n> +\n> + LOG(RPI, Info) << \"Setting ISP Output 1 (internal) to a resolution of \"\n> + << output1Format.toString();\n> +\n> + ret = data->isp_[Isp::Output1].dev()->setFormat(&output1Format);\n> + if (ret)\n> + return -EINVAL;\n> }\n>\n> /* ISP statistics output format. */\n> --\n> 2.25.1\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 30404BD808\n\tfor ;\n\tWed, 20 Jan 2021 09:53:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B13368182;\n\tWed, 20 Jan 2021 10:53:21 +0100 (CET)","from mail-oo1-xc29.google.com (mail-oo1-xc29.google.com\n\t[IPv6:2607:f8b0:4864:20::c29])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CDC7A68177\n\tfor ;\n\tWed, 20 Jan 2021 10:53:20 +0100 (CET)","by mail-oo1-xc29.google.com with SMTP id x203so5640520ooa.9\n\tfor ;\n\tWed, 20 Jan 2021 01:53:20 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"oZ2Q4ykg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=7dsSPFJ0/7NdOocRTGA2bLt0UvnG9dW2vQHfHJpI0SM=;\n\tb=oZ2Q4ykgPrWxEcQPwAHqHPQkKit2jfzIXBvoAUeFHolttbnr3vhR/wThkIefHiPaDF\n\tKF3PS0rCMAYtqziaOChJfiUPdyLalrgSBqcjKNCALI4i8gphoTKL+GLefFnv54vcfXda\n\t2nvfzM97K1haAU1l5Eog9Xsqucl1fYAaoNvBRuMU9339vhylcZFlpN9DP5k2cs4Js0bB\n\tQmH7hcFNATeaGIoCYLlk9eBf2w9B5iCy8QnXzFdw7KLmK6JAji5PPxhYDLAkg0mBxR3Z\n\tUP9GXWRFrE7b3FjvLwVbZHt0xWIRRXNDdNHg0QsyBhK0xxt+N09SQWeV6ygTthhTSw3i\n\t3Xjw==","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=7dsSPFJ0/7NdOocRTGA2bLt0UvnG9dW2vQHfHJpI0SM=;\n\tb=h21s40YwRdMzomh55TYWDU9CnLpifrmZ/0wuGuUCvD2MtBP99E/cHJcXe4T/Sk8dft\n\tKgP7VUFyYyUko5rFf4sHeXn40oSU9dXTU6r0cJX7CFN8/ToATmxMhSKkjMiNzLnCZ57Q\n\tN6AW+iVAIJu627PJpB6cvOz32x16cnynUHach+FPQNzMlx1e3d7Y9Mqx6rGTSzux3/w7\n\twW8oESz/BiB9+IE0lPTNhMD50Qrm2B96sU3ScQpO6wd1m/WlBXh2CD/o34e/8elhgNZ/\n\tTV+8Fix+InJ0egceikUetwF0p5V26kXR61hlaJXVHdEmg30e+5O2FXXihnpSOFNbS4ki\n\tYSEA==","X-Gm-Message-State":"AOAM532+3Ao0AXCRj0J2P2pqWfM7sA/1sNgXFrV2mp70mqBvEGFfjiC1\n\tfk4Kgj5s89umgpENYDvBuAHvrY3ahWFVu2a/VM1yCw==","X-Google-Smtp-Source":"ABdhPJzwBHhbRwRytEPiI6VJY8SiPvj5ApUUqBAWcf7EypyHye8/79LOa9r1BQkjRUD12hzuEbjhuOBw3rJOE+gkuXA=","X-Received":"by 2002:a4a:bc8d:: with SMTP id m13mr743965oop.72.1611136399689; \n\tWed, 20 Jan 2021 01:53:19 -0800 (PST)","MIME-Version":"1.0","References":"<20210120083449.642418-1-naush@raspberrypi.com>\n\t<20210120083449.642418-3-naush@raspberrypi.com>","In-Reply-To":"<20210120083449.642418-3-naush@raspberrypi.com>","From":"David Plowman ","Date":"Wed, 20 Jan 2021 09:53:08 +0000","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH 2/5] pipeline: raspberrypi: Set the\n\tISP Output1 to 1/4 resolution if unused","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14624,"web_url":"https://patchwork.libcamera.org/comment/14624/","msgid":"","date":"2021-01-20T10:00:23","subject":"Re: [libcamera-devel] [PATCH 2/5] pipeline: raspberrypi: Set the\n\tISP Output1 to 1/4 resolution if unused","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your review feedback.\n\nOn Wed, 20 Jan 2021 at 09:53, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for this patch. Just one small tidiness thing...\n>\n> On Wed, 20 Jan 2021 at 08:34, Naushir Patuck \n> wrote:\n> >\n> > In preparation for fast colour denoise, set the low resolution ISP\n> > output stream (Output1) to a 1/4 resolution of the application requested\n> > stream (Output0). This only happens if the application has not requested\n> > an additional YUV or RGB stream.\n> >\n> > We also constrain this 1/4 resolution to at most 1200 pixels in the\n> > largest dimension to avoid being too taxing on memory usage and system\n> > bandwidth.\n> >\n> > Also switch the default StreamRole::VideoRecording to YUV420 to allow\n> > fast colour denoise to run.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 42 ++++++++++++++++++-\n> > 1 file changed, 41 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index e03bcb036f9f..282c4ba75d89 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -496,7 +496,7 @@ CameraConfiguration\n> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> >\n> > case StreamRole::VideoRecording:\n> > fmts = data->isp_[Isp::Output0].dev()->formats();\n> > - pixelFormat = formats::NV12;\n> > + pixelFormat = formats::YUV420;\n> > size = { 1920, 1080 };\n> > bufferCount = 4;\n> > outCount++;\n> > @@ -608,6 +608,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > * StreamConfiguration appropriately.\n> > */\n> > V4L2DeviceFormat format;\n> > + bool output1Set = false;\n> > for (unsigned i = 0; i < config->size(); i++) {\n> > StreamConfiguration &cfg = config->at(i);\n> >\n> > @@ -632,6 +633,9 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > format.size = cfg.size;\n> > format.fourcc = fourcc;\n> >\n> > + LOG(RPI, Info) << \"Setting \" << stream->name() << \" to a\n> resolution of \"\n> > + << format.toString();\n> > +\n> > ret = stream->dev()->setFormat(&format);\n> > if (ret)\n> > return -EINVAL;\n> > @@ -645,6 +649,42 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> >\n> > cfg.setStream(stream);\n> > stream->setExternal(true);\n> > +\n> > + if (i != maxIndex)\n> > + output1Set = true;\n> > + }\n> > +\n> > + /*\n> > + * If ISP::Output1 stream has not been requested by the\n> application, we\n> > + * set it up for internal use now. This second stream will be\n> used for\n> > + * fast colour denoise, and must be a quarter resolution of the\n> ISP::Output0\n> > + * stream. However, also limit the maximum size to 1200 pixels\n> in the\n> > + * larger dimension, just to avoid being wasteful with buffer\n> allocations\n> > + * and memory bandwidth.\n> > + */\n> > + if (!output1Set) {\n> > + V4L2DeviceFormat output1Format = format;\n> > + constexpr unsigned int maxDimensions = 1200;\n> > +\n> > + output1Format.size = format.size / 2;\n> > + if (output1Format.size.width > maxDimensions) {\n> > + output1Format.size.height = maxDimensions *\n> output1Format.size.height /\n> > +\n> output1Format.size.width;\n> > + output1Format.size.height &= ~1;\n> > + output1Format.size.width = maxDimensions;\n> > + } else if (output1Format.size.height > maxDimensions) {\n> > + output1Format.size.width = maxDimensions *\n> output1Format.size.width /\n> > +\n> output1Format.size.height;\n> > + output1Format.size.width &= ~1;\n> > + output1Format.size.height = maxDimensions;\n> > + }\n>\n> I wonder if we can tidy this up with those new(ish) Size methods?\n> Maybe along the lines of\n>\n> const Size maxSize = Size(maxDimensions ,\n> maxDimensions).boundedToAspectRatio(format.size);\n> output1Format.size = (format.size / 2).boundedTo(maxSize).alignedDownTo(2,\n> 2);\n>\n\nOoh, that is much nicer! Have to admit, I didn't much explore the Size\nclass to see if this could be done.\nI will make the suggested change on the next revision.\n\nRegards,\nNaush\n\n\n\n>\n> Thanks!\n> David\n>\n> > +\n> > + LOG(RPI, Info) << \"Setting ISP Output 1 (internal) to a\n> resolution of \"\n> > + << output1Format.toString();\n> > +\n> > + ret =\n> data->isp_[Isp::Output1].dev()->setFormat(&output1Format);\n> > + if (ret)\n> > + return -EINVAL;\n> > }\n> >\n> > /* ISP statistics output format. */\n> > --\n> > 2.25.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 EBE1AC0F2A\n\tfor ;\n\tWed, 20 Jan 2021 10:00:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B9EF86818F;\n\tWed, 20 Jan 2021 11:00:42 +0100 (CET)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0EB668177\n\tfor ;\n\tWed, 20 Jan 2021 11:00:40 +0100 (CET)","by mail-lf1-x136.google.com with SMTP id m25so33280528lfc.11\n\tfor ;\n\tWed, 20 Jan 2021 02:00:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"b475miKe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=hTe0Z8rEZBLIPMUNSfAx2TU/HKutTgYrubiyUqlMoPA=;\n\tb=b475miKeGN3KT7fz4HY7aIVnhexjCfq/WQ7BlWZ5iTyr7yOW4R2KmibecgyTh8uIBl\n\tbYIvacCw3Ul/Pq+tMPJPfCmPqbOnCILP7g4/JhJ5KT3Uxy4tLdsQJAD4d44EcPp70zYs\n\tm2DvVkwhb3VMjxHWtajVBzRbfsYs1zfR7qLiZP8K90nNUR2EjyGFt3or9EiXvUaXoP/x\n\tYoQ9ck2phCVqUdgUBNHDmQvzrKsLq7d8a1yK/gjUFZwEmGwDe/+eg9gbNNyJuVhDRo/9\n\tClGW3YlS49CCLCHzPJ7PzW5PTJB/ockTdIay0icCey9thodgYCAOgPi+fsNHMD7J6Uts\n\tElWg==","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=hTe0Z8rEZBLIPMUNSfAx2TU/HKutTgYrubiyUqlMoPA=;\n\tb=r60f32xbGDQm527VyY5KpR+MvHR9MoFK6wBBVU/hOlSnY6HBue2HSnaQzRsesvppj6\n\trUbjkEZ6eKpwGw7Mrd3AT2xawiKMux9VKZPqgn6ApY/226umurqC6P9G0NyZtTMOoaTH\n\tzs2irm/h5a8bmdNpwNbWzhVgBHtAcrwK1/tmTxDCWzvL5HZrSThGP0gXMzMuIKqXqShu\n\tzxZ5ZgavndLP1xQMhwVHxlyLTwMMqigw86WAiQYbNsf3iZD/iWvr6G9TddmnLB6ybH9u\n\tA8ETRAqwio1Mjfnjt5As0ifhvibHExWIQ04UB57KgqZSBNI2lA5DF9xtfjoEKyc7JYzF\n\t+5DA==","X-Gm-Message-State":"AOAM533wFkLIe6UGrOqSC3FHdzrmuUwftVGKDkiXkmR8fFiW2IagVl5A\n\tCJ69uZrQQJMEQwvPQXZcq0ZHH4uy+G7eEh7J9Fd6egWAFko=","X-Google-Smtp-Source":"ABdhPJwOQZcMNNTL7X4twaO4mj25x/VW1JBo8cR+rhiS1wt4h4VAoVLoQ2s7mb0NuOsW3QEsTgZbLHEa9K/vH8yJMAg=","X-Received":"by 2002:a19:ee09:: with SMTP id g9mr1867392lfb.272.1611136840170;\n\tWed, 20 Jan 2021 02:00:40 -0800 (PST)","MIME-Version":"1.0","References":"<20210120083449.642418-1-naush@raspberrypi.com>\n\t<20210120083449.642418-3-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 20 Jan 2021 10:00:23 +0000","Message-ID":"","To":"David Plowman ","Subject":"Re: [libcamera-devel] [PATCH 2/5] pipeline: raspberrypi: Set the\n\tISP Output1 to 1/4 resolution if unused","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============4906279945832607424==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]