<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>