[libcamera-devel,v2,1/2] libcamera: framebuffer_allocator: Make allocate() accept count
diff mbox series

Message ID 20210416222830.335755-2-nfraprado@collabora.com
State Superseded
Headers show
Series
  • lc-compliance: Add test to queue more requests than hardware depth
Related show

Commit Message

Nícolas F. R. A. Prado April 16, 2021, 10:28 p.m. UTC 
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@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(-)

Comments

Hirokazu Honda April 19, 2021, 5:56 a.m. UTC | #1 
Hi Nicolas, thanks for the patch.

On Sat, Apr 17, 2021 at 7:29 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> 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@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.

-Hiro

>
>  } /* namespace libcamera */
> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> index 0c85631a1da2..f1ae37288d50 100644
> --- a/include/libcamera/framebuffer_allocator.h
> +++ b/include/libcamera/framebuffer_allocator.h
> @@ -25,7 +25,7 @@ public:
>         FrameBufferAllocator(std::shared_ptr<Camera> camera);
>         ~FrameBufferAllocator();
>
> -       int allocate(Stream *stream);
> +       int allocate(Stream *stream, unsigned int count);
>         int free(Stream *stream);
>
>         bool allocated() const { return !buffers_.empty(); }
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c6454db6b2c4..37c51f3f0604 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -76,7 +76,8 @@ public:
>         virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
>
>         virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> -                                      std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> +                                      std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                      unsigned int count) = 0;
>
>         virtual int start(Camera *camera, const ControlList *controls) = 0;
>         virtual void stop(Camera *camera) = 0;
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 7b55fc677022..dc24b12f4d10 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -80,7 +80,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
>         /* Identify the stream with the least number of buffers. */
>         unsigned int nbuffers = UINT_MAX;
>         for (StreamConfiguration &cfg : *config_) {
> -               ret = allocator->allocate(cfg.stream());
> +               ret = allocator->allocate(cfg.stream(), 0);
>                 if (ret < 0) {
>                         std::cerr << "Can't allocate buffers" << std::endl;
>                         return -ENOMEM;
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> index 64e862a08e3a..42e4c9b1302d 100644
> --- a/src/lc-compliance/simple_capture.cpp
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -39,10 +39,10 @@ Results::Result SimpleCapture::configure(StreamRole role)
>         return { Results::Pass, "Configure camera" };
>  }
>
> -Results::Result SimpleCapture::start()
> +Results::Result SimpleCapture::start(unsigned int numBuffers)
>  {
>         Stream *stream = config_->at(0).stream();
> -       if (allocator_->allocate(stream) < 0)
> +       if (allocator_->allocate(stream, numBuffers) < 0)
>                 return { Results::Fail, "Failed to allocate buffers" };
>
>         if (camera_->start())
> @@ -73,7 +73,7 @@ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
>
>  Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
>  {
> -       Results::Result ret = start();
> +       Results::Result ret = start(0);
>         if (ret.first != Results::Pass)
>                 return ret;
>
> @@ -161,7 +161,7 @@ SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)
>
>  Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
>  {
> -       Results::Result ret = start();
> +       Results::Result ret = start(0);
>         if (ret.first != Results::Pass)
>                 return ret;
>
> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> index d9de53fb63a3..2b1493ecaf96 100644
> --- a/src/lc-compliance/simple_capture.h
> +++ b/src/lc-compliance/simple_capture.h
> @@ -23,7 +23,7 @@ protected:
>         SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
>         virtual ~SimpleCapture();
>
> -       Results::Result start();
> +       Results::Result start(unsigned int numBuffers);
>         void stop();
>
>         virtual void requestComplete(libcamera::Request *request) = 0;
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 763f3b9926fd..7df110ee2f52 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -657,7 +657,8 @@ void Camera::disconnect()
>  }
>
>  int Camera::exportFrameBuffers(Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count)
>  {
>         Private *const d = LIBCAMERA_D_PTR();
>
> @@ -673,7 +674,7 @@ int Camera::exportFrameBuffers(Stream *stream,
>
>         return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
>                                       ConnectionTypeBlocking, this, stream,
> -                                     buffers);
> +                                     buffers, count);
>  }
>
>  /**
> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> index 2fbba37a1b0b..6b07203017cd 100644
> --- a/src/libcamera/framebuffer_allocator.cpp
> +++ b/src/libcamera/framebuffer_allocator.cpp
> @@ -70,6 +70,7 @@ FrameBufferAllocator::~FrameBufferAllocator()
>  /**
>   * \brief Allocate buffers for a configured stream
>   * \param[in] stream The stream to allocate buffers for
> + * \param[in] count The amount of buffers to allocate
>   *
>   * Allocate buffers suitable for capturing frames from the \a stream. The Camera
>   * shall have been previously configured with Camera::configure() and shall be
> @@ -85,14 +86,14 @@ FrameBufferAllocator::~FrameBufferAllocator()
>   * not part of the active camera configuration
>   * \retval -EBUSY Buffers are already allocated for the \a stream
>   */
> -int FrameBufferAllocator::allocate(Stream *stream)
> +int FrameBufferAllocator::allocate(Stream *stream, unsigned int count)
>  {
>         if (buffers_.count(stream)) {
>                 LOG(Allocator, Error) << "Buffers already allocated for stream";
>                 return -EBUSY;
>         }
>
> -       int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);
> +       int ret = camera_->exportFrameBuffers(stream, &buffers_[stream], count);
>         if (ret == -EINVAL)
>                 LOG(Allocator, Error)
>                         << "Stream is not part of " << camera_->id()
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 519cad4f8148..7a44900b9fbc 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -131,7 +131,8 @@ public:
>         int configure(Camera *camera, CameraConfiguration *config) override;
>
>         int exportFrameBuffers(Camera *camera, Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count) override;
>
>         int start(Camera *camera, const ControlList *controls) override;
>         void stop(Camera *camera) override;
> @@ -641,10 +642,12 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  }
>
>  int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> -                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                           unsigned int count)
>  {
>         IPU3CameraData *data = cameraData(camera);
> -       unsigned int count = stream->configuration().bufferCount;
> +       if (!count)
> +               count = stream->configuration().bufferCount;
>
>         if (stream == &data->outStream_)
>                 return data->imgu_->output_->exportBuffers(count, buffers);
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index f22e286ed87a..3ab27123b1ac 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -250,7 +250,8 @@ public:
>         int configure(Camera *camera, CameraConfiguration *config) override;
>
>         int exportFrameBuffers(Camera *camera, Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count) override;
>
>         int start(Camera *camera, const ControlList *controls) override;
>         void stop(Camera *camera) override;
> @@ -786,10 +787,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  }
>
>  int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> -                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                          unsigned int count)
>  {
>         RPi::Stream *s = static_cast<RPi::Stream *>(stream);
> -       unsigned int count = stream->configuration().bufferCount;
> +       if (!count)
> +               count = stream->configuration().bufferCount;
>         int ret = s->dev()->exportBuffers(count, buffers);
>
>         s->setExportedBuffers(buffers);
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 549f4a4e61a8..4cc3c7739251 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -140,7 +140,8 @@ public:
>         int configure(Camera *camera, CameraConfiguration *config) override;
>
>         int exportFrameBuffers(Camera *camera, Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count) override;
>
>         int start(Camera *camera, const ControlList *controls) override;
>         void stop(Camera *camera) override;
> @@ -666,10 +667,12 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  }
>
>  int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> -                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                             unsigned int count)
>  {
>         RkISP1CameraData *data = cameraData(camera);
> -       unsigned int count = stream->configuration().bufferCount;
> +       if (!count)
> +               count = stream->configuration().bufferCount;
>
>         if (stream == &data->mainPathStream_)
>                 return mainPath_.exportBuffers(count, buffers);
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index f6095d38e97a..e9b0698813fc 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -223,7 +223,8 @@ public:
>         int configure(Camera *camera, CameraConfiguration *config) override;
>
>         int exportFrameBuffers(Camera *camera, Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count) override;
>
>         int start(Camera *camera, const ControlList *controls) override;
>         void stop(Camera *camera) override;
> @@ -762,10 +763,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  }
>
>  int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> -                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                             unsigned int count)
>  {
>         SimpleCameraData *data = cameraData(camera);
> -       unsigned int count = stream->configuration().bufferCount;
> +       if (!count)
> +               count = stream->configuration().bufferCount;
>
>         /*
>          * Export buffers on the converter or capture video node, depending on
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index b6c6ade5ebaf..298e7031d23b 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -70,7 +70,8 @@ public:
>         int configure(Camera *camera, CameraConfiguration *config) override;
>
>         int exportFrameBuffers(Camera *camera, Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count) override;
>
>         int start(Camera *camera, const ControlList *controls) override;
>         void stop(Camera *camera) override;
> @@ -224,10 +225,12 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>  }
>
>  int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> -                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                          unsigned int count)
>  {
>         UVCCameraData *data = cameraData(camera);
> -       unsigned int count = stream->configuration().bufferCount;
> +       if (!count)
> +               count = stream->configuration().bufferCount;
>
>         return data->video_->exportBuffers(count, buffers);
>  }
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 8f5f4ba30953..2f889347b624 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -84,7 +84,8 @@ public:
>         int configure(Camera *camera, CameraConfiguration *config) override;
>
>         int exportFrameBuffers(Camera *camera, Stream *stream,
> -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                              unsigned int count) override;
>
>         int start(Camera *camera, const ControlList *controls) override;
>         void stop(Camera *camera) override;
> @@ -299,10 +300,12 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
>  }
>
>  int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> -                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> +                                           unsigned int count)
>  {
>         VimcCameraData *data = cameraData(camera);
> -       unsigned int count = stream->configuration().bufferCount;
> +       if (!count)
> +               count = stream->configuration().bufferCount;
>
>         return data->video_->exportBuffers(count, buffers);
>  }
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 3b3150bdbbf7..ab5f21a01438 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -323,6 +323,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
>   * \param[in] camera The camera
>   * \param[in] stream The stream to allocate buffers for
>   * \param[out] buffers Array of buffers successfully allocated
> + * \param[in] count The amount of buffers to allocate
>   *
>   * This method allocates buffers for the \a stream from the devices associated
>   * with the stream in the corresponding pipeline handler. Those buffers shall be
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 39d034de6bb2..4e1dbb0656c8 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -463,7 +463,7 @@ int MainWindow::startCapture()
>         for (StreamConfiguration &config : *config_) {
>                 Stream *stream = config.stream();
>
> -               ret = allocator_->allocate(stream);
> +               ret = allocator_->allocate(stream, 0);
>                 if (ret < 0) {
>                         qWarning() << "Failed to allocate capture buffers";
>                         goto error;
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index c4bc21100777..6cbca15b30bc 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -96,7 +96,7 @@ protected:
>
>                 Stream *stream = cfg.stream();
>
> -               int ret = allocator_->allocate(stream);
> +               int ret = allocator_->allocate(stream, 0);
>                 if (ret < 0)
>                         return TestFail;
>
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index f0c3d7764027..9e076d05bc6f 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -118,7 +118,7 @@ protected:
>                 /* Use internally allocated buffers. */
>                 allocator_ = new FrameBufferAllocator(camera_);
>                 Stream *stream = *camera_->streams().begin();
> -               if (allocator_->allocate(stream) < 0)
> +               if (allocator_->allocate(stream, 0) < 0)
>                         return TestFail;
>
>                 if (camera_->start())
> diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp
> index 5de8201e45f6..f5c2e2c0169a 100644
> --- a/test/mapped-buffer.cpp
> +++ b/test/mapped-buffer.cpp
> @@ -54,7 +54,7 @@ protected:
>
>                 stream_ = cfg.stream();
>
> -               int ret = allocator_->allocate(stream_);
> +               int ret = allocator_->allocate(stream_, 0);
>                 if (ret < 0)
>                         return TestFail;
>
> --
> 2.31.1
>
Laurent Pinchart April 19, 2021, 6:07 a.m. UTC | #2 
Hello,

On Mon, Apr 19, 2021 at 02:56:33PM +0900, Hirokazu Honda wrote:
> Hi Nicolas, thanks for the patch.
> 
> 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@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 ?

> >  } /* namespace libcamera */
> > diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> > index 0c85631a1da2..f1ae37288d50 100644
> > --- a/include/libcamera/framebuffer_allocator.h
> > +++ b/include/libcamera/framebuffer_allocator.h
> > @@ -25,7 +25,7 @@ public:
> >         FrameBufferAllocator(std::shared_ptr<Camera> camera);
> >         ~FrameBufferAllocator();
> >
> > -       int allocate(Stream *stream);
> > +       int allocate(Stream *stream, unsigned int count);
> >         int free(Stream *stream);
> >
> >         bool allocated() const { return !buffers_.empty(); }
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index c6454db6b2c4..37c51f3f0604 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -76,7 +76,8 @@ public:
> >         virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
> >
> >         virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                                      std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> > +                                      std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                      unsigned int count) = 0;
> >
> >         virtual int start(Camera *camera, const ControlList *controls) = 0;
> >         virtual void stop(Camera *camera) = 0;
> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > index 7b55fc677022..dc24b12f4d10 100644
> > --- a/src/cam/capture.cpp
> > +++ b/src/cam/capture.cpp
> > @@ -80,7 +80,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
> >         /* Identify the stream with the least number of buffers. */
> >         unsigned int nbuffers = UINT_MAX;
> >         for (StreamConfiguration &cfg : *config_) {
> > -               ret = allocator->allocate(cfg.stream());
> > +               ret = allocator->allocate(cfg.stream(), 0);
> >                 if (ret < 0) {
> >                         std::cerr << "Can't allocate buffers" << std::endl;
> >                         return -ENOMEM;
> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> > index 64e862a08e3a..42e4c9b1302d 100644
> > --- a/src/lc-compliance/simple_capture.cpp
> > +++ b/src/lc-compliance/simple_capture.cpp
> > @@ -39,10 +39,10 @@ Results::Result SimpleCapture::configure(StreamRole role)
> >         return { Results::Pass, "Configure camera" };
> >  }
> >
> > -Results::Result SimpleCapture::start()
> > +Results::Result SimpleCapture::start(unsigned int numBuffers)
> >  {
> >         Stream *stream = config_->at(0).stream();
> > -       if (allocator_->allocate(stream) < 0)
> > +       if (allocator_->allocate(stream, numBuffers) < 0)
> >                 return { Results::Fail, "Failed to allocate buffers" };
> >
> >         if (camera_->start())
> > @@ -73,7 +73,7 @@ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
> >
> >  Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
> >  {
> > -       Results::Result ret = start();
> > +       Results::Result ret = start(0);
> >         if (ret.first != Results::Pass)
> >                 return ret;
> >
> > @@ -161,7 +161,7 @@ SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)
> >
> >  Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
> >  {
> > -       Results::Result ret = start();
> > +       Results::Result ret = start(0);
> >         if (ret.first != Results::Pass)
> >                 return ret;
> >
> > diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> > index d9de53fb63a3..2b1493ecaf96 100644
> > --- a/src/lc-compliance/simple_capture.h
> > +++ b/src/lc-compliance/simple_capture.h
> > @@ -23,7 +23,7 @@ protected:
> >         SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
> >         virtual ~SimpleCapture();
> >
> > -       Results::Result start();
> > +       Results::Result start(unsigned int numBuffers);
> >         void stop();
> >
> >         virtual void requestComplete(libcamera::Request *request) = 0;
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 763f3b9926fd..7df110ee2f52 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -657,7 +657,8 @@ void Camera::disconnect()
> >  }
> >
> >  int Camera::exportFrameBuffers(Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count)
> >  {
> >         Private *const d = LIBCAMERA_D_PTR();
> >
> > @@ -673,7 +674,7 @@ int Camera::exportFrameBuffers(Stream *stream,
> >
> >         return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
> >                                       ConnectionTypeBlocking, this, stream,
> > -                                     buffers);
> > +                                     buffers, count);
> >  }
> >
> >  /**
> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> > index 2fbba37a1b0b..6b07203017cd 100644
> > --- a/src/libcamera/framebuffer_allocator.cpp
> > +++ b/src/libcamera/framebuffer_allocator.cpp
> > @@ -70,6 +70,7 @@ FrameBufferAllocator::~FrameBufferAllocator()
> >  /**
> >   * \brief Allocate buffers for a configured stream
> >   * \param[in] stream The stream to allocate buffers for
> > + * \param[in] count The amount of buffers to allocate
> >   *
> >   * Allocate buffers suitable for capturing frames from the \a stream. The Camera
> >   * shall have been previously configured with Camera::configure() and shall be
> > @@ -85,14 +86,14 @@ FrameBufferAllocator::~FrameBufferAllocator()
> >   * not part of the active camera configuration
> >   * \retval -EBUSY Buffers are already allocated for the \a stream
> >   */
> > -int FrameBufferAllocator::allocate(Stream *stream)
> > +int FrameBufferAllocator::allocate(Stream *stream, unsigned int count)
> >  {
> >         if (buffers_.count(stream)) {
> >                 LOG(Allocator, Error) << "Buffers already allocated for stream";
> >                 return -EBUSY;
> >         }
> >
> > -       int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);
> > +       int ret = camera_->exportFrameBuffers(stream, &buffers_[stream], count);
> >         if (ret == -EINVAL)
> >                 LOG(Allocator, Error)
> >                         << "Stream is not part of " << camera_->id()
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 519cad4f8148..7a44900b9fbc 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -131,7 +131,8 @@ public:
> >         int configure(Camera *camera, CameraConfiguration *config) override;
> >
> >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count) override;
> >
> >         int start(Camera *camera, const ControlList *controls) override;
> >         void stop(Camera *camera) override;
> > @@ -641,10 +642,12 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  }
> >
> >  int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> > -                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                           unsigned int count)
> >  {
> >         IPU3CameraData *data = cameraData(camera);
> > -       unsigned int count = stream->configuration().bufferCount;
> > +       if (!count)
> > +               count = stream->configuration().bufferCount;
> >
> >         if (stream == &data->outStream_)
> >                 return data->imgu_->output_->exportBuffers(count, buffers);
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index f22e286ed87a..3ab27123b1ac 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -250,7 +250,8 @@ public:
> >         int configure(Camera *camera, CameraConfiguration *config) override;
> >
> >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count) override;
> >
> >         int start(Camera *camera, const ControlList *controls) override;
> >         void stop(Camera *camera) override;
> > @@ -786,10 +787,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> >  }
> >
> >  int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> > -                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                          unsigned int count)
> >  {
> >         RPi::Stream *s = static_cast<RPi::Stream *>(stream);
> > -       unsigned int count = stream->configuration().bufferCount;
> > +       if (!count)
> > +               count = stream->configuration().bufferCount;
> >         int ret = s->dev()->exportBuffers(count, buffers);
> >
> >         s->setExportedBuffers(buffers);
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 549f4a4e61a8..4cc3c7739251 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -140,7 +140,8 @@ public:
> >         int configure(Camera *camera, CameraConfiguration *config) override;
> >
> >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count) override;
> >
> >         int start(Camera *camera, const ControlList *controls) override;
> >         void stop(Camera *camera) override;
> > @@ -666,10 +667,12 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  }
> >
> >  int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> > -                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                             unsigned int count)
> >  {
> >         RkISP1CameraData *data = cameraData(camera);
> > -       unsigned int count = stream->configuration().bufferCount;
> > +       if (!count)
> > +               count = stream->configuration().bufferCount;
> >
> >         if (stream == &data->mainPathStream_)
> >                 return mainPath_.exportBuffers(count, buffers);
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index f6095d38e97a..e9b0698813fc 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -223,7 +223,8 @@ public:
> >         int configure(Camera *camera, CameraConfiguration *config) override;
> >
> >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count) override;
> >
> >         int start(Camera *camera, const ControlList *controls) override;
> >         void stop(Camera *camera) override;
> > @@ -762,10 +763,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> >  }
> >
> >  int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> > -                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                             unsigned int count)
> >  {
> >         SimpleCameraData *data = cameraData(camera);
> > -       unsigned int count = stream->configuration().bufferCount;
> > +       if (!count)
> > +               count = stream->configuration().bufferCount;
> >
> >         /*
> >          * Export buffers on the converter or capture video node, depending on
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index b6c6ade5ebaf..298e7031d23b 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -70,7 +70,8 @@ public:
> >         int configure(Camera *camera, CameraConfiguration *config) override;
> >
> >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count) override;
> >
> >         int start(Camera *camera, const ControlList *controls) override;
> >         void stop(Camera *camera) override;
> > @@ -224,10 +225,12 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> >  }
> >
> >  int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> > -                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                          unsigned int count)
> >  {
> >         UVCCameraData *data = cameraData(camera);
> > -       unsigned int count = stream->configuration().bufferCount;
> > +       if (!count)
> > +               count = stream->configuration().bufferCount;
> >
> >         return data->video_->exportBuffers(count, buffers);
> >  }
> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > index 8f5f4ba30953..2f889347b624 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -84,7 +84,8 @@ public:
> >         int configure(Camera *camera, CameraConfiguration *config) override;
> >
> >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                              unsigned int count) override;
> >
> >         int start(Camera *camera, const ControlList *controls) override;
> >         void stop(Camera *camera) override;
> > @@ -299,10 +300,12 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> >  }
> >
> >  int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> > -                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > +                                           unsigned int count)
> >  {
> >         VimcCameraData *data = cameraData(camera);
> > -       unsigned int count = stream->configuration().bufferCount;
> > +       if (!count)
> > +               count = stream->configuration().bufferCount;
> >
> >         return data->video_->exportBuffers(count, buffers);
> >  }
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 3b3150bdbbf7..ab5f21a01438 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -323,6 +323,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
> >   * \param[in] camera The camera
> >   * \param[in] stream The stream to allocate buffers for
> >   * \param[out] buffers Array of buffers successfully allocated
> > + * \param[in] count The amount of buffers to allocate
> >   *
> >   * This method allocates buffers for the \a stream from the devices associated
> >   * with the stream in the corresponding pipeline handler. Those buffers shall be
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 39d034de6bb2..4e1dbb0656c8 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -463,7 +463,7 @@ int MainWindow::startCapture()
> >         for (StreamConfiguration &config : *config_) {
> >                 Stream *stream = config.stream();
> >
> > -               ret = allocator_->allocate(stream);
> > +               ret = allocator_->allocate(stream, 0);
> >                 if (ret < 0) {
> >                         qWarning() << "Failed to allocate capture buffers";
> >                         goto error;
> > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > index c4bc21100777..6cbca15b30bc 100644
> > --- a/test/camera/capture.cpp
> > +++ b/test/camera/capture.cpp
> > @@ -96,7 +96,7 @@ protected:
> >
> >                 Stream *stream = cfg.stream();
> >
> > -               int ret = allocator_->allocate(stream);
> > +               int ret = allocator_->allocate(stream, 0);
> >                 if (ret < 0)
> >                         return TestFail;
> >
> > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> > index f0c3d7764027..9e076d05bc6f 100644
> > --- a/test/camera/statemachine.cpp
> > +++ b/test/camera/statemachine.cpp
> > @@ -118,7 +118,7 @@ protected:
> >                 /* Use internally allocated buffers. */
> >                 allocator_ = new FrameBufferAllocator(camera_);
> >                 Stream *stream = *camera_->streams().begin();
> > -               if (allocator_->allocate(stream) < 0)
> > +               if (allocator_->allocate(stream, 0) < 0)
> >                         return TestFail;
> >
> >                 if (camera_->start())
> > diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp
> > index 5de8201e45f6..f5c2e2c0169a 100644
> > --- a/test/mapped-buffer.cpp
> > +++ b/test/mapped-buffer.cpp
> > @@ -54,7 +54,7 @@ protected:
> >
> >                 stream_ = cfg.stream();
> >
> > -               int ret = allocator_->allocate(stream_);
> > +               int ret = allocator_->allocate(stream_, 0);
> >                 if (ret < 0)
> >                         return TestFail;
> >
Hirokazu Honda April 19, 2021, 6:09 a.m. UTC | #3 
Hi Laurent,

On Mon, Apr 19, 2021 at 3:08 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hello,
>
> On Mon, Apr 19, 2021 at 02:56:33PM +0900, Hirokazu Honda wrote:
> > Hi Nicolas, thanks for the patch.
> >
> > 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@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 ?
>

I got it. It sounds good to me.

> > >  } /* namespace libcamera */
> > > diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> > > index 0c85631a1da2..f1ae37288d50 100644
> > > --- a/include/libcamera/framebuffer_allocator.h
> > > +++ b/include/libcamera/framebuffer_allocator.h
> > > @@ -25,7 +25,7 @@ public:
> > >         FrameBufferAllocator(std::shared_ptr<Camera> camera);
> > >         ~FrameBufferAllocator();
> > >
> > > -       int allocate(Stream *stream);
> > > +       int allocate(Stream *stream, unsigned int count);
> > >         int free(Stream *stream);
> > >
> > >         bool allocated() const { return !buffers_.empty(); }
> > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > > index c6454db6b2c4..37c51f3f0604 100644
> > > --- a/include/libcamera/internal/pipeline_handler.h
> > > +++ b/include/libcamera/internal/pipeline_handler.h
> > > @@ -76,7 +76,8 @@ public:
> > >         virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
> > >
> > >         virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                                      std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> > > +                                      std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                      unsigned int count) = 0;
> > >
> > >         virtual int start(Camera *camera, const ControlList *controls) = 0;
> > >         virtual void stop(Camera *camera) = 0;
> > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > > index 7b55fc677022..dc24b12f4d10 100644
> > > --- a/src/cam/capture.cpp
> > > +++ b/src/cam/capture.cpp
> > > @@ -80,7 +80,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
> > >         /* Identify the stream with the least number of buffers. */
> > >         unsigned int nbuffers = UINT_MAX;
> > >         for (StreamConfiguration &cfg : *config_) {
> > > -               ret = allocator->allocate(cfg.stream());
> > > +               ret = allocator->allocate(cfg.stream(), 0);
> > >                 if (ret < 0) {
> > >                         std::cerr << "Can't allocate buffers" << std::endl;
> > >                         return -ENOMEM;
> > > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> > > index 64e862a08e3a..42e4c9b1302d 100644
> > > --- a/src/lc-compliance/simple_capture.cpp
> > > +++ b/src/lc-compliance/simple_capture.cpp
> > > @@ -39,10 +39,10 @@ Results::Result SimpleCapture::configure(StreamRole role)
> > >         return { Results::Pass, "Configure camera" };
> > >  }
> > >
> > > -Results::Result SimpleCapture::start()
> > > +Results::Result SimpleCapture::start(unsigned int numBuffers)
> > >  {
> > >         Stream *stream = config_->at(0).stream();
> > > -       if (allocator_->allocate(stream) < 0)
> > > +       if (allocator_->allocate(stream, numBuffers) < 0)
> > >                 return { Results::Fail, "Failed to allocate buffers" };
> > >
> > >         if (camera_->start())
> > > @@ -73,7 +73,7 @@ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
> > >
> > >  Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
> > >  {
> > > -       Results::Result ret = start();
> > > +       Results::Result ret = start(0);
> > >         if (ret.first != Results::Pass)
> > >                 return ret;
> > >
> > > @@ -161,7 +161,7 @@ SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)
> > >
> > >  Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
> > >  {
> > > -       Results::Result ret = start();
> > > +       Results::Result ret = start(0);
> > >         if (ret.first != Results::Pass)
> > >                 return ret;
> > >
> > > diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> > > index d9de53fb63a3..2b1493ecaf96 100644
> > > --- a/src/lc-compliance/simple_capture.h
> > > +++ b/src/lc-compliance/simple_capture.h
> > > @@ -23,7 +23,7 @@ protected:
> > >         SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
> > >         virtual ~SimpleCapture();
> > >
> > > -       Results::Result start();
> > > +       Results::Result start(unsigned int numBuffers);
> > >         void stop();
> > >
> > >         virtual void requestComplete(libcamera::Request *request) = 0;
> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > > index 763f3b9926fd..7df110ee2f52 100644
> > > --- a/src/libcamera/camera.cpp
> > > +++ b/src/libcamera/camera.cpp
> > > @@ -657,7 +657,8 @@ void Camera::disconnect()
> > >  }
> > >
> > >  int Camera::exportFrameBuffers(Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count)
> > >  {
> > >         Private *const d = LIBCAMERA_D_PTR();
> > >
> > > @@ -673,7 +674,7 @@ int Camera::exportFrameBuffers(Stream *stream,
> > >
> > >         return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
> > >                                       ConnectionTypeBlocking, this, stream,
> > > -                                     buffers);
> > > +                                     buffers, count);
> > >  }
> > >
> > >  /**
> > > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> > > index 2fbba37a1b0b..6b07203017cd 100644
> > > --- a/src/libcamera/framebuffer_allocator.cpp
> > > +++ b/src/libcamera/framebuffer_allocator.cpp
> > > @@ -70,6 +70,7 @@ FrameBufferAllocator::~FrameBufferAllocator()
> > >  /**
> > >   * \brief Allocate buffers for a configured stream
> > >   * \param[in] stream The stream to allocate buffers for
> > > + * \param[in] count The amount of buffers to allocate
> > >   *
> > >   * Allocate buffers suitable for capturing frames from the \a stream. The Camera
> > >   * shall have been previously configured with Camera::configure() and shall be
> > > @@ -85,14 +86,14 @@ FrameBufferAllocator::~FrameBufferAllocator()
> > >   * not part of the active camera configuration
> > >   * \retval -EBUSY Buffers are already allocated for the \a stream
> > >   */
> > > -int FrameBufferAllocator::allocate(Stream *stream)
> > > +int FrameBufferAllocator::allocate(Stream *stream, unsigned int count)
> > >  {
> > >         if (buffers_.count(stream)) {
> > >                 LOG(Allocator, Error) << "Buffers already allocated for stream";
> > >                 return -EBUSY;
> > >         }
> > >
> > > -       int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);
> > > +       int ret = camera_->exportFrameBuffers(stream, &buffers_[stream], count);
> > >         if (ret == -EINVAL)
> > >                 LOG(Allocator, Error)
> > >                         << "Stream is not part of " << camera_->id()
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 519cad4f8148..7a44900b9fbc 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -131,7 +131,8 @@ public:
> > >         int configure(Camera *camera, CameraConfiguration *config) override;
> > >
> > >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count) override;
> > >
> > >         int start(Camera *camera, const ControlList *controls) override;
> > >         void stop(Camera *camera) override;
> > > @@ -641,10 +642,12 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > >  }
> > >
> > >  int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                           unsigned int count)
> > >  {
> > >         IPU3CameraData *data = cameraData(camera);
> > > -       unsigned int count = stream->configuration().bufferCount;
> > > +       if (!count)
> > > +               count = stream->configuration().bufferCount;
> > >
> > >         if (stream == &data->outStream_)
> > >                 return data->imgu_->output_->exportBuffers(count, buffers);
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index f22e286ed87a..3ab27123b1ac 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -250,7 +250,8 @@ public:
> > >         int configure(Camera *camera, CameraConfiguration *config) override;
> > >
> > >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count) override;
> > >
> > >         int start(Camera *camera, const ControlList *controls) override;
> > >         void stop(Camera *camera) override;
> > > @@ -786,10 +787,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > >  }
> > >
> > >  int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> > > -                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                          unsigned int count)
> > >  {
> > >         RPi::Stream *s = static_cast<RPi::Stream *>(stream);
> > > -       unsigned int count = stream->configuration().bufferCount;
> > > +       if (!count)
> > > +               count = stream->configuration().bufferCount;
> > >         int ret = s->dev()->exportBuffers(count, buffers);
> > >
> > >         s->setExportedBuffers(buffers);
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 549f4a4e61a8..4cc3c7739251 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -140,7 +140,8 @@ public:
> > >         int configure(Camera *camera, CameraConfiguration *config) override;
> > >
> > >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count) override;
> > >
> > >         int start(Camera *camera, const ControlList *controls) override;
> > >         void stop(Camera *camera) override;
> > > @@ -666,10 +667,12 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > >  }
> > >
> > >  int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> > > -                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                             unsigned int count)
> > >  {
> > >         RkISP1CameraData *data = cameraData(camera);
> > > -       unsigned int count = stream->configuration().bufferCount;
> > > +       if (!count)
> > > +               count = stream->configuration().bufferCount;
> > >
> > >         if (stream == &data->mainPathStream_)
> > >                 return mainPath_.exportBuffers(count, buffers);
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > index f6095d38e97a..e9b0698813fc 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -223,7 +223,8 @@ public:
> > >         int configure(Camera *camera, CameraConfiguration *config) override;
> > >
> > >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count) override;
> > >
> > >         int start(Camera *camera, const ControlList *controls) override;
> > >         void stop(Camera *camera) override;
> > > @@ -762,10 +763,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> > >  }
> > >
> > >  int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                                             std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                             unsigned int count)
> > >  {
> > >         SimpleCameraData *data = cameraData(camera);
> > > -       unsigned int count = stream->configuration().bufferCount;
> > > +       if (!count)
> > > +               count = stream->configuration().bufferCount;
> > >
> > >         /*
> > >          * Export buffers on the converter or capture video node, depending on
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index b6c6ade5ebaf..298e7031d23b 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -70,7 +70,8 @@ public:
> > >         int configure(Camera *camera, CameraConfiguration *config) override;
> > >
> > >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count) override;
> > >
> > >         int start(Camera *camera, const ControlList *controls) override;
> > >         void stop(Camera *camera) override;
> > > @@ -224,10 +225,12 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> > >  }
> > >
> > >  int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                                          std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                          unsigned int count)
> > >  {
> > >         UVCCameraData *data = cameraData(camera);
> > > -       unsigned int count = stream->configuration().bufferCount;
> > > +       if (!count)
> > > +               count = stream->configuration().bufferCount;
> > >
> > >         return data->video_->exportBuffers(count, buffers);
> > >  }
> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > > index 8f5f4ba30953..2f889347b624 100644
> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > > @@ -84,7 +84,8 @@ public:
> > >         int configure(Camera *camera, CameraConfiguration *config) override;
> > >
> > >         int exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                              std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > > +                              std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                              unsigned int count) override;
> > >
> > >         int start(Camera *camera, const ControlList *controls) override;
> > >         void stop(Camera *camera) override;
> > > @@ -299,10 +300,12 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> > >  }
> > >
> > >  int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> > > -                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > > +                                           std::vector<std::unique_ptr<FrameBuffer>> *buffers,
> > > +                                           unsigned int count)
> > >  {
> > >         VimcCameraData *data = cameraData(camera);
> > > -       unsigned int count = stream->configuration().bufferCount;
> > > +       if (!count)
> > > +               count = stream->configuration().bufferCount;
> > >
> > >         return data->video_->exportBuffers(count, buffers);
> > >  }
> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > > index 3b3150bdbbf7..ab5f21a01438 100644
> > > --- a/src/libcamera/pipeline_handler.cpp
> > > +++ b/src/libcamera/pipeline_handler.cpp
> > > @@ -323,6 +323,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
> > >   * \param[in] camera The camera
> > >   * \param[in] stream The stream to allocate buffers for
> > >   * \param[out] buffers Array of buffers successfully allocated
> > > + * \param[in] count The amount of buffers to allocate
> > >   *
> > >   * This method allocates buffers for the \a stream from the devices associated
> > >   * with the stream in the corresponding pipeline handler. Those buffers shall be
> > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > > index 39d034de6bb2..4e1dbb0656c8 100644
> > > --- a/src/qcam/main_window.cpp
> > > +++ b/src/qcam/main_window.cpp
> > > @@ -463,7 +463,7 @@ int MainWindow::startCapture()
> > >         for (StreamConfiguration &config : *config_) {
> > >                 Stream *stream = config.stream();
> > >
> > > -               ret = allocator_->allocate(stream);
> > > +               ret = allocator_->allocate(stream, 0);
> > >                 if (ret < 0) {
> > >                         qWarning() << "Failed to allocate capture buffers";
> > >                         goto error;
> > > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > > index c4bc21100777..6cbca15b30bc 100644
> > > --- a/test/camera/capture.cpp
> > > +++ b/test/camera/capture.cpp
> > > @@ -96,7 +96,7 @@ protected:
> > >
> > >                 Stream *stream = cfg.stream();
> > >
> > > -               int ret = allocator_->allocate(stream);
> > > +               int ret = allocator_->allocate(stream, 0);
> > >                 if (ret < 0)
> > >                         return TestFail;
> > >
> > > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> > > index f0c3d7764027..9e076d05bc6f 100644
> > > --- a/test/camera/statemachine.cpp
> > > +++ b/test/camera/statemachine.cpp
> > > @@ -118,7 +118,7 @@ protected:
> > >                 /* Use internally allocated buffers. */
> > >                 allocator_ = new FrameBufferAllocator(camera_);
> > >                 Stream *stream = *camera_->streams().begin();
> > > -               if (allocator_->allocate(stream) < 0)
> > > +               if (allocator_->allocate(stream, 0) < 0)
> > >                         return TestFail;
> > >
> > >                 if (camera_->start())
> > > diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp
> > > index 5de8201e45f6..f5c2e2c0169a 100644
> > > --- a/test/mapped-buffer.cpp
> > > +++ b/test/mapped-buffer.cpp
> > > @@ -54,7 +54,7 @@ protected:
> > >
> > >                 stream_ = cfg.stream();
> > >
> > > -               int ret = allocator_->allocate(stream_);
> > > +               int ret = allocator_->allocate(stream_, 0);
> > >                 if (ret < 0)
> > >                         return TestFail;
> > >
>
> --
> Regards,
>
> Laurent Pinchart
Nícolas F. R. A. Prado April 19, 2021, 1:43 p.m. UTC | #4 
Em 2021-04-19 03:07, Laurent Pinchart escreveu:

> Hello,
>
> On Mon, Apr 19, 2021 at 02:56:33PM +0900, Hirokazu Honda wrote:
> > Hi Nicolas, thanks for the patch.
> > 
> > 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@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?

Thanks,
Nícolas
Nícolas F. R. A. Prado April 19, 2021, 5:38 p.m. UTC | #5 
Em 2021-04-16 19:28, Nícolas F. R. A. Prado escreveu:

> 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@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/src/libcamera/pipeline_handler.cpp
> b/src/libcamera/pipeline_handler.cpp
> index 3b3150bdbbf7..ab5f21a01438 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -323,6 +323,7 @@ const ControlList &PipelineHandler::properties(const Camera
> *camera) const
> * \param[in] camera The camera
> * \param[in] stream The stream to allocate buffers for
> * \param[out] buffers Array of buffers successfully allocated
> + * \param[in] count The amount of buffers to allocate

Should the 'count' parameter come before 'buffers' so that all 'in' are before
all 'out' parameters?

Thanks,
Nícolas
Laurent Pinchart April 19, 2021, 7:18 p.m. UTC | #6 
Hi Nícolas,

On Mon, Apr 19, 2021 at 02:38:21PM -0300, Nícolas F. R. A. Prado wrote:
> Em 2021-04-16 19:28, Nícolas F. R. A. Prado escreveu:
> 
> > 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@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/src/libcamera/pipeline_handler.cpp
> > b/src/libcamera/pipeline_handler.cpp
> > index 3b3150bdbbf7..ab5f21a01438 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -323,6 +323,7 @@ const ControlList &PipelineHandler::properties(const Camera
> > *camera) const
> > * \param[in] camera The camera
> > * \param[in] stream The stream to allocate buffers for
> > * \param[out] buffers Array of buffers successfully allocated
> > + * \param[in] count The amount of buffers to allocate
> 
> Should the 'count' parameter come before 'buffers' so that all 'in' are before
> all 'out' parameters?

I'd have a preference for that, yes.
Laurent Pinchart April 19, 2021, 7:20 p.m. UTC | #7 
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@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.

Patch
diff mbox series 

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);
 };
 
 } /* namespace libcamera */
diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
index 0c85631a1da2..f1ae37288d50 100644
--- a/include/libcamera/framebuffer_allocator.h
+++ b/include/libcamera/framebuffer_allocator.h
@@ -25,7 +25,7 @@  public:
 	FrameBufferAllocator(std::shared_ptr<Camera> camera);
 	~FrameBufferAllocator();
 
-	int allocate(Stream *stream);
+	int allocate(Stream *stream, unsigned int count);
 	int free(Stream *stream);
 
 	bool allocated() const { return !buffers_.empty(); }
diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index c6454db6b2c4..37c51f3f0604 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -76,7 +76,8 @@  public:
 	virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
 
 	virtual int exportFrameBuffers(Camera *camera, Stream *stream,
-				       std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
+				       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+				       unsigned int count) = 0;
 
 	virtual int start(Camera *camera, const ControlList *controls) = 0;
 	virtual void stop(Camera *camera) = 0;
diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
index 7b55fc677022..dc24b12f4d10 100644
--- a/src/cam/capture.cpp
+++ b/src/cam/capture.cpp
@@ -80,7 +80,7 @@  int Capture::capture(FrameBufferAllocator *allocator)
 	/* Identify the stream with the least number of buffers. */
 	unsigned int nbuffers = UINT_MAX;
 	for (StreamConfiguration &cfg : *config_) {
-		ret = allocator->allocate(cfg.stream());
+		ret = allocator->allocate(cfg.stream(), 0);
 		if (ret < 0) {
 			std::cerr << "Can't allocate buffers" << std::endl;
 			return -ENOMEM;
diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
index 64e862a08e3a..42e4c9b1302d 100644
--- a/src/lc-compliance/simple_capture.cpp
+++ b/src/lc-compliance/simple_capture.cpp
@@ -39,10 +39,10 @@  Results::Result SimpleCapture::configure(StreamRole role)
 	return { Results::Pass, "Configure camera" };
 }
 
-Results::Result SimpleCapture::start()
+Results::Result SimpleCapture::start(unsigned int numBuffers)
 {
 	Stream *stream = config_->at(0).stream();
-	if (allocator_->allocate(stream) < 0)
+	if (allocator_->allocate(stream, numBuffers) < 0)
 		return { Results::Fail, "Failed to allocate buffers" };
 
 	if (camera_->start())
@@ -73,7 +73,7 @@  SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
 
 Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
 {
-	Results::Result ret = start();
+	Results::Result ret = start(0);
 	if (ret.first != Results::Pass)
 		return ret;
 
@@ -161,7 +161,7 @@  SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)
 
 Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
 {
-	Results::Result ret = start();
+	Results::Result ret = start(0);
 	if (ret.first != Results::Pass)
 		return ret;
 
diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
index d9de53fb63a3..2b1493ecaf96 100644
--- a/src/lc-compliance/simple_capture.h
+++ b/src/lc-compliance/simple_capture.h
@@ -23,7 +23,7 @@  protected:
 	SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
 	virtual ~SimpleCapture();
 
-	Results::Result start();
+	Results::Result start(unsigned int numBuffers);
 	void stop();
 
 	virtual void requestComplete(libcamera::Request *request) = 0;
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 763f3b9926fd..7df110ee2f52 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -657,7 +657,8 @@  void Camera::disconnect()
 }
 
 int Camera::exportFrameBuffers(Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count)
 {
 	Private *const d = LIBCAMERA_D_PTR();
 
@@ -673,7 +674,7 @@  int Camera::exportFrameBuffers(Stream *stream,
 
 	return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
 				      ConnectionTypeBlocking, this, stream,
-				      buffers);
+				      buffers, count);
 }
 
 /**
diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
index 2fbba37a1b0b..6b07203017cd 100644
--- a/src/libcamera/framebuffer_allocator.cpp
+++ b/src/libcamera/framebuffer_allocator.cpp
@@ -70,6 +70,7 @@  FrameBufferAllocator::~FrameBufferAllocator()
 /**
  * \brief Allocate buffers for a configured stream
  * \param[in] stream The stream to allocate buffers for
+ * \param[in] count The amount of buffers to allocate
  *
  * Allocate buffers suitable for capturing frames from the \a stream. The Camera
  * shall have been previously configured with Camera::configure() and shall be
@@ -85,14 +86,14 @@  FrameBufferAllocator::~FrameBufferAllocator()
  * not part of the active camera configuration
  * \retval -EBUSY Buffers are already allocated for the \a stream
  */
-int FrameBufferAllocator::allocate(Stream *stream)
+int FrameBufferAllocator::allocate(Stream *stream, unsigned int count)
 {
 	if (buffers_.count(stream)) {
 		LOG(Allocator, Error) << "Buffers already allocated for stream";
 		return -EBUSY;
 	}
 
-	int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);
+	int ret = camera_->exportFrameBuffers(stream, &buffers_[stream], count);
 	if (ret == -EINVAL)
 		LOG(Allocator, Error)
 			<< "Stream is not part of " << camera_->id()
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 519cad4f8148..7a44900b9fbc 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -131,7 +131,8 @@  public:
 	int configure(Camera *camera, CameraConfiguration *config) override;
 
 	int exportFrameBuffers(Camera *camera, Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count) override;
 
 	int start(Camera *camera, const ControlList *controls) override;
 	void stop(Camera *camera) override;
@@ -641,10 +642,12 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 }
 
 int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
