[{"id":26277,"web_url":"https://patchwork.libcamera.org/comment/26277/","msgid":"<167421200240.42371.2054313804090776400@Monstersaurus>","date":"2023-01-20T10:53:22","subject":"Re: [libcamera-devel] [PATCH v5 05/12] pipeline: raspberrypi:\n\tHandle OptionalStream hints for Unicam Image","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:46)\n> Look for OptionalStream flag in the hints field of the Unicam Image\n> StreamConfiguration structure. If this flag is not set, it guarantees that the\n> application will provide buffers for Unicam Image, so override the\n> minUnicamBuffers and minTotalUnicamBuffers config parameters in the following\n> way:\n> \n> - If startup drop frames are required, allocate at least 1 internal buffer.\n> - If no startup drop frames are required, do not allocate any internal buffers.\n> \n\nAs highlighted somewhere else, the only part I hadn't considered when\nsuggesting this way around is the raw sensor mode configuration. But\nthat might still be ok.\n\nAnd probably means we really need a helper or something to improve how\nwe handle configuring the sensor modes/sizes separately from the output\nstream, as here we would have to make sure that configuring a RAW stream\nwhen the application doesn't provide buffers will have to report it's an\nOptionalStream in the hint....\n\n\n> All other buffer allocations remain unchanged.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++++++++++-----\n> 1 file changed, 28 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 8d325c7a18c5..6ed4cc2c7ba7 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1448,12 +1448,33 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> {\n> RPiCameraData *data = cameraData(camera);\n> - unsigned int numRawBuffers = 0;\n> + unsigned int minUnicamBuffers = data->config_.minUnicamBuffers,\n> + minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers,\n> + numRawBuffers = 0;\n> int ret;\n> \n> for (Stream *s : camera->streams()) {\n> - if (isRaw(s->configuration().pixelFormat)) {\n> + if (s == &data->unicam_[Unicam::Image]) {\n> numRawBuffers = s->configuration().bufferCount;\n> + /*\n> + * If the application provides a guarantees that Unicam\n> + * image buffers will always be provided for the RAW stream\n> + * in a Request, we need:\n> + * - at least 1 internal Unicam buffer to handle startup frame drops,\n> + * - no internal Unicam buffers if there are no startup frame drops.\n> + */\n> + if (!(s->configuration().hints &\n> + StreamConfiguration::Hint::OptionalStream)) {\n> + if (data->dropFrameCount_) {\n> + minUnicamBuffers = std::max(1u, minUnicamBuffers);\n> + minTotalUnicamBuffers =\n> + std::max(minUnicamBuffers, minTotalUnicamBuffers);\n> + } else {\n> + minUnicamBuffers = 0;\n> + minTotalUnicamBuffers = 0;\n> + }\n> + }\n> +\n> break;\n> }\n> }\n> @@ -1465,7 +1486,6 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> * For Unicam, allocate a minimum number of buffers for internal\n> * use as we want to avoid any frame drops.\n> */\n> - const unsigned int minBuffers = data->config_.minTotalUnicamBuffers;\n> if (stream == &data->unicam_[Unicam::Image]) {\n> /*\n> * If an application has configured a RAW stream, allocate\n> @@ -1473,8 +1493,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> * we have at least minUnicamBuffers of internal buffers\n> * to use to minimise frame drops.\n> */\n> - numBuffers = std::max(data->config_.minUnicamBuffers,\n> - minBuffers - numRawBuffers);\n> + numBuffers = std::max(minUnicamBuffers,\n> + minTotalUnicamBuffers - numRawBuffers);\n> } else if (stream == &data->isp_[Isp::Input]) {\n> /*\n> * ISP input buffers are imported from Unicam, so follow\n> @@ -1482,15 +1502,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> * available.\n> */\n> numBuffers = numRawBuffers +\n> - std::max(data->config_.minUnicamBuffers,\n> - minBuffers - numRawBuffers);\n> + std::max(minUnicamBuffers,\n> + minTotalUnicamBuffers - numRawBuffers);\n> \n> } else if (stream == &data->unicam_[Unicam::Embedded]) {\n> /*\n> * Embedded data buffers are (currently) for internal use,\n> * so allocate the minimum required to avoid frame drops.\n> */\n> - numBuffers = minBuffers;\n> + numBuffers = data->config_.minTotalUnicamBuffers;\n> } else {\n> /*\n> * Since the ISP runs synchronous with the IPA and requests,\n> -- \n> 2.25.1\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 7F614BE175\n\tfor ;\n\tFri, 20 Jan 2023 10:53:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F34296045E;\n\tFri, 20 Jan 2023 11:53:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 650CE6045E\n\tfor ;\n\tFri, 20 Jan 2023 11:53:25 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA5FE514;\n\tFri, 20 Jan 2023 11:53:24 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674212007;\n\tbh=Ik3DA4VePhZ4JGjMl7APIISN1cXfl1ZBuxZMxs82XA0=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=DxLk/s4I5EMOqiRip5c2hzDb1uf8kUgOK1s5TwIpspdwUhqSX5Kx/7kD1+xQTFtGE\n\tzpiOOx2wAbNzPUgNaXQyLUrbMOfq5kHHZCun6lueXSzDU8kSMhWWZ2xIE3O9vlx4pN\n\tKqMcW+A4TOsy+xGrmk4tylEZVjw+CAb+7syWqjiOuJrlO00WYqedlukqoSuY0rEPc6\n\tBry5HHOw8N+LWuRyab1BH0TJbuWFu0W5Yef+mBoVercmF1ui/lo2jvKH03IUlzyhEE\n\tkZKmJOCNScnRQ9y/HWHDtGTqt3S75iZZOiIq1G7XZ9d5iRgQLgi8gTIUaSzL8BwKWe\n\to6PuSvUWM3NjA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674212005;\n\tbh=Ik3DA4VePhZ4JGjMl7APIISN1cXfl1ZBuxZMxs82XA0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=M0JILGTjyKh/eSgJktZt0Fz6BMAHgmuh6Noj9+h9wwz5vuVWdk8GRw9fikMJkBQcQ\n\tgBLRHlPFLhe4565pVNr1yG8Fi+UUsI5L692Pfnxw9QWzgG2aSTC8ofkWQ6eMtXh9C/\n\tNaSeAzRc20hlWeBLznAYjK+/Lx9Qwl1c0fWuqoDE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"M0JILGTj\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230118085953.7027-6-naush@raspberrypi.com>","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-6-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 20 Jan 2023 10:53:22 +0000","Message-ID":"<167421200240.42371.2054313804090776400@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 05/12] pipeline: raspberrypi:\n\tHandle OptionalStream hints for Unicam Image","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26313,"web_url":"https://patchwork.libcamera.org/comment/26313/","msgid":"","date":"2023-01-22T21:57:31","subject":"Re: [libcamera-devel] [PATCH v5 05/12] pipeline: raspberrypi:\n\tHandle OptionalStream hints for Unicam Image","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Jan 20, 2023 at 10:53:22AM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:46)\n> > Look for OptionalStream flag in the hints field of the Unicam Image\n> > StreamConfiguration structure. If this flag is not set, it guarantees that the\n> > application will provide buffers for Unicam Image, so override the\n> > minUnicamBuffers and minTotalUnicamBuffers config parameters in the following\n> > way:\n> > \n> > - If startup drop frames are required, allocate at least 1 internal buffer.\n> > - If no startup drop frames are required, do not allocate any internal buffers.\n> \n> As highlighted somewhere else, the only part I hadn't considered when\n> suggesting this way around is the raw sensor mode configuration. But\n> that might still be ok.\n> \n> And probably means we really need a helper or something to improve how\n> we handle configuring the sensor modes/sizes separately from the output\n> stream, as here we would have to make sure that configuring a RAW stream\n> when the application doesn't provide buffers will have to report it's an\n> OptionalStream in the hint....\n> \n> > All other buffer allocations remain unchanged.\n> > \n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++++++++++-----\n> > 1 file changed, 28 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 8d325c7a18c5..6ed4cc2c7ba7 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1448,12 +1448,33 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> > int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > {\n> > RPiCameraData *data = cameraData(camera);\n> > - unsigned int numRawBuffers = 0;\n> > + unsigned int minUnicamBuffers = data->config_.minUnicamBuffers,\n> > + minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers,\n> > + numRawBuffers = 0;\n\nOne variable per line please.\n\n> > int ret;\n> > \n> > for (Stream *s : camera->streams()) {\n> > - if (isRaw(s->configuration().pixelFormat)) {\n> > + if (s == &data->unicam_[Unicam::Image]) {\n\nIs this a functional change or do you expect it to preserve the existing\nbehaviour ? I'm wondering how it affects use cases with YUV sensors.\n\n> > numRawBuffers = s->configuration().bufferCount;\n> > + /*\n> > + * If the application provides a guarantees that Unicam\n> > + * image buffers will always be provided for the RAW stream\n> > + * in a Request, we need:\n> > + * - at least 1 internal Unicam buffer to handle startup frame drops,\n> > + * - no internal Unicam buffers if there are no startup frame drops.\n> > + */\n> > + if (!(s->configuration().hints &\n> > + StreamConfiguration::Hint::OptionalStream)) {\n> > + if (data->dropFrameCount_) {\n> > + minUnicamBuffers = std::max(1u, minUnicamBuffers);\n> > + minTotalUnicamBuffers =\n> > + std::max(minUnicamBuffers, minTotalUnicamBuffers);\n> > + } else {\n> > + minUnicamBuffers = 0;\n> > + minTotalUnicamBuffers = 0;\n> > + }\n\nWhy do you override the minimums from the configuration only when not\ndropping startup frames ? I'm not saying this is wrong, but I think\nbetter documentation is needed (likely in the example configuration file\nin patch 07/12) to explain the behaviour and how the configuration\nparameters influence it.\n\n> > + }\n> > +\n> > break;\n> > }\n> > }\n> > @@ -1465,7 +1486,6 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > * For Unicam, allocate a minimum number of buffers for internal\n> > * use as we want to avoid any frame drops.\n> > */\n> > - const unsigned int minBuffers = data->config_.minTotalUnicamBuffers;\n> > if (stream == &data->unicam_[Unicam::Image]) {\n> > /*\n> > * If an application has configured a RAW stream, allocate\n> > @@ -1473,8 +1493,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > * we have at least minUnicamBuffers of internal buffers\n> > * to use to minimise frame drops.\n> > */\n> > - numBuffers = std::max(data->config_.minUnicamBuffers,\n> > - minBuffers - numRawBuffers);\n> > + numBuffers = std::max(minUnicamBuffers,\n> > + minTotalUnicamBuffers - numRawBuffers);\n> > } else if (stream == &data->isp_[Isp::Input]) {\n> > /*\n> > * ISP input buffers are imported from Unicam, so follow\n> > @@ -1482,15 +1502,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > * available.\n> > */\n> > numBuffers = numRawBuffers +\n> > - std::max(data->config_.minUnicamBuffers,\n> > - minBuffers - numRawBuffers);\n> > + std::max(minUnicamBuffers,\n> > + minTotalUnicamBuffers - numRawBuffers);\n> > \n> > } else if (stream == &data->unicam_[Unicam::Embedded]) {\n> > /*\n> > * Embedded data buffers are (currently) for internal use,\n> > * so allocate the minimum required to avoid frame drops.\n> > */\n> > - numBuffers = minBuffers;\n> > + numBuffers = data->config_.minTotalUnicamBuffers;\n> > } else {\n> > /*\n> > * Since the ISP runs synchronous with the IPA and requests,","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 719C4BEFBE\n\tfor ;\n\tSun, 22 Jan 2023 21:57:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1275625E4;\n\tSun, 22 Jan 2023 22:57:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C734625DD\n\tfor ;\n\tSun, 22 Jan 2023 22:57:34 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 92E8A471;\n\tSun, 22 Jan 2023 22:57:33 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674424655;\n\tbh=lGjX8lR/GUtOF/LZytxfOw5GRCKY7tZ/Xt3b3PtA2Ow=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gApEuiztCiL/rbhow4lKNjFM/3T8W2WTFfnm3DJr7x45+r5yJkP24TO7v3YcijjsP\n\tq1U1M/DMczQ5Zk/mhi9PYfrzWfAZwPBuW4NULpLaN4HUWtyAoSJW7Gvb1cVZRKiH45\n\toIxZODtKPAIsO34wK45+JyBZlAQijfSNJ0/OHqJyIAtsHUJXaKz7lnbIpzvqDu2A9G\n\tRGvHQLybFA2NKSqbb1AXoYw4crq5IwSOB0lcJTXbnzwm2LYFlu0TVEo1QW7X/ygATk\n\tFuzGQ1ADmXLgIZNEQPHCvYkTM+iMIKsRb59PB598HWwcJcbNIW00lgCGasje1ArVYI\n\tACsKn+wj5cwDg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674424653;\n\tbh=lGjX8lR/GUtOF/LZytxfOw5GRCKY7tZ/Xt3b3PtA2Ow=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=npfVPGqznbhkdcvp8ld5zl+XS308kyqrUbpwtaBmFH7ZG6actjsbDXVhDsnGWcHaj\n\t7fvPKlFmpljhx/e8Kwby9lLauXq1GNLJ0Jxc27MQ9AwABuIl6G5jIVIr1gv/D1XgdZ\n\t/y5E41GJpWpuaO6m4jNtg4I0b1NiyuuOxsIug2m8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"npfVPGqz\"; dkim-atps=neutral","Date":"Sun, 22 Jan 2023 23:57:31 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-6-naush@raspberrypi.com>\n\t<167421200240.42371.2054313804090776400@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<167421200240.42371.2054313804090776400@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v5 05/12] pipeline: raspberrypi:\n\tHandle OptionalStream hints for Unicam Image","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26349,"web_url":"https://patchwork.libcamera.org/comment/26349/","msgid":"","date":"2023-01-25T12:03:55","subject":"Re: [libcamera-devel] [PATCH v5 05/12] pipeline: raspberrypi:\n\tHandle OptionalStream hints for Unicam Image","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Sun, 22 Jan 2023 at 21:57, Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Fri, Jan 20, 2023 at 10:53:22AM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:46)\n> > > Look for OptionalStream flag in the hints field of the Unicam Image\n> > > StreamConfiguration structure. If this flag is not set, it guarantees that the\n> > > application will provide buffers for Unicam Image, so override the\n> > > minUnicamBuffers and minTotalUnicamBuffers config parameters in the following\n> > > way:\n> > >\n> > > - If startup drop frames are required, allocate at least 1 internal buffer.\n> > > - If no startup drop frames are required, do not allocate any internal buffers.\n> >\n> > As highlighted somewhere else, the only part I hadn't considered when\n> > suggesting this way around is the raw sensor mode configuration. But\n> > that might still be ok.\n> >\n> > And probably means we really need a helper or something to improve how\n> > we handle configuring the sensor modes/sizes separately from the output\n> > stream, as here we would have to make sure that configuring a RAW stream\n> > when the application doesn't provide buffers will have to report it's an\n> > OptionalStream in the hint....\n> >\n> > > All other buffer allocations remain unchanged.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++++++++++-----\n> > > 1 file changed, 28 insertions(+), 8 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 8d325c7a18c5..6ed4cc2c7ba7 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -1448,12 +1448,33 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> > > int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > > {\n> > > RPiCameraData *data = cameraData(camera);\n> > > - unsigned int numRawBuffers = 0;\n> > > + unsigned int minUnicamBuffers = data->config_.minUnicamBuffers,\n> > > + minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers,\n> > > + numRawBuffers = 0;\n>\n> One variable per line please.\n\nAck.\n\n>\n> > > int ret;\n> > >\n> > > for (Stream *s : camera->streams()) {\n> > > - if (isRaw(s->configuration().pixelFormat)) {\n> > > + if (s == &data->unicam_[Unicam::Image]) {\n>\n> Is this a functional change or do you expect it to preserve the existing\n> behaviour ? I'm wondering how it affects use cases with YUV sensors\n\nSince we don't support YUV sensors in the pipeline handler (just yet), the\nbehavior here is identical. The reason for the change is to be more explicit\nwith the stream identification, which becomes important when we add a test for\nIsp::Output0 in a subsequent change.\n\n.\n>\n> > > numRawBuffers = s->configuration().bufferCount;\n> > > + /*\n> > > + * If the application provides a guarantees that Unicam\n> > > + * image buffers will always be provided for the RAW stream\n> > > + * in a Request, we need:\n> > > + * - at least 1 internal Unicam buffer to handle startup frame drops,\n> > > + * - no internal Unicam buffers if there are no startup frame drops.\n> > > + */\n> > > + if (!(s->configuration().hints &\n> > > + StreamConfiguration::Hint::OptionalStream)) {\n> > > + if (data->dropFrameCount_) {\n> > > + minUnicamBuffers = std::max(1u, minUnicamBuffers);\n> > > + minTotalUnicamBuffers =\n> > > + std::max(minUnicamBuffers, minTotalUnicamBuffers);\n> > > + } else {\n> > > + minUnicamBuffers = 0;\n> > > + minTotalUnicamBuffers = 0;\n> > > + }\n>\n> Why do you override the minimums from the configuration only when not\n> dropping startup frames ? I'm not saying this is wrong, but I think\n> better documentation is needed (likely in the example configuration file\n> in patch 07/12) to explain the behaviour and how the configuration\n> parameters influence it.\n\nThe overrides are needed to ensure we can correctly drop frames by using at\nleast one internal Unicam buffer. I will add a note the explain this behavior\nin the config file.\n\nRegards,\nNaush\n\n\n>\n> > > + }\n> > > +\n> > > break;\n> > > }\n> > > }\n> > > @@ -1465,7 +1486,6 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > > * For Unicam, allocate a minimum number of buffers for internal\n> > > * use as we want to avoid any frame drops.\n> > > */\n> > > - const unsigned int minBuffers = data->config_.minTotalUnicamBuffers;\n> > > if (stream == &data->unicam_[Unicam::Image]) {\n> > > /*\n> > > * If an application has configured a RAW stream, allocate\n> > > @@ -1473,8 +1493,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > > * we have at least minUnicamBuffers of internal buffers\n> > > * to use to minimise frame drops.\n> > > */\n> > > - numBuffers = std::max(data->config_.minUnicamBuffers,\n> > > - minBuffers - numRawBuffers);\n> > > + numBuffers = std::max(minUnicamBuffers,\n> > > + minTotalUnicamBuffers - numRawBuffers);\n> > > } else if (stream == &data->isp_[Isp::Input]) {\n> > > /*\n> > > * ISP input buffers are imported from Unicam, so follow\n> > > @@ -1482,15 +1502,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> > > * available.\n> > > */\n> > > numBuffers = numRawBuffers +\n> > > - std::max(data->config_.minUnicamBuffers,\n> > > - minBuffers - numRawBuffers);\n> > > + std::max(minUnicamBuffers,\n> > > + minTotalUnicamBuffers - numRawBuffers);\n> > >\n> > > } else if (stream == &data->unicam_[Unicam::Embedded]) {\n> > > /*\n> > > * Embedded data buffers are (currently) for internal use,\n> > > * so allocate the minimum required to avoid frame drops.\n> > > */\n> > > - numBuffers = minBuffers;\n> > > + numBuffers = data->config_.minTotalUnicamBuffers;\n> > > } else {\n> > > /*\n> > > * Since the ISP runs synchronous with the IPA and requests,\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 5B81FBDB1D\n\tfor ;\n\tWed, 25 Jan 2023 12:04:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92F46625E4;\n\tWed, 25 Jan 2023 13:04:14 +0100 (CET)","from mail-yw1-x112f.google.com (mail-yw1-x112f.google.com\n\t[IPv6:2607:f8b0:4864:20::112f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C01E603C0\n\tfor ;\n\tWed, 25 Jan 2023 13:04:13 +0100 (CET)","by mail-yw1-x112f.google.com with SMTP id\n\t00721157ae682-4fd37a1551cso235274027b3.13\n\tfor ;\n\tWed, 25 Jan 2023 04:04:13 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674648254;\n\tbh=77XjdDhBZNvYWXHxrx5lxuzwBSWDtA9dl6N5aw0LANw=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=oEBfqor1CRm1GHOAQlNlrTHSeimCFYmarXqs+PW7xdNYE1xHuvIXEgkhZZjFE7kYg\n\tej9qYp+vbUy2UY9KxWpyEkzm0oZqzakbrxSuBitSrLQZ2vVxWsRPuKjAemNuXiPHHs\n\tbsX9mitoTxEeW0fXu8sdxX/A3sBffbzE91GtKkf7ajzQ4Y+PFkd+48w9Xt0fNnfsGl\n\tCsNqYTXvBcsvIhCqnHyYJhNq2HILzRMIZAzJJEgt/2+SQ3pud0/GQnPOcqLV5hmNfn\n\txvbO7liSDMEuGy/DLI89FiR48ei5v24d/eV8sxxNk4zRjg5W3WI/126qHodjFoKyOg\n\t7XhlRa+dTmqXQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=qmq9p6VBp0FftmHycC7ivJbnxaRNqmqZg+TCp06YYNg=;\n\tb=E/gtIjpml+y14sSIIfxxk0smsdRe9HsjgMSMWmJ2U2Cmt+BSST6SjSSRBqVLN1QgLl\n\tiyMVRDEdTVTIU8AEk3cI3CMHc+GX2wobNf8eTbEwNr5uvHtovWRhYfCeQDVr8/9wh3HR\n\tKudyKCW+1gIJme0/telbZuoZeFyjZ28BOEZaKVPDgr4/TmSoIDpNuXHjTgwJOtepRFHN\n\ti8TA4x/nHQ9KCLHbvnfPpvBJQ6quI1b6UHuoLspABOqAggCrQfhDXtO2zHsJP4XGO78n\n\tp7ATm0fvDPBnwtvZ7ASuoEcNvaZjRrlUvzVeB7z4j4vu4eYWG83GEXMCeI9mlxa+FFYk\n\t4pcA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"E/gtIjpm\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=qmq9p6VBp0FftmHycC7ivJbnxaRNqmqZg+TCp06YYNg=;\n\tb=RmErDHrjI4i8sl7/oXhoDwyzDsWqKoSBse3AtjjQVSb3KPWVe0018cOJSlPyETbfgw\n\t5C6w2E3A/kjejgdgFUjy0aKboUMLh3izILR9rffL8OJw17xHORXZ3JP5Of0zexxGLcWT\n\tzAm3XxduvUovZ5xHEpCt3pWk2LhqVq/ubBT/Y0mJrQFm9dLoY0iJiV2fH8ajihWOrg6r\n\t9592IU0m7PiPsGu3/6/wq8won0AqXJfiFNe8IRzJGJjXXhSGV5sMWz9zCSqRG+z6q1V7\n\turQvNMxIhyDqppM62DdWst1GXtIEK4fVjfM2bXU4n36R72DRrLFb6EvyrUP6su1pNZg2\n\t7YxA==","X-Gm-Message-State":"AO0yUKWJMjK7Cx4C/H+XavYfNVLWqNgRf/seeepS67W+E/5qXS2iQcdI\n\tyAGJpPjEEO9qz40YH1L71iPsv4Y4DJepMCEAqPQpGhmtzB3siuW2zEI=","X-Google-Smtp-Source":"AK7set/dv904MKTvFLmXVQ97mxKNhGQo4cTa8MMBaUrVA4Vc6WxuHUWraDaHfT0avmXWS2cWyy/guEVCUdw2rniO6OY=","X-Received":"by 2002:a81:ad55:0:b0:506:644b:c30 with SMTP id\n\tl21-20020a81ad55000000b00506644b0c30mr315712ywk.109.1674648251315;\n\tWed, 25 Jan 2023 04:04:11 -0800 (PST)","MIME-Version":"1.0","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-6-naush@raspberrypi.com>\n\t<167421200240.42371.2054313804090776400@Monstersaurus>\n\t","In-Reply-To":"","Date":"Wed, 25 Jan 2023 12:03:55 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v5 05/12] pipeline: raspberrypi:\n\tHandle OptionalStream hints for Unicam Image","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]