[libcamera-devel] [PATCH 3/3] pipeline: raspberrypi: Increase the V4L2BufferCache slot allocations

[Roman Stratiienko]

Naush,

Please let me once again remind you that I have to set the
configuration variable by downstream patch which determines the max.
number of buffers for Android to allocate.

--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -429,9 +429,9 @@ CameraConfiguration::Status
RPiCameraConfiguration::validate()
                        cfg.frameSize = unicamFormat.planes[0].size;
                        rawCount++;
                } else {
+                       cfg.bufferCount = 3;
                        outSize[outCount] = std::make_pair(count, cfg.size);
                        /* Record the largest resolution for fixups later. */
                        if (maxSize < cfg.size) {

So maybe this value could be used to calculate cache size. Sorry if I
am talking about wrong things, I am new to the libcamera codebase.

Roman.

чт, 11 нояб. 2021 г. в 10:16, Naushir Patuck <naush at raspberrypi.com>:
>
> Hi Umang,
>
> Thank you for your reviews!
>
> On Wed, 10 Nov 2021 at 18:35, Umang Jain <umang.jain at ideasonboard.com> wrote:
>>
>> Hi Naush
>>
>> Thank you for the patch
>>
>> On 11/10/21 3:38 PM, Naushir Patuck wrote:
>> > If a stream is marked as external, double the number of V4L2BufferCache slots
>> > that are allocated. This is to account for additional buffers that may be
>> > allocated directly by the application.
>>
>>
>> One clarification pleease, does this mean applications can still
>> allocate more buffers on the fly (i.e. the count can increase in
>> future), even after RPi pipeline-handler has started?
>
>
> My understanding is that the Android layer does exactly this.
>
> As pointed out by Kieran, one issue is that we may not know
> the exact number of buffers allocated by the application.  Hence
> we need a mechanism where the buffer cache sizing might have
> to become dynamic to account for additional buffers.  For now,
> over allocating the slots in the cache will be sufficient.
>
> Regards,
> Naush
>
>>
>>
>> >
>> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>>
>> Patch looks good so,
>>
>> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>>
>> > ---
>> >   src/libcamera/pipeline/raspberrypi/rpi_stream.cpp | 8 ++++++++
>> >   1 file changed, 8 insertions(+)
>> >
>> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
>> > index b3265d0e8aab..67901936d6b6 100644
>> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
>> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
>> > @@ -107,6 +107,14 @@ int Stream::prepareBuffers(unsigned int count)
>> >               count = bufferMap_.size();
>> >       }
>> >
>> > +     /*
>> > +      * If this is an external stream, we must allocate slots for buffers that
>> > +      * might be externally allocated. We have no indication of how many buffers
>> > +      * may be used, so this might overallocate slots in the buffer cache.
>> > +      */
>> > +     if (isExternal())
>> > +             count = count * 2;
>> > +
>> >       return dev_->importBuffers(count);
>> >   }
>> >