[{"id":25969,"web_url":"https://patchwork.libcamera.org/comment/25969/","msgid":"<Y4ntTP00kycN5dNA@pendragon.ideasonboard.com>","date":"2022-12-02T12:19:24","subject":"Re: [libcamera-devel] [PATCH v2 03/10] pipeline: raspberrypi: Split\n\tout ISP Output0 buffer allocation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Nov 29, 2022 at 01:45:27PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Add a new config parameter numOutput0Buffers specifying the number of internally\n> allocated ISP Output0 buffers. This parameter defaults to 1.\n> \n> Split out the buffer allocation logic so that the buffer count for ISP Output0\n> can be different from the ISP Output1 and Statistics streams.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 21 +++++++++++++++----\n>  1 file changed, 17 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 4486d31ea78d..f2b695af2399 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -305,6 +305,11 @@ public:\n>  \t\t * the Unicam Image steram.\n>  \t\t */\n>  \t\tunsigned int minTotalUnicamBuffers;\n> +\t\t/*\n> +\t\t * The number of internal buffers allocated for the ISP Output0\n> +\t\t * stream.\n\nI'd comment here that the value can only be 0 or 1.\n\n> +\t\t */\n> +\t\tunsigned int numOutput0Buffers;\n>  \t};\n>  \n>  \tConfig config_;\n> @@ -1418,6 +1423,7 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)\n>  \tconfig = {\n>  \t\t.minUnicamBuffers = 2,\n>  \t\t.minTotalUnicamBuffers = 4,\n> +\t\t.numOutput0Buffers = 1,\n>  \t};\n>  \n>  \treturn 0;\n> @@ -1502,12 +1508,19 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n>  \t\t\t * so allocate the minimum required to avoid frame drops.\n>  \t\t\t */\n>  \t\t\tnumBuffers = minBuffers;\n> -\t\t} else {\n> +\t\t} else if (stream == &data->isp_[Isp::Output0]) {\n>  \t\t\t/*\n>  \t\t\t * Since the ISP runs synchronous with the IPA and requests,\n> -\t\t\t * we only ever need one set of internal buffers. Any buffers\n> -\t\t\t * the application wants to hold onto will already be exported\n> -\t\t\t * through PipelineHandlerRPi::exportFrameBuffers().\n> +\t\t\t * we only ever need a maximum of one internal buffer. Any\n> +\t\t\t * buffers the application wants to hold onto will already\n> +\t\t\t * be exported through PipelineHandlerRPi::exportFrameBuffers().\n> +\t\t\t */\n> +\t\t\tnumBuffers = std::min(1u, data->config_.numOutput0Buffers);\n\nIs it OK to call Stream::prepareBuffers() below if numBuffers is 0 ?\n\n> +\t\t} else {\n> +\t\t\t/*\n> +\t\t\t * Same reasoning as above, we only ever need a maximum\n> +\t\t\t * of one internal buffer for Output1 (required for colour\n> +\t\t\t * denoise) and ISP statistics.\n>  \t\t\t */\n>  \t\t\tnumBuffers = 1;\n>  \t\t}","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E5729BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Dec 2022 12:19:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6539963336;\n\tFri,  2 Dec 2022 13:19:28 +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 BBFBC60483\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Dec 2022 13:19:26 +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 2D0372D8;\n\tFri,  2 Dec 2022 13:19:26 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669983568;\n\tbh=wcThVc1BUzBPYXTPpR7QE6zYvYxisU4c3fpGqLFwgiA=;\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=K2Gj7XeU4scD86g7D+7CMCoQLtUdtXFzOY5bOb3rby+a2m8V3k/H5j7LZd/gwB6bj\n\tGcF0GDS1YiL8ea44nHm3ym1JmYt6Lo3kaMYcheZ39yGS/5Ss1QD3x21C8ec6xWyQqq\n\t+cMa5O+OqN0E2mVpyzCJNodHPfIM+SAqylFrxiW0sTp4hZV0gOKfl7iPO93lARWGuW\n\tyHI7qzlA/L80Rx21Ri741fd0Q9lAjggDZw4m1SZMrhOPyOZ9+lSWdUlgHjuWWMQdFe\n\twvr2ndUT4kTl4+C//RCfwQyOnbu4rlb+Y9PTWkZ/UdNV1FG3zAJuyTVpA5MogS1J+P\n\tIfQ5fnUKDZQFQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669983566;\n\tbh=wcThVc1BUzBPYXTPpR7QE6zYvYxisU4c3fpGqLFwgiA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p+KZdjDjVB7SrTpjiFOTbUpf0CQTo04Tcyeep970xikMyEP4POlg00YasFDrr14WQ\n\tjqucT46uGnqq6miw/H54tLg+yu5P0W9oinvB3yrHL8qNxH4r6tfejbsbnBQV2B36pS\n\tjIcaw6EiP8bColgOzxH73k8ILEA0qWisVKSFcMBI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p+KZdjDj\"; dkim-atps=neutral","Date":"Fri, 2 Dec 2022 14:19:24 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<Y4ntTP00kycN5dNA@pendragon.ideasonboard.com>","References":"<20221129134534.2933-1-naush@raspberrypi.com>\n\t<20221129134534.2933-4-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221129134534.2933-4-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 03/10] pipeline: raspberrypi: Split\n\tout ISP Output0 buffer allocation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25972,"web_url":"https://patchwork.libcamera.org/comment/25972/","msgid":"<CAEmqJPqWpGW-q28iEcCCEj4DEERFSTD52apixV_dg03gpVH65Q@mail.gmail.com>","date":"2022-12-02T13:05:54","subject":"Re: [libcamera-devel] [PATCH v2 03/10] pipeline: raspberrypi: Split\n\tout ISP Output0 buffer allocation","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 Fri, 2 Dec 2022 at 12:19, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Tue, Nov 29, 2022 at 01:45:27PM +0000, Naushir Patuck via\n> libcamera-devel wrote:\n> > Add a new config parameter numOutput0Buffers specifying the number of\n> internally\n> > allocated ISP Output0 buffers. This parameter defaults to 1.\n> >\n> > Split out the buffer allocation logic so that the buffer count for ISP\n> Output0\n> > can be different from the ISP Output1 and Statistics streams.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 21 +++++++++++++++----\n> >  1 file changed, 17 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 4486d31ea78d..f2b695af2399 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -305,6 +305,11 @@ public:\n> >                * the Unicam Image steram.\n> >                */\n> >               unsigned int minTotalUnicamBuffers;\n> > +             /*\n> > +              * The number of internal buffers allocated for the ISP\n> Output0\n> > +              * stream.\n>\n> I'd comment here that the value can only be 0 or 1.\n>\n\nAck.\n\n\n>\n> > +              */\n> > +             unsigned int numOutput0Buffers;\n> >       };\n> >\n> >       Config config_;\n> > @@ -1418,6 +1423,7 @@ int\n> PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)\n> >       config = {\n> >               .minUnicamBuffers = 2,\n> >               .minTotalUnicamBuffers = 4,\n> > +             .numOutput0Buffers = 1,\n> >       };\n> >\n> >       return 0;\n> > @@ -1502,12 +1508,19 @@ int PipelineHandlerRPi::prepareBuffers(Camera\n> *camera)\n> >                        * so allocate the minimum required to avoid frame\n> drops.\n> >                        */\n> >                       numBuffers = minBuffers;\n> > -             } else {\n> > +             } else if (stream == &data->isp_[Isp::Output0]) {\n> >                       /*\n> >                        * Since the ISP runs synchronous with the IPA and\n> requests,\n> > -                      * we only ever need one set of internal buffers.\n> Any buffers\n> > -                      * the application wants to hold onto will already\n> be exported\n> > -                      * through\n> PipelineHandlerRPi::exportFrameBuffers().\n> > +                      * we only ever need a maximum of one internal\n> buffer. Any\n> > +                      * buffers the application wants to hold onto will\n> already\n> > +                      * be exported through\n> PipelineHandlerRPi::exportFrameBuffers().\n> > +                      */\n> > +                     numBuffers = std::min(1u,\n> data->config_.numOutput0Buffers);\n>\n> Is it OK to call Stream::prepareBuffers() below if numBuffers is 0 ?\n>\n\nYes it is. The count param tells the stream how many (if any) buffers to\nallocate for internal use. Stream::prepareBuffers will call\nV4L2VideoDevice::importBuffers\nfor externally/internally allocated buffers.\n\nRegards,\nNaush\n\n\n>\n> > +             } else {\n> > +                     /*\n> > +                      * Same reasoning as above, we only ever need a\n> maximum\n> > +                      * of one internal buffer for Output1 (required\n> for colour\n> > +                      * denoise) and ISP statistics.\n> >                        */\n> >                       numBuffers = 1;\n> >               }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C421EBE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Dec 2022 13:06:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E60363336;\n\tFri,  2 Dec 2022 14:06:12 +0100 (CET)","from mail-io1-xd30.google.com (mail-io1-xd30.google.com\n\t[IPv6:2607:f8b0:4864:20::d30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C78E960483\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Dec 2022 14:06:10 +0100 (CET)","by mail-io1-xd30.google.com with SMTP id g26so2999112iob.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 02 Dec 2022 05:06:10 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669986372;\n\tbh=ztf7MiCK9tC+huotF9q4xfuepIBWkH4fjbLBypNo1hk=;\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=DYICx+Tvlxj4gLz8R8VdO9Ht9c0H+9GeDHOsa1v2APvmTpGdnpACsl35j4iqoYO39\n\t/dYMCblbLAjK05No8Hhdxzvyyj7jIH8/IVt9Tfa9GfKrP1YMB8fP/2JOVFRZGKpauq\n\t4d1obQKCXovyd4gWjAfc62KR8b1RYkfLwQX8aR41DdlI9cSqBLYIx5kIPO1nhGNAvI\n\tRywU/+uGI4pPtGaDIti2Z6LeOEXBpoE8AFGZmip7KKl9HHxNRaLsPWYRb3VqT0ZCMM\n\tr6WnEwHLEStaHxaFFn7GDqQr1zsIF0Dp6I+5C0Mqj0wEzf+PEkg72qQv9TBuDn5dhi\n\tDM04A64TUKPpA==","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=wRO0TdWto+GHpolOwWprwkvHUcfKtpDe/IoSI6Ocdps=;\n\tb=lUDOMYaNCDiBfy0gt3o5iJQSDGONcIGjrdDI/6FzrFzcIBaB1n56G16t8AyExeUtv6\n\tdlQuTbzX9hyLXsr0lbCJRG3MZmENw1GuxIEuth1BsnXYj7fpqxlSPSf8O47Brd+jz57B\n\tt2mwfYDDVLZo0X5reiflZS6v8ppAqgWKht7mwN2GkK5Cz/+ln42LNvN0aiH8G6oyfmZh\n\tKxKlFdzhyZquXSmGu9IENZdmkmVaCHr1ave8wavktdKvxdBP+cATif2uVZOX1Tki729f\n\txr84Gc/DRXcSWx+e/4ZOqUKG5ZDxFLzDx38Y3Yi+Zc74BoAhG7wLQCsL96590os8EKXJ\n\tBoCw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"lUDOMYaN\"; 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=wRO0TdWto+GHpolOwWprwkvHUcfKtpDe/IoSI6Ocdps=;\n\tb=OBddKLDYw4lPDZmJ6u14vu0IdPeCsJJWg9MbvLpOJCizfW/eYWFTUyijgfQ9nNQLOg\n\tUXBW/s8etsrlkQX5sBLKouZ8umj/wSbBUq3RhlxeJE6CysWWCRRUvtcmHwJJZ6E+9+fl\n\t8Fs2emmG7jwBzUot7cqQO3gn9pbzCb3ORM6do40aShwEzvXxkfzR5NS+TVh+gf0XUuPf\n\t8MVHYS4HAecEFHAX9/lg6AYw2O6IxpHYlQ8+g0kfFERV/n9tYVLkQDW55tueoV6Og9j8\n\tOiXYyvneqErfLu6DHS890h3XQtFL+PG5l3Spb657gfwOgGs/Fjytwb7IQGApASez2M37\n\tik4A==","X-Gm-Message-State":"ANoB5pkZQ+z1kCpNsjI8nrrLmn2r9zVGyvTTVhMumwrnFNTvf0O4u56r\n\t9ws6oCLJUz1w/mbmTdzojPjY69r+dXIyEguzduJyZI1CjEQei2d1","X-Google-Smtp-Source":"AA0mqf7QI14bnVUoeBYqCq6d4Z73hsxBMCfyHoa/sZ0Shd/aQiImDOCmPNY3NP2JAecmu/dGJRn/I+z3wHQlop21PfY=","X-Received":"by 2002:a02:6a43:0:b0:375:4725:4b4f with SMTP id\n\tm3-20020a026a43000000b0037547254b4fmr33164014jaf.52.1669986369581;\n\tFri, 02 Dec 2022 05:06:09 -0800 (PST)","MIME-Version":"1.0","References":"<20221129134534.2933-1-naush@raspberrypi.com>\n\t<20221129134534.2933-4-naush@raspberrypi.com>\n\t<Y4ntTP00kycN5dNA@pendragon.ideasonboard.com>","In-Reply-To":"<Y4ntTP00kycN5dNA@pendragon.ideasonboard.com>","Date":"Fri, 2 Dec 2022 13:05:54 +0000","Message-ID":"<CAEmqJPqWpGW-q28iEcCCEj4DEERFSTD52apixV_dg03gpVH65Q@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"00000000000056d9f105eed80094\"","Subject":"Re: [libcamera-devel] [PATCH v2 03/10] pipeline: raspberrypi: Split\n\tout ISP Output0 buffer allocation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]