[libcamera-devel] [PATCH v7 02/11] libcamera: framebuffer_allocator: Make allocate() require count
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Aug 1 19:23:54 CEST 2021
Hello,
On Wed, Jul 28, 2021 at 09:28:36AM +0100, Kieran Bingham wrote:
> On 23/07/2021 00:28, Nícolas F. R. A. Prado wrote:
> > Make FrameBufferAllocator::allocate() require a 'count' argument for the
> > number of buffers to be allocated.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > ---
> >
> > No changes in v7
> >
> > Changes in v6:
> > - Changed static_cast to convert 'count' to unsigned int instead of
> > 'bufferCount' to int when comparing
> >
> > Changes in v5:
> > - Made sure that qcam allocates at least 2 buffers
> >
> > include/libcamera/camera.h | 2 +-
> > include/libcamera/framebuffer_allocator.h | 2 +-
> > include/libcamera/internal/pipeline_handler.h | 2 +-
> > src/android/camera_stream.cpp | 5 ++++-
> > src/cam/capture.cpp | 9 +++------
> > src/gstreamer/gstlibcameraallocator.cpp | 4 +++-
> > src/lc-compliance/simple_capture.cpp | 7 +++++--
> > src/libcamera/camera.cpp | 4 ++--
> > src/libcamera/framebuffer_allocator.cpp | 5 +++--
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--
> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--
> > src/libcamera/pipeline/simple/simple.cpp | 4 ++--
> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 ++++---
> > src/libcamera/pipeline/vimc/vimc.cpp | 7 ++++---
> > src/libcamera/pipeline_handler.cpp | 1 +
> > src/qcam/main_window.cpp | 11 ++++++++++-
> > src/v4l2/v4l2_camera.cpp | 2 +-
> > test/camera/capture.cpp | 4 +++-
> > test/camera/statemachine.cpp | 4 +++-
> > test/mapped-buffer.cpp | 4 +++-
> > 21 files changed, 60 insertions(+), 36 deletions(-)
>
> This patch should also update the Documentation/application-developer
> guide which references the public API in section
> "Using the libcamera ``FrameBufferAllocator``"
>
> It can be a patch on top if it helps, or squashed into this one.
I'm fine with either, it may be easier to keep it on top, but it should
be part of this series.
> With that, I think I quite like this change as it gives better
> flexibility to the application which otherwise might provide buffers
> elsewhere anyway.
>
> With the Documentation fixed, or with that being prepared as a separate
> patch:
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index b081907e0cb1..9f1767e4c406 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -116,7 +116,7 @@ private:
> > void requestComplete(Request *request);
> >
> > friend class FrameBufferAllocator;
> > - int exportFrameBuffers(Stream *stream,
> > + int exportFrameBuffers(Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> > };
> >
> > diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> > index cbc9ce101889..2d5a6e98e10c 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 9e2d65d6f2c5..0b4b2e4947c0 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -76,7 +76,7 @@ public:
> > const StreamRoles &roles) = 0;
> > virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
> >
> > - virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> > + virtual int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> >
> > virtual int start(Camera *camera, const ControlList *controls) = 0;
> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> > index bf4a7b41a70a..b168e3c0c288 100644
> > --- a/src/android/camera_stream.cpp
> > +++ b/src/android/camera_stream.cpp
> > @@ -13,6 +13,7 @@
> > #include "jpeg/post_processor_jpeg.h"
> >
> > #include <libcamera/formats.h>
> > +#include <libcamera/property_ids.h>
> >
> > using namespace libcamera;
> >
> > @@ -81,8 +82,10 @@ int CameraStream::configure()
> > return ret;
> > }
> >
> > + unsigned int bufferCount = cameraDevice_->camera()->properties().get(properties::MinimumRequests);
> > +
> > if (allocator_) {
> > - int ret = allocator_->allocate(stream());
> > + int ret = allocator_->allocate(stream(), bufferCount);
> > if (ret < 0)
> > return ret;
> >
> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > index 3c3e3a53adf7..633f9c6e4f25 100644
> > --- a/src/cam/capture.cpp
> > +++ b/src/cam/capture.cpp
> > @@ -11,6 +11,7 @@
> > #include <sstream>
> >
> > #include <libcamera/control_ids.h>
> > +#include <libcamera/property_ids.h>
> >
> > #include "capture.h"
> > #include "main.h"
> > @@ -81,17 +82,13 @@ int Capture::capture(FrameBufferAllocator *allocator)
> > {
> > int ret;
> >
> > - /* Identify the stream with the least number of buffers. */
> > - unsigned int nbuffers = UINT_MAX;
> > + unsigned int nbuffers = camera_->properties().get(properties::MinimumRequests);
> > for (StreamConfiguration &cfg : *config_) {
> > - ret = allocator->allocate(cfg.stream());
> > + ret = allocator->allocate(cfg.stream(), nbuffers);
Seems that MinimumRequests should be a per-stream property (which we
don't support yet). Could you add a
\todo Should this be a per-stream property?
to patch 01/11 ?
> > if (ret < 0) {
> > std::cerr << "Can't allocate buffers" << std::endl;
> > return -ENOMEM;
> > }
> > -
> > - unsigned int allocated = allocator->buffers(cfg.stream()).size();
> > - nbuffers = std::min(nbuffers, allocated);
Given that the allocator could still return less buffers than requested,
shouldn't we keep this code ? Or are you relying on the (undocumented)
assumption that the allocator will always be able to allocate at least
MinimumRequests ?
> > }
> >
> > /*
> > diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp
> > index 7bd8ba2db71d..8cc42edfaf61 100644
> > --- a/src/gstreamer/gstlibcameraallocator.cpp
> > +++ b/src/gstreamer/gstlibcameraallocator.cpp
> > @@ -10,6 +10,7 @@
> >
> > #include <libcamera/camera.h>
> > #include <libcamera/framebuffer_allocator.h>
> > +#include <libcamera/property_ids.h>
> > #include <libcamera/stream.h>
> >
> > #include "gstlibcamera-utils.h"
> > @@ -188,13 +189,14 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera,
> > {
> > auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,
> > nullptr));
> > + unsigned int bufferCount = camera->properties().get(properties::MinimumRequests);
> >
> > self->fb_allocator = new FrameBufferAllocator(camera);
> > for (StreamConfiguration &streamCfg : *config_) {
> > Stream *stream = streamCfg.stream();
> > gint ret;
> >
> > - ret = self->fb_allocator->allocate(stream);
> > + ret = self->fb_allocator->allocate(stream, bufferCount);
> > if (ret == 0)
> > return nullptr;
> >
> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> > index 25097f28a603..6444203547be 100644
> > --- a/src/lc-compliance/simple_capture.cpp
> > +++ b/src/lc-compliance/simple_capture.cpp
> > @@ -7,6 +7,8 @@
> >
> > #include <gtest/gtest.h>
> >
> > +#include <libcamera/property_ids.h>
> > +
> > #include "simple_capture.h"
> >
> > using namespace libcamera;
> > @@ -44,11 +46,12 @@ void SimpleCapture::configure(StreamRole role)
> >
> > void SimpleCapture::start()
> > {
> > + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests);
> > Stream *stream = config_->at(0).stream();
> > - int count = allocator_->allocate(stream);
> > + int count = allocator_->allocate(stream, bufferCount);
> >
> > ASSERT_GE(count, 0) << "Failed to allocate buffers";
> > - EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected";
> > + EXPECT_EQ(static_cast<unsigned int>(count), bufferCount) << "Allocated less buffers than expected";
Is the cast required ? bufferCount is currently an unsigned int.
> >
> > camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);
> >
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index c8858e71d105..e5a55a674afb 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -660,7 +660,7 @@ void Camera::disconnect()
> > disconnected.emit(this);
> > }
> >
> > -int Camera::exportFrameBuffers(Stream *stream,
> > +int Camera::exportFrameBuffers(Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > Private *const d = _d();
> > @@ -677,7 +677,7 @@ int Camera::exportFrameBuffers(Stream *stream,
> >
> > return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
> > ConnectionTypeBlocking, this, stream,
> > - buffers);
> > + count, buffers);
> > }
> >
> > /**
> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> > index 695073fd1c1c..2fc0db56176a 100644
> > --- a/src/libcamera/framebuffer_allocator.cpp
> > +++ b/src/libcamera/framebuffer_allocator.cpp
> > @@ -71,6 +71,7 @@ FrameBufferAllocator::~FrameBufferAllocator()
> > /**
> > * \brief Allocate buffers for a configured stream
> > * \param[in] stream The stream to allocate buffers for
> > + * \param[in] count The number 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
Could you add a sentence to indicate that the function can allocate less
buffers than requested ?
* This function may allocate less buffers than requested, due to memory and
* other system constraints. The caller shall always check the return value to
* verify if the number of allocate buffers matches its needs.
> > @@ -86,14 +87,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, count, &buffers_[stream]);
> > 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 76c3bb3d8aa9..5fd1757bfe13 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -134,7 +134,7 @@ public:
> > const StreamRoles &roles) override;
> > int configure(Camera *camera, CameraConfiguration *config) override;
> >
> > - int exportFrameBuffers(Camera *camera, Stream *stream,
> > + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> >
> > int start(Camera *camera, const ControlList *controls) override;
> > @@ -654,10 +654,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > }
> >
> > int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> > + unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > IPU3CameraData *data = cameraData(camera);
> > - unsigned int 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 f821d8fe1b6c..d1cd3d9dc082 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -251,7 +251,7 @@ public:
> > CameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override;
> > int configure(Camera *camera, CameraConfiguration *config) override;
> >
> > - int exportFrameBuffers(Camera *camera, Stream *stream,
> > + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> >
> > int start(Camera *camera, const ControlList *controls) override;
> > @@ -795,10 +795,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > }
> >
> > int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> > + unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > RPi::Stream *s = static_cast<RPi::Stream *>(stream);
> > - unsigned int 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 cb4ca9a85fa8..11325875b929 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -141,7 +141,7 @@ public:
> > const StreamRoles &roles) override;
> > int configure(Camera *camera, CameraConfiguration *config) override;
> >
> > - int exportFrameBuffers(Camera *camera, Stream *stream,
> > + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> >
> > int start(Camera *camera, const ControlList *controls) override;
> > @@ -671,10 +671,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > }
> >
> > int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> > + unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > RkISP1CameraData *data = cameraData(camera);
> > - unsigned int 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 348c554c8be4..1c25a7344f5f 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -228,7 +228,7 @@ public:
> > const StreamRoles &roles) override;
> > int configure(Camera *camera, CameraConfiguration *config) override;
> >
> > - int exportFrameBuffers(Camera *camera, Stream *stream,
> > + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> >
> > int start(Camera *camera, const ControlList *controls) override;
> > @@ -776,10 +776,10 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> > }
> >
> > int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> > + unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > SimpleCameraData *data = cameraData(camera);
> > - unsigned int 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 6ad7fb3c9f0c..fd39b3d3c72c 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -69,7 +69,7 @@ public:
> > const StreamRoles &roles) override;
> > int configure(Camera *camera, CameraConfiguration *config) override;
> >
> > - int exportFrameBuffers(Camera *camera, Stream *stream,
> > + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> >
> > int start(Camera *camera, const ControlList *controls) override;
> > @@ -223,11 +223,12 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> > return 0;
> > }
> >
> > -int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> > +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera,
> > + [[maybe_unused]] Stream *stream,
> > + unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > UVCCameraData *data = cameraData(camera);
> > - unsigned int 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 44c198ccf8b8..e89d53182c6d 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -84,7 +84,7 @@ public:
> > const StreamRoles &roles) override;
> > int configure(Camera *camera, CameraConfiguration *config) override;
> >
> > - int exportFrameBuffers(Camera *camera, Stream *stream,
> > + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> >
> > int start(Camera *camera, const ControlList *controls) override;
> > @@ -299,11 +299,12 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> > return 0;
> > }
> >
> > -int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> > +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera,
> > + [[maybe_unused]] Stream *stream,
> > + unsigned int count,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > VimcCameraData *data = cameraData(camera);
> > - unsigned int 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 c9928d444b04..dc83bf6924bf 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
> > * \brief Allocate and export buffers for \a stream
> > * \param[in] camera The camera
> > * \param[in] stream The stream to allocate buffers for
> > + * \param[in] count The number of buffers to allocate
> > * \param[out] buffers Array of buffers successfully allocated
> > *
> > * This method allocates buffers for the \a stream from the devices associated
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 39d034de6bb2..d7475b142d6f 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -25,6 +25,7 @@
> > #include <QtDebug>
> >
> > #include <libcamera/camera_manager.h>
> > +#include <libcamera/property_ids.h>
> > #include <libcamera/version.h>
> >
> > #include "dng_writer.h"
> > @@ -463,7 +464,15 @@ int MainWindow::startCapture()
> > for (StreamConfiguration &config : *config_) {
> > Stream *stream = config.stream();
> >
> > - ret = allocator_->allocate(stream);
> > + unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);
> > +
> > + /*
> > + * Need at least two buffers, one for capture and another for
> > + * display
s/display/display./
> > + */
> > + bufferCount = std::max(bufferCount, 2U);
This requires inclusion of <cmath>.
I think we should use bufferCount + 1, not max(bufferCount, 2). qcam
holds on one buffer for display. If the camera needs, let's say, 2
buffers to be able to capture, it will return a first buffer which will
then be queued to the display. At that point, the camera will have a
single buffer, and will hold onto it until a new buffer is queued. This
will never happen, as the buffer being displayed won't be returned to
libcamera until a new buffer is ready. We thus need three buffers in
that case.
Thinking even more about this, I think we need
bufferCount = std::max(bufferCount + 1, 3U);
Even if the camera needs a single buffer only (in which case we would
use two buffers, one for capture and one for display), it's clear that
it will lead to frame drops. A minimum of 3 buffers is thus better.
The cam application should also have a similar logic. It doesn't hold
onto a buffer, so in that case
bufferCount = std::max(bufferCount, 2U);
should be good enough to start with.
> > +
> > + ret = allocator_->allocate(stream, bufferCount);
> > if (ret < 0) {
> > qWarning() << "Failed to allocate capture buffers";
> > goto error;
> > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> > index 157ab94e0544..d01eacfa2b84 100644
> > --- a/src/v4l2/v4l2_camera.cpp
> > +++ b/src/v4l2/v4l2_camera.cpp
> > @@ -161,7 +161,7 @@ int V4L2Camera::allocBuffers(unsigned int count)
> > {
> > Stream *stream = config_->at(0).stream();
> >
> > - int ret = bufferAllocator_->allocate(stream);
> > + int ret = bufferAllocator_->allocate(stream, count);
> > if (ret < 0)
> > return ret;
> >
> > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > index 238d98dbba16..2a531805cb2b 100644
> > --- a/test/camera/capture.cpp
> > +++ b/test/camera/capture.cpp
> > @@ -8,6 +8,7 @@
> > #include <iostream>
> >
> > #include <libcamera/framebuffer_allocator.h>
> > +#include <libcamera/property_ids.h>
> >
> > #include <libcamera/base/event_dispatcher.h>
> > #include <libcamera/base/thread.h>
> > @@ -96,7 +97,8 @@ protected:
> >
> > Stream *stream = cfg.stream();
> >
> > - int ret = allocator_->allocate(stream);
> > + unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);
> > + int ret = allocator_->allocate(stream, bufferCount);
Similarly, this will result in frame drop at least with some pipeline
handlers. I fear we need to go one step further and already design the
API to tell applications how many buffers to allocate to avoid frame
draps. Per-frame control can be left out for now.
> > if (ret < 0)
> > return TestFail;
> >
> > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> > index 26fb5ca17139..70015bb56c74 100644
> > --- a/test/camera/statemachine.cpp
> > +++ b/test/camera/statemachine.cpp
> > @@ -8,6 +8,7 @@
> > #include <iostream>
> >
> > #include <libcamera/framebuffer_allocator.h>
> > +#include <libcamera/property_ids.h>
> >
> > #include "camera_test.h"
> > #include "test.h"
> > @@ -119,7 +120,8 @@ protected:
> > /* Use internally allocated buffers. */
> > allocator_ = new FrameBufferAllocator(camera_);
> > Stream *stream = *camera_->streams().begin();
> > - if (allocator_->allocate(stream) < 0)
> > + unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);
> > + if (allocator_->allocate(stream, bufferCount) < 0)
> > return TestFail;
> >
> > if (camera_->start())
> > diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp
> > index c9479194cb68..e01211965364 100644
> > --- a/test/mapped-buffer.cpp
> > +++ b/test/mapped-buffer.cpp
> > @@ -8,6 +8,7 @@
> > #include <iostream>
> >
> > #include <libcamera/framebuffer_allocator.h>
> > +#include <libcamera/property_ids.h>
> >
> > #include "libcamera/internal/framebuffer.h"
> >
> > @@ -54,7 +55,8 @@ protected:
> >
> > stream_ = cfg.stream();
> >
> > - int ret = allocator_->allocate(stream_);
> > + unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);
> > + int ret = allocator_->allocate(stream_, bufferCount);
> > if (ret < 0)
> > return TestFail;
> >
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list