[libcamera-devel] [PATCH v2 1/2] libcamera: framebuffer_allocator: Make allocate() accept count

[Laurent Pinchart]

Hi Nícolas,

On Mon, Apr 19, 2021 at 10:43:40AM -0300, Nícolas F. R. A. Prado wrote:
> Em 2021-04-19 03:07, Laurent Pinchart escreveu:
> > On Mon, Apr 19, 2021 at 02:56:33PM +0900, Hirokazu Honda wrote:
> > > On Sat, Apr 17, 2021 at 7:29 AM Nícolas F. R. A. Prado wrote:
> > > >
> > > > Add a 'count' argument to FrameBufferAllocator::allocate() so that a
> > > > custom amount of buffers can be allocated. If 0 is passed, the pipeline
> > > > handler allocates the default amount, which is the configured
> > > > bufferCount.
> > > >
> > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > > > ---
> > > >  include/libcamera/camera.h                         | 3 ++-
> > > >  include/libcamera/framebuffer_allocator.h          | 2 +-
> > > >  include/libcamera/internal/pipeline_handler.h      | 3 ++-
> > > >  src/cam/capture.cpp                                | 2 +-
> > > >  src/lc-compliance/simple_capture.cpp               | 8 ++++----
> > > >  src/lc-compliance/simple_capture.h                 | 2 +-
> > > >  src/libcamera/camera.cpp                           | 5 +++--
> > > >  src/libcamera/framebuffer_allocator.cpp            | 5 +++--
> > > >  src/libcamera/pipeline/ipu3/ipu3.cpp               | 9 ++++++---
> > > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ++++++---
> > > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp           | 9 ++++++---
> > > >  src/libcamera/pipeline/simple/simple.cpp           | 9 ++++++---
> > > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       | 9 ++++++---
> > > >  src/libcamera/pipeline/vimc/vimc.cpp               | 9 ++++++---
> > > >  src/libcamera/pipeline_handler.cpp                 | 1 +
> > > >  src/qcam/main_window.cpp                           | 2 +-
> > > >  test/camera/capture.cpp                            | 2 +-
> > > >  test/camera/statemachine.cpp                       | 2 +-
> > > >  test/mapped-buffer.cpp                             | 2 +-
> > > >  19 files changed, 58 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > > > index 326b14d0ca01..ef6113113b6c 100644
> > > > --- a/include/libcamera/camera.h
> > > > +++ b/include/libcamera/camera.h
> > > > @@ -116,7 +116,8 @@ private:
> > > >
> > > >         friend class FrameBufferAllocator;
> > > >         int exportFrameBuffers(Stream *stream,
> > > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> > > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > > +                              unsigned int count);
> > > >  };
> > > 
> > > In my humble opinion, giving some number a special meaning is not a good design.
> > > I would like to ask others' opinions.
> > > Alternative (better) design is change the type of count to std::optional.
> >
> > std::optional isn't an option, as it's a C++17 feature, and the public
> > API has to stay compatible with C++14 to support Chromium (the browser,
> > not the OS).
> >
> > How about making the buffer count mandatory, always passing a value
> > explicitly ?
> 
> Sounds good to me.
> 
> Given that this makes the configuration's bufferCount pointless, I
> should add a patch removing it in this series, right?

On top of the series, yes.

-- 
Regards,

Laurent Pinchart