-					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+					    std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+					    unsigned int count)
 {
 	IPU3CameraData *data = cameraData(camera);
-	unsigned int count = stream->configuration().bufferCount;
+	if (!count)
+		count = stream->configuration().bufferCount;
 
 	if (stream == &data->outStream_)
 		return data->imgu_->output_->exportBuffers(count, buffers);
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index f22e286ed87a..3ab27123b1ac 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -250,7 +250,8 @@  public:
 	int configure(Camera *camera, CameraConfiguration *config) override;
 
 	int exportFrameBuffers(Camera *camera, Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count) override;
 
 	int start(Camera *camera, const ControlList *controls) override;
 	void stop(Camera *camera) override;
@@ -786,10 +787,12 @@  int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
 }
 
 int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
-					   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+					   std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+					   unsigned int count)
 {
 	RPi::Stream *s = static_cast<RPi::Stream *>(stream);
-	unsigned int count = stream->configuration().bufferCount;
+	if (!count)
+		count = stream->configuration().bufferCount;
 	int ret = s->dev()->exportBuffers(count, buffers);
 
 	s->setExportedBuffers(buffers);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 549f4a4e61a8..4cc3c7739251 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -140,7 +140,8 @@  public:
 	int configure(Camera *camera, CameraConfiguration *config) override;
 
 	int exportFrameBuffers(Camera *camera, Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count) override;
 
 	int start(Camera *camera, const ControlList *controls) override;
 	void stop(Camera *camera) override;
@@ -666,10 +667,12 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 }
 
 int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
