| Message ID | 20210421165139.318432-3-nfraprado@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Nícolas, Thank you for the patch. On Wed, Apr 21, 2021 at 01:51:37PM -0300, Nícolas F. R. A. Prado wrote: > Make FrameBufferAllocator::allocate() require a 'count' argument for the > amount of buffers to be allocated. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > include/libcamera/camera.h | 2 +- > include/libcamera/framebuffer_allocator.h | 2 +- > include/libcamera/internal/pipeline_handler.h | 2 +- > src/cam/capture.cpp | 10 ++++------ > src/gstreamer/gstlibcameraallocator.cpp | 4 +++- > src/lc-compliance/simple_capture.cpp | 13 +++++++++++-- > src/lc-compliance/simple_capture.h | 1 + > src/lc-compliance/single_stream.cpp | 6 ++++++ > 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 | 4 +++- > src/v4l2/v4l2_camera.cpp | 2 +- > test/camera/capture.cpp | 4 +++- > test/camera/statemachine.cpp | 4 +++- > test/mapped-buffer.cpp | 4 +++- > 22 files changed, 63 insertions(+), 35 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index d71641805c0a..656cd92aecde 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -115,7 +115,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 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..dc04ba041fe5 100644 > --- a/include/libcamera/internal/pipeline_handler.h > +++ b/include/libcamera/internal/pipeline_handler.h > @@ -75,7 +75,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/cam/capture.cpp b/src/cam/capture.cpp > index 7b55fc677022..8ad121f5adc8 100644 > --- a/src/cam/capture.cpp > +++ b/src/cam/capture.cpp > @@ -10,6 +10,8 @@ > #include <limits.h> > #include <sstream> > > +#include <libcamera/property_ids.h> > + > #include "capture.h" > #include "main.h" > > @@ -77,17 +79,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::QueueDepth); > for (StreamConfiguration &cfg : *config_) { > - ret = allocator->allocate(cfg.stream()); > + ret = allocator->allocate(cfg.stream(), nbuffers); > 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); If not enough memory is available to allocate nbuffers, or if a driver decides that the number is too high, we could receive less buffers than requested. We would then crash below, when creating the requests. I think we still need to handle this case. > } > > /* > diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp > index 7bd8ba2db71d..e7a5b5a27226 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)); > + int bufferCount = camera->properties().get(properties::QueueDepth); It's an unsigned int above, should it be unsigned here too ? Same for lc-compliance and other places below. > > 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 64e862a08e3a..875772a80c27 100644 > --- a/src/lc-compliance/simple_capture.cpp > +++ b/src/lc-compliance/simple_capture.cpp > @@ -5,6 +5,8 @@ > * simple_capture.cpp - Simple capture helper > */ > > +#include <libcamera/property_ids.h> > + > #include "simple_capture.h" > > using namespace libcamera; > @@ -39,12 +41,19 @@ Results::Result SimpleCapture::configure(StreamRole role) > return { Results::Pass, "Configure camera" }; > } > > -Results::Result SimpleCapture::start() > +Results::Result SimpleCapture::allocateBuffers() > { > + int minBuffers = camera_->properties().get(properties::QueueDepth); Depending on how you decide to document and name QueueDepth (see the review of 1/4), this variable may be better named numBuffers (or something else). > Stream *stream = config_->at(0).stream(); > - if (allocator_->allocate(stream) < 0) > + > + if (allocator_->allocate(stream, minBuffers) < 0) > return { Results::Fail, "Failed to allocate buffers" }; > > + return { Results::Pass, "Allocated buffers" }; Do we need a message when returning Pass ? > +} > + > +Results::Result SimpleCapture::start() > +{ > if (camera_->start()) > return { Results::Fail, "Failed to start camera" }; > > diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h > index d9de53fb63a3..82e2c56a55f1 100644 > --- a/src/lc-compliance/simple_capture.h > +++ b/src/lc-compliance/simple_capture.h > @@ -18,6 +18,7 @@ class SimpleCapture > { > public: > Results::Result configure(libcamera::StreamRole role); > + Results::Result allocateBuffers(); > > protected: > SimpleCapture(std::shared_ptr<libcamera::Camera> camera); > diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp > index 8318b42f42d6..649291c7bb73 100644 > --- a/src/lc-compliance/single_stream.cpp > +++ b/src/lc-compliance/single_stream.cpp > @@ -23,6 +23,9 @@ Results::Result testRequestBalance(std::shared_ptr<Camera> camera, > return ret; > > for (unsigned int starts = 0; starts < startCycles; starts++) { > + capture.allocateBuffers(); > + if (ret.first != Results::Pass) > + return ret; > ret = capture.capture(numRequests); > if (ret.first != Results::Pass) > return ret; > @@ -39,6 +42,9 @@ Results::Result testRequestUnbalance(std::shared_ptr<Camera> camera, > SimpleCaptureUnbalanced capture(camera); > > Results::Result ret = capture.configure(role); > + if (ret.first != Results::Pass) > + return ret; > + capture.allocateBuffers(); > if (ret.first != Results::Pass) > return ret; > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 763f3b9926fd..e4da6e042bd0 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -656,7 +656,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 = LIBCAMERA_D_PTR(); > @@ -673,7 +673,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 2fbba37a1b0b..92d121f8c4ac 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 s/amount/number/ Should this be also updated to explicitly mention that the number of allocated buffers may be different than count ? Same for the PipelineHandler class below. > * > * 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, 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 6067db2f37a3..c8fcc2fda75f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -130,7 +130,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; > @@ -641,10 +641,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 8d1ade3a4352..3f35596fe550 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -249,7 +249,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; > @@ -786,10 +786,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 7d876e9387d7..2d95c1ca2a43 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -140,7 +140,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; > @@ -667,10 +667,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 6ee24f2f14e8..b9f14be6733f 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -225,7 +225,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; > @@ -764,10 +764,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 591f46b60d23..a148c35f1265 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 605b3fe89152..22d6fdcbb141 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 3b3150bdbbf7..cdd456c599a4 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -322,6 +322,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 amount of buffers to allocate s/amount/number/ Very nice patch overall, I think it really improves the API. > * \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..d2ddd976584a 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,8 @@ int MainWindow::startCapture() > for (StreamConfiguration &config : *config_) { > Stream *stream = config.stream(); > > - ret = allocator_->allocate(stream); > + int minBuffers = camera_->properties().get(properties::QueueDepth); > + ret = allocator_->allocate(stream, minBuffers); > 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 97825c715bba..53d97f3e6b86 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 c4bc21100777..24ffbd8b2f6f 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/internal/event_dispatcher.h" > #include "libcamera/internal/thread.h" > @@ -96,7 +97,8 @@ protected: > > Stream *stream = cfg.stream(); > > - int ret = allocator_->allocate(stream); > + int minBuffers = camera_->properties().get(properties::QueueDepth); > + int ret = allocator_->allocate(stream, minBuffers); > if (ret < 0) > return TestFail; > > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp > index f0c3d7764027..80caa897bcaa 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" > @@ -118,7 +119,8 @@ protected: > /* Use internally allocated buffers. */ > allocator_ = new FrameBufferAllocator(camera_); > Stream *stream = *camera_->streams().begin(); > - if (allocator_->allocate(stream) < 0) > + int minBuffers = camera_->properties().get(properties::QueueDepth); > + if (allocator_->allocate(stream, minBuffers) < 0) > return TestFail; > > if (camera_->start()) > diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp > index 5de8201e45f6..7c0264157229 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/buffer.h" > > @@ -54,7 +55,8 @@ protected: > > stream_ = cfg.stream(); > > - int ret = allocator_->allocate(stream_); > + int minBuffers = camera_->properties().get(properties::QueueDepth); > + int ret = allocator_->allocate(stream_, minBuffers); > if (ret < 0) > return TestFail; >
Em 2021-04-25 23:59, Laurent Pinchart escreveu: > Hi Nícolas, > > Thank you for the patch. > > On Wed, Apr 21, 2021 at 01:51:37PM -0300, Nícolas F. R. A. Prado wrote: > > Make FrameBufferAllocator::allocate() require a 'count' argument for the > > amount of buffers to be allocated. > > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > --- > > include/libcamera/camera.h | 2 +- > > include/libcamera/framebuffer_allocator.h | 2 +- > > include/libcamera/internal/pipeline_handler.h | 2 +- > > src/cam/capture.cpp | 10 ++++------ > > src/gstreamer/gstlibcameraallocator.cpp | 4 +++- > > src/lc-compliance/simple_capture.cpp | 13 +++++++++++-- > > src/lc-compliance/simple_capture.h | 1 + > > src/lc-compliance/single_stream.cpp | 6 ++++++ > > 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 | 4 +++- > > src/v4l2/v4l2_camera.cpp | 2 +- > > test/camera/capture.cpp | 4 +++- > > test/camera/statemachine.cpp | 4 +++- > > test/mapped-buffer.cpp | 4 +++- > > 22 files changed, 63 insertions(+), 35 deletions(-) > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index d71641805c0a..656cd92aecde 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -115,7 +115,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 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..dc04ba041fe5 100644 > > --- a/include/libcamera/internal/pipeline_handler.h > > +++ b/include/libcamera/internal/pipeline_handler.h > > @@ -75,7 +75,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/cam/capture.cpp b/src/cam/capture.cpp > > index 7b55fc677022..8ad121f5adc8 100644 > > --- a/src/cam/capture.cpp > > +++ b/src/cam/capture.cpp > > @@ -10,6 +10,8 @@ > > #include <limits.h> > > #include <sstream> > > > > +#include <libcamera/property_ids.h> > > + > > #include "capture.h" > > #include "main.h" > > > > @@ -77,17 +79,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::QueueDepth); > > for (StreamConfiguration &cfg : *config_) { > > - ret = allocator->allocate(cfg.stream()); > > + ret = allocator->allocate(cfg.stream(), nbuffers); > > 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); > > If not enough memory is available to allocate nbuffers, or if a driver > decides that the number is too high, we could receive less buffers than > requested. We would then crash below, when creating the requests. I > think we still need to handle this case. Hm, but doesn't V4L2VideoDevice::requestBuffers() catch that in if (rb.count < count) { LOG(V4L2, Error) << "Not enough buffers provided by V4L2VideoDevice"; requestBuffers(0, memoryType); return -ENOMEM; } ? That return value is returned all the way up which means Capture::capture() will return as well. Not so much related to this but I just realized it: Isn't it a problem that the QueueDepth is a property global to the camera, instead of specific to a stream? > > > } > > > > /* > > diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp > > index 7bd8ba2db71d..e7a5b5a27226 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)); > > + int bufferCount = camera->properties().get(properties::QueueDepth); > > It's an unsigned int above, should it be unsigned here too ? Same for > lc-compliance and other places below. Sure. I didn't use unsigned int because int32_t in the property_ids.yaml sounds like signed. I'll change everything to unsigned. > > > > > 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 64e862a08e3a..875772a80c27 100644 > > --- a/src/lc-compliance/simple_capture.cpp > > +++ b/src/lc-compliance/simple_capture.cpp > > @@ -5,6 +5,8 @@ > > * simple_capture.cpp - Simple capture helper > > */ > > > > +#include <libcamera/property_ids.h> > > + > > #include "simple_capture.h" > > > > using namespace libcamera; > > @@ -39,12 +41,19 @@ Results::Result SimpleCapture::configure(StreamRole role) > > return { Results::Pass, "Configure camera" }; > > } > > > > -Results::Result SimpleCapture::start() > > +Results::Result SimpleCapture::allocateBuffers() > > { > > + int minBuffers = camera_->properties().get(properties::QueueDepth); > > Depending on how you decide to document and name QueueDepth (see the > review of 1/4), this variable may be better named numBuffers (or > something else). > > > Stream *stream = config_->at(0).stream(); > > - if (allocator_->allocate(stream) < 0) > > + > > + if (allocator_->allocate(stream, minBuffers) < 0) > > return { Results::Fail, "Failed to allocate buffers" }; > > > > + return { Results::Pass, "Allocated buffers" }; > > Do we need a message when returning Pass ? Well, the Pass message gets used to report which test passed, although since this is only part of the test it doesn't get used. In any case the return type requires a string, so at least an empty string should be returned (but then, might as well give a meaningful message). Thanks, Nícolas
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index d71641805c0a..656cd92aecde 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -115,7 +115,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 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..dc04ba041fe5 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -75,7 +75,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/cam/capture.cpp b/src/cam/capture.cpp index 7b55fc677022..8ad121f5adc8 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -10,6 +10,8 @@ #include <limits.h> #include <sstream> +#include <libcamera/property_ids.h> + #include "capture.h" #include "main.h" @@ -77,17 +79,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::QueueDepth); for (StreamConfiguration &cfg : *config_) { - ret = allocator->allocate(cfg.stream()); + ret = allocator->allocate(cfg.stream(), nbuffers); 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); } /* diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp index 7bd8ba2db71d..e7a5b5a27226 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)); + int bufferCount = camera->properties().get(properties::QueueDepth); 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 64e862a08e3a..875772a80c27 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -5,6 +5,8 @@ * simple_capture.cpp - Simple capture helper */ +#include <libcamera/property_ids.h> + #include "simple_capture.h" using namespace libcamera; @@ -39,12 +41,19 @@ Results::Result SimpleCapture::configure(StreamRole role) return { Results::Pass, "Configure camera" }; } -Results::Result SimpleCapture::start() +Results::Result SimpleCapture::allocateBuffers() { + int minBuffers = camera_->properties().get(properties::QueueDepth); Stream *stream = config_->at(0).stream(); - if (allocator_->allocate(stream) < 0) + + if (allocator_->allocate(stream, minBuffers) < 0) return { Results::Fail, "Failed to allocate buffers" }; + return { Results::Pass, "Allocated buffers" }; +} + +Results::Result SimpleCapture::start() +{ if (camera_->start()) return { Results::Fail, "Failed to start camera" }; diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index d9de53fb63a3..82e2c56a55f1 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -18,6 +18,7 @@ class SimpleCapture { public: Results::Result configure(libcamera::StreamRole role); + Results::Result allocateBuffers(); protected: SimpleCapture(std::shared_ptr<libcamera::Camera> camera); diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp index 8318b42f42d6..649291c7bb73 100644 --- a/src/lc-compliance/single_stream.cpp +++ b/src/lc-compliance/single_stream.cpp @@ -23,6 +23,9 @@ Results::Result testRequestBalance(std::shared_ptr<Camera> camera, return ret; for (unsigned int starts = 0; starts < startCycles; starts++) { + capture.allocateBuffers(); + if (ret.first != Results::Pass) + return ret; ret = capture.capture(numRequests); if (ret.first != Results::Pass) return ret; @@ -39,6 +42,9 @@ Results::Result testRequestUnbalance(std::shared_ptr<Camera> camera, SimpleCaptureUnbalanced capture(camera); Results::Result ret = capture.configure(role); + if (ret.first != Results::Pass) + return ret; + capture.allocateBuffers(); if (ret.first != Results::Pass) return ret; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 763f3b9926fd..e4da6e042bd0 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -656,7 +656,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 = LIBCAMERA_D_PTR(); @@ -673,7 +673,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 2fbba37a1b0b..92d121f8c4ac 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, 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 6067db2f37a3..c8fcc2fda75f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -130,7 +130,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; @@ -641,10 +641,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 8d1ade3a4352..3f35596fe550 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -249,7 +249,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; @@ -786,10 +786,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 7d876e9387d7..2d95c1ca2a43 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -140,7 +140,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; @@ -667,10 +667,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 6ee24f2f14e8..b9f14be6733f 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -225,7 +225,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; @@ -764,10 +764,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 591f46b60d23..a148c35f1265 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 605b3fe89152..22d6fdcbb141 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 3b3150bdbbf7..cdd456c599a4 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -322,6 +322,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 amount 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..d2ddd976584a 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,8 @@ int MainWindow::startCapture() for (StreamConfiguration &config : *config_) { Stream *stream = config.stream(); - ret = allocator_->allocate(stream); + int minBuffers = camera_->properties().get(properties::QueueDepth); + ret = allocator_->allocate(stream, minBuffers); 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 97825c715bba..53d97f3e6b86 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 c4bc21100777..24ffbd8b2f6f 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/internal/event_dispatcher.h" #include "libcamera/internal/thread.h" @@ -96,7 +97,8 @@ protected: Stream *stream = cfg.stream(); - int ret = allocator_->allocate(stream); + int minBuffers = camera_->properties().get(properties::QueueDepth); + int ret = allocator_->allocate(stream, minBuffers); if (ret < 0) return TestFail; diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index f0c3d7764027..80caa897bcaa 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" @@ -118,7 +119,8 @@ protected: /* Use internally allocated buffers. */ allocator_ = new FrameBufferAllocator(camera_); Stream *stream = *camera_->streams().begin(); - if (allocator_->allocate(stream) < 0) + int minBuffers = camera_->properties().get(properties::QueueDepth); + if (allocator_->allocate(stream, minBuffers) < 0) return TestFail; if (camera_->start()) diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp index 5de8201e45f6..7c0264157229 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/buffer.h" @@ -54,7 +55,8 @@ protected: stream_ = cfg.stream(); - int ret = allocator_->allocate(stream_); + int minBuffers = camera_->properties().get(properties::QueueDepth); + int ret = allocator_->allocate(stream_, minBuffers); if (ret < 0) return TestFail;
Make FrameBufferAllocator::allocate() require a 'count' argument for the amount of buffers to be allocated. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- include/libcamera/camera.h | 2 +- include/libcamera/framebuffer_allocator.h | 2 +- include/libcamera/internal/pipeline_handler.h | 2 +- src/cam/capture.cpp | 10 ++++------ src/gstreamer/gstlibcameraallocator.cpp | 4 +++- src/lc-compliance/simple_capture.cpp | 13 +++++++++++-- src/lc-compliance/simple_capture.h | 1 + src/lc-compliance/single_stream.cpp | 6 ++++++ 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 | 4 +++- src/v4l2/v4l2_camera.cpp | 2 +- test/camera/capture.cpp | 4 +++- test/camera/statemachine.cpp | 4 +++- test/mapped-buffer.cpp | 4 +++- 22 files changed, 63 insertions(+), 35 deletions(-)