From patchwork Wed Apr 21 16:51:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 12051 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 26F77BDB15 for ; Wed, 21 Apr 2021 16:52:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E696A68857; Wed, 21 Apr 2021 18:52:26 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7643368835 for ; Wed, 21 Apr 2021 18:52:25 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 38CB31F423EC From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Wed, 21 Apr 2021 13:51:37 -0300 Message-Id: <20210421165139.318432-3-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210421165139.318432-1-nfraprado@collabora.com> References: <20210421165139.318432-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 2/4] libcamera: framebuffer_allocator: Make allocate() require count X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?utf-8?q?Andr=C3=A9_Almeida?= , kernel@collabora.com Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Make FrameBufferAllocator::allocate() require a 'count' argument for the amount of buffers to be allocated. Signed-off-by: Nícolas F. R. A. Prado --- 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> *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); ~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> *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 #include +#include + #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 #include +#include #include #include "gstlibcamera-utils.h" @@ -188,13 +189,14 @@ gst_libcamera_allocator_new(std::shared_ptr 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 + #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 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, 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, 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> *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> *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> *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> *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> *buffers) { RPi::Stream *s = static_cast(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> *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> *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> *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> *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> *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> *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> *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> *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 #include +#include #include #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 #include +#include #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 #include +#include #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 #include +#include #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;