-					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+					      std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+					      unsigned int count)
 {
 	RkISP1CameraData *data = cameraData(camera);
-	unsigned int count = stream->configuration().bufferCount;
+	if (!count)
+		count = stream->configuration().bufferCount;
 
 	if (stream == &data->mainPathStream_)
 		return mainPath_.exportBuffers(count, buffers);
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index f6095d38e97a..e9b0698813fc 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -223,7 +223,8 @@  public:
 	int configure(Camera *camera, CameraConfiguration *config) override;
 
 	int exportFrameBuffers(Camera *camera, Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count) override;
 
 	int start(Camera *camera, const ControlList *controls) override;
 	void stop(Camera *camera) override;
@@ -762,10 +763,12 @@  int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
 }
 
 int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
-					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+					      std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+					      unsigned int count)
 {
 	SimpleCameraData *data = cameraData(camera);
-	unsigned int count = stream->configuration().bufferCount;
+	if (!count)
+		count = stream->configuration().bufferCount;
 
 	/*
 	 * Export buffers on the converter or capture video node, depending on
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index b6c6ade5ebaf..298e7031d23b 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -70,7 +70,8 @@  public:
 	int configure(Camera *camera, CameraConfiguration *config) override;
 
 	int exportFrameBuffers(Camera *camera, Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count) override;
 
 	int start(Camera *camera, const ControlList *controls) override;
 	void stop(Camera *camera) override;
@@ -224,10 +225,12 @@  int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
 }
 
 int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
-					   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+					   std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+					   unsigned int count)
 {
 	UVCCameraData *data = cameraData(camera);
-	unsigned int count = stream->configuration().bufferCount;
+	if (!count)
+		count = stream->configuration().bufferCount;
 
 	return data->video_->exportBuffers(count, buffers);
 }
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 8f5f4ba30953..2f889347b624 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -84,7 +84,8 @@  public:
 	int configure(Camera *camera, CameraConfiguration *config) override;
 
 	int exportFrameBuffers(Camera *camera, Stream *stream,
-			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
+			       std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+			       unsigned int count) override;
 
 	int start(Camera *camera, const ControlList *controls) override;
 	void stop(Camera *camera) override;
@@ -299,10 +300,12 @@  int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
 }
 
 int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
-					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+					    std::vector<std::unique_ptr<FrameBuffer>> *buffers,
+					    unsigned int count)
 {
 	VimcCameraData *data = cameraData(camera);
-	unsigned int count = stream->configuration().bufferCount;
+	if (!count)
+		count = stream->configuration().bufferCount;
 
 	return data->video_->exportBuffers(count, buffers);
 }
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 3b3150bdbbf7..ab5f21a01438 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -323,6 +323,7 @@  const ControlList &PipelineHandler::properties(const Camera *camera) const
  * \param[in] camera The camera
  * \param[in] stream The stream to allocate buffers for
  * \param[out] buffers Array of buffers successfully allocated
+ * \param[in] count The amount of buffers to allocate
  *
  * This method allocates buffers for the \a stream from the devices associated
  * with the stream in the corresponding pipeline handler. Those buffers shall be
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 39d034de6bb2..4e1dbb0656c8 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -463,7 +463,7 @@  int MainWindow::startCapture()
 	for (StreamConfiguration &config : *config_) {
 		Stream *stream = config.stream();
 
-		ret = allocator_->allocate(stream);
+		ret = allocator_->allocate(stream, 0);
 		if (ret < 0) {
 			qWarning() << "Failed to allocate capture buffers";
 			goto error;
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index c4bc21100777..6cbca15b30bc 100644
--- a/test/camera/capture.cpp
+++ b/test/camera/capture.cpp
@@ -96,7 +96,7 @@  protected:
 
 		Stream *stream = cfg.stream();
 
-		int ret = allocator_->allocate(stream);
+		int ret = allocator_->allocate(stream, 0);
 		if (ret < 0)
 			return TestFail;
 
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index f0c3d7764027..9e076d05bc6f 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -118,7 +118,7 @@  protected:
 		/* Use internally allocated buffers. */
 		allocator_ = new FrameBufferAllocator(camera_);
 		Stream *stream = *camera_->streams().begin();
-		if (allocator_->allocate(stream) < 0)
+		if (allocator_->allocate(stream, 0) < 0)
 			return TestFail;
 
 		if (camera_->start())
diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp
index 5de8201e45f6..f5c2e2c0169a 100644
--- a/test/mapped-buffer.cpp
+++ b/test/mapped-buffer.cpp
@@ -54,7 +54,7 @@  protected:
 
 		stream_ = cfg.stream();
 
-		int ret = allocator_->allocate(stream_);
+		int ret = allocator_->allocate(stream_, 0);
 		if (ret < 0)
 			return TestFail;