<div dir="ltr"><div dir="ltr">Hi Roman,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 11 Nov 2021 at 08:36, Roman Stratiienko <<a href="mailto:r.stratiienko@gmail.com">r.stratiienko@gmail.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">Naush,<br>
<br>
Please let me once again remind you that I have to set the<br>
configuration variable by downstream patch which determines the max.<br>
number of buffers for Android to allocate.<br>
<br>
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
@@ -429,9 +429,9 @@ CameraConfiguration::Status<br>
RPiCameraConfiguration::validate()<br>
                        cfg.frameSize = unicamFormat.planes[0].size;<br>
                        rawCount++;<br>
                } else {<br>
+                       cfg.bufferCount = 3;<br>
                        outSize[outCount] = std::make_pair(count, cfg.size);<br>
                        /* Record the largest resolution for fixups later. */<br>
                        if (maxSize < cfg.size) {<br>
<br>
So maybe this value could be used to calculate cache size. Sorry if I<br>
am talking about wrong things, I am new to the libcamera codebase.<br></blockquote><div><br></div><div>I think this change effectively over allocates the buffer cache, and fixes</div><div>the original problem you were seeing.  The buffercount does indirectly</div><div>get used to size the buffer cache in the existing code.</div><div><br></div><div>I would not necessarily say this is the correct fix though, and we probably</div><div>need to understand the Android layer's buffer allocation mechanism better</div><div>for the Raspberry Pi pipeline handler.</div><div><br></div><div>Naush</div><div><br></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>
Roman.<br>
<br>
чт, 11 нояб. 2021 г. в 10:16, Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>>:<br>
><br>
> Hi Umang,<br>
><br>
> Thank you for your reviews!<br>
><br>
> On Wed, 10 Nov 2021 at 18:35, Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>> wrote:<br>
>><br>
>> Hi Naush<br>
>><br>
>> Thank you for the patch<br>
>><br>
>> On 11/10/21 3:38 PM, Naushir Patuck wrote:<br>
>> > If a stream is marked as external, double the number of V4L2BufferCache slots<br>
>> > that are allocated. This is to account for additional buffers that may be<br>
>> > allocated directly by the application.<br>
>><br>
>><br>
>> One clarification pleease, does this mean applications can still<br>
>> allocate more buffers on the fly (i.e. the count can increase in<br>
>> future), even after RPi pipeline-handler has started?<br>
><br>
><br>
> My understanding is that the Android layer does exactly this.<br>
><br>
> As pointed out by Kieran, one issue is that we may not know<br>
> the exact number of buffers allocated by the application.  Hence<br>
> we need a mechanism where the buffer cache sizing might have<br>
> to become dynamic to account for additional buffers.  For now,<br>
> over allocating the slots in the cache will be sufficient.<br>
><br>
> Regards,<br>
> Naush<br>
><br>
>><br>
>><br>
>> ><br>
>> > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
>><br>
>> Patch looks good so,<br>
>><br>
>> Reviewed-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>><br>
>><br>
>> > ---<br>
>> >   src/libcamera/pipeline/raspberrypi/rpi_stream.cpp | 8 ++++++++<br>
>> >   1 file changed, 8 insertions(+)<br>
>> ><br>
>> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp<br>
>> > index b3265d0e8aab..67901936d6b6 100644<br>
>> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp<br>
>> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp<br>
>> > @@ -107,6 +107,14 @@ int Stream::prepareBuffers(unsigned int count)<br>
>> >               count = bufferMap_.size();<br>
>> >       }<br>
>> ><br>
>> > +     /*<br>
>> > +      * If this is an external stream, we must allocate slots for buffers that<br>
>> > +      * might be externally allocated. We have no indication of how many buffers<br>
>> > +      * may be used, so this might overallocate slots in the buffer cache.<br>
>> > +      */<br>
>> > +     if (isExternal())<br>
>> > +             count = count * 2;<br>
>> > +<br>
>> >       return dev_->importBuffers(count);<br>
>> >   }<br>
>> ><br>
</blockquote></div></div>