<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thank you for your feedback!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 2 Dec 2022 at 12:19, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Tue, Nov 29, 2022 at 01:45:27PM +0000, Naushir Patuck via libcamera-devel wrote:<br>
> Add a new config parameter numOutput0Buffers specifying the number of internally<br>
> allocated ISP Output0 buffers. This parameter defaults to 1.<br>
> <br>
> Split out the buffer allocation logic so that the buffer count for ISP Output0<br>
> can be different from the ISP Output1 and Statistics streams.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> ---<br>
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 21 +++++++++++++++----<br>
>  1 file changed, 17 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index 4486d31ea78d..f2b695af2399 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -305,6 +305,11 @@ public:<br>
>                * the Unicam Image steram.<br>
>                */<br>
>               unsigned int minTotalUnicamBuffers;<br>
> +             /*<br>
> +              * The number of internal buffers allocated for the ISP Output0<br>
> +              * stream.<br>
<br>
I'd comment here that the value can only be 0 or 1.<br></blockquote><div><br></div><div>Ack.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +              */<br>
> +             unsigned int numOutput0Buffers;<br>
>       };<br>
>  <br>
>       Config config_;<br>
> @@ -1418,6 +1423,7 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)<br>
>       config = {<br>
>               .minUnicamBuffers = 2,<br>
>               .minTotalUnicamBuffers = 4,<br>
> +             .numOutput0Buffers = 1,<br>
>       };<br>
>  <br>
>       return 0;<br>
> @@ -1502,12 +1508,19 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)<br>
>                        * so allocate the minimum required to avoid frame drops.<br>
>                        */<br>
>                       numBuffers = minBuffers;<br>
> -             } else {<br>
> +             } else if (stream == &data->isp_[Isp::Output0]) {<br>
>                       /*<br>
>                        * Since the ISP runs synchronous with the IPA and requests,<br>
> -                      * we only ever need one set of internal buffers. Any buffers<br>
> -                      * the application wants to hold onto will already be exported<br>
> -                      * through PipelineHandlerRPi::exportFrameBuffers().<br>
> +                      * we only ever need a maximum of one internal buffer. Any<br>
> +                      * buffers the application wants to hold onto will already<br>
> +                      * be exported through PipelineHandlerRPi::exportFrameBuffers().<br>
> +                      */<br>
> +                     numBuffers = std::min(1u, data->config_.numOutput0Buffers);<br>
<br>
Is it OK to call Stream::prepareBuffers() below if numBuffers is 0 ?<br></blockquote><div><br></div><div>Yes it is. The count param tells the stream how many (if any) buffers to</div><div>allocate for internal use. Stream::prepareBuffers will call V4L2VideoDevice::importBuffers</div><div>for externally/internally allocated buffers.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             } else {<br>
> +                     /*<br>
> +                      * Same reasoning as above, we only ever need a maximum<br>
> +                      * of one internal buffer for Output1 (required for colour<br>
> +                      * denoise) and ISP statistics.<br>
>                        */<br>
>                       numBuffers = 1;<br>
>               }<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>