From patchwork Thu Jul 22 23:28:42 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: 13076 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 CFF24C0109 for ; Thu, 22 Jul 2021 23:29:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 936F5687AC; Fri, 23 Jul 2021 01:29:29 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F423D6877B for ; Fri, 23 Jul 2021 01:29:27 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2434:a4c5:aa5e:1d8c:5e2c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 55AAA1F447CE; Fri, 23 Jul 2021 00:29:26 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 22 Jul 2021 20:28:42 -0300 Message-Id: <20210722232851.747614-3-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210722232851.747614-1-nfraprado@collabora.com> References: <20210722232851.747614-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 02/11] 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: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Make FrameBufferAllocator::allocate() require a 'count' argument for the number of buffers to be allocated. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- 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(-) 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> *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); ~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> *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 +#include 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 #include +#include #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); 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..8cc42edfaf61 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)); + 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 +#include + #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(count), bufferCount) << "Allocated less buffers than expected"; 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> *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 @@ -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> *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> *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> *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> *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 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> *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> *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> *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> *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> *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 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> *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 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 #include +#include #include #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 + */ + bufferCount = std::max(bufferCount, 2U); + + 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 #include +#include #include #include @@ -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); 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 #include +#include #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 #include +#include #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;