[libcamera-devel] [PATCH v1 1/4] pipeline: rpi: Increase buffer import count to 32

[Naushir Patuck]

Hi Jacopo,

On Mon, 24 Jul 2023 at 08:29, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Naush
>
> On Fri, Jul 21, 2023 at 10:37:56AM +0100, Naushir Patuck via libcamera-devel wrote:
> > Hardcode the maximum number of buffers imported to the V4L2 video device
> > to 32. This only has a minor disadvantage of over-allocating cache slots
> > and V4L2 buffer indexes, but does allow more headroom for using dma
> > buffers allocated from outside of libcamera.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  .../pipeline/rpi/common/rpi_stream.cpp        | 35 +++++--------------
> >  1 file changed, 9 insertions(+), 26 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > index c158843cb0ed..1d05c5acc0d9 100644
> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > @@ -94,35 +94,18 @@ int Stream::prepareBuffers(unsigned int count)
> >  {
> >       int ret;
> >
> > -     if (!(flags_ & StreamFlag::ImportOnly)) {
> > -             if (count) {
> > -                     /* Export some frame buffers for internal use. */
> > -                     ret = dev_->exportBuffers(count, &internalBuffers_);
> > -                     if (ret < 0)
> > -                             return ret;
> > -
> > -                     /* Add these exported buffers to the internal/external buffer list. */
> > -                     setExportedBuffers(&internalBuffers_);
> > -                     resetBuffers();
> > -             }
> > +     if (!(flags_ & StreamFlag::ImportOnly) && count) {
>
> Is there a case where this functions is called with a 0 'count' ?

I *think* count will always be non-zero.  So the test above is
probably redundant!

Regards,
Naush

>
> > +             /* Export some frame buffers for internal use. */
> > +             ret = dev_->exportBuffers(count, &internalBuffers_);
> > +             if (ret < 0)
> > +                     return ret;
> >
> > -             /* We must import all internal/external exported buffers. */
> > -             count = bufferMap_.size();
> > +             /* Add these exported buffers to the internal/external buffer list. */
> > +             setExportedBuffers(&internalBuffers_);
> > +             resetBuffers();
> >       }
> >
> > -     /*
> > -      * 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.
> > -      * Similarly, if this stream is only importing buffers, we do the same.
> > -      *
> > -      * \todo Find a better heuristic, or, even better, an exact solution to
> > -      * this issue.
> > -      */
> > -     if ((flags_ & StreamFlag::External) || (flags_ & StreamFlag::ImportOnly))
> > -             count = count * 2;
> > -
> > -     return dev_->importBuffers(count);
> > +     return dev_->importBuffers(32);
> >  }
> >
> >  int Stream::queueBuffer(FrameBuffer *buffer)
> > --
> > 2.34.1
> >