From patchwork Thu May 6 18:02:44 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: 12210 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 A6465BDE7F for ; Thu, 6 May 2021 18:03:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7075A6891D; Thu, 6 May 2021 20:03:56 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D182C68915 for ; Thu, 6 May 2021 20:03:54 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:995d:672b:100f:2fd9]) (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 1BA621F43B8B; Thu, 6 May 2021 19:03:52 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 6 May 2021 15:02:44 -0300 Message-Id: <20210506180249.318346-2-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210506180249.318346-1-nfraprado@collabora.com> References: <20210506180249.318346-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 1/6] libcamera: property: Add MinNumRequests property 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" The MinNumRequests property reports the bare minimum number of requests needed in the camera pipeline for capture to be possible. It is equal to the number of buffers required by the driver. At this quantity, there's no guarantee that frames won't be dropped or that manual per-frame controls will apply correctly. The quantity needed for those may be added as separate properties in the future. Signed-off-by: Nícolas F. R. A. Prado --- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++++++ src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 ++++++ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++ src/libcamera/pipeline/simple/simple.cpp | 2 ++ src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 ++ src/libcamera/pipeline/vimc/vimc.cpp | 3 +++ src/libcamera/property_ids.yaml | 9 +++++++++ 7 files changed, 30 insertions(+) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ade8ffbddb0e..13d7971da409 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1049,6 +1049,12 @@ int PipelineHandlerIPU3::registerCameras() /* Initialize the camera properties. */ data->properties_ = cio2->sensor()->properties(); + /* + * \todo Make sure this is correct even after the stream is + * configured to CIO2 + */ + data->properties_.set(properties::MinNumRequests, 1); + ret = initControls(data.get()); if (ret) continue; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index b22564938cdc..ccc8a2bedcc6 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1035,6 +1035,12 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + /* + * \todo Make sure this is correct after the stream is configured for + * any of the roles + */ + data->properties_.set(properties::MinNumRequests, 1); + /* * Set a default value for the ScalerCropMaximum property to show * that we support its use, however, initialise it to zero because diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index adebe8b5c8cd..9a1b6d4f239f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -943,6 +944,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + data->properties_.set(properties::MinNumRequests, 3); /* * \todo Read dealy values from the sensor itself or from a diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index e1ee640b0a92..42c53af28097 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -425,6 +426,7 @@ int SimpleCameraData::init() } properties_ = sensor_->properties(); + properties_.set(properties::MinNumRequests, 3); return 0; } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index faa8d6b05f46..cb8f40dca279 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -525,6 +525,8 @@ int UVCCameraData::init(MediaDevice *media) properties_.set(properties::PixelArraySize, resolution); properties_.set(properties::PixelArrayActiveAreas, { Rectangle(resolution) }); + properties_.set(properties::MinNumRequests, 1); + /* Initialise the supported controls. */ ControlInfoMap::Map ctrls; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index ce83dcaab8ea..ec991083b104 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -516,6 +517,8 @@ int VimcCameraData::init() /* Initialize the camera properties. */ properties_ = sensor_->properties(); + properties_.set(properties::MinNumRequests, 2); + return 0; } diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml index 104e9aaf4fa3..55f96a78d1e7 100644 --- a/src/libcamera/property_ids.yaml +++ b/src/libcamera/property_ids.yaml @@ -678,6 +678,15 @@ controls: \todo Turn this property into a "maximum control value" for the ScalerCrop control once "dynamic" controls have been implemented. + - MinNumRequests: + type: int32_t + description: | + The bare minimum number of requests needed in the camera pipeline for + capture to be possible, as required by the driver. Note that this + quantity does not guarantee that frames won't be dropped or that manual + per-frame controls will be applied properly. + + # ---------------------------------------------------------------------------- # Draft properties section From patchwork Thu May 6 18:02:45 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: 12211 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 2809CBDE7F for ; Thu, 6 May 2021 18:03:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E9B196890C; Thu, 6 May 2021 20:03:58 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 04A9268901 for ; Thu, 6 May 2021 20:03:57 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:995d:672b:100f:2fd9]) (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 350151F43B91; Thu, 6 May 2021 19:03:55 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 6 May 2021 15:02:45 -0300 Message-Id: <20210506180249.318346-3-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210506180249.318346-1-nfraprado@collabora.com> References: <20210506180249.318346-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 2/6] 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 --- 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 | 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 +++- 23 files changed, 66 insertions(+), 36 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 31dadf285a58..31cfa91cbc9e 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/android/camera_stream.cpp b/src/android/camera_stream.cpp index b2f03b505199..859c8807e585 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::MinNumRequests); + 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..584097fae38c 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::MinNumRequests); 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..37fe809bc575 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::MinNumRequests); 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..f961e71f1815 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() { + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests); Stream *stream = config_->at(0).stream(); - if (allocator_->allocate(stream) < 0) + + if (allocator_->allocate(stream, bufferCount) < 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 1340c266cc5f..5ed69a74582d 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..4ad035be8b71 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 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 @@ -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 13d7971da409..d9e0d8a32afb 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 ccc8a2bedcc6..56eaf77c7566 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 9a1b6d4f239f..d8cc7c2c9fb5 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; @@ -670,10 +670,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 42c53af28097..c949efe5c422 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -224,7 +224,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; @@ -765,10 +765,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 cb8f40dca279..1f77629bb8fc 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 ec991083b104..82e520dd7162 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 f41b7a7b3308..5151678ca630 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 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..5db1b880dec3 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); + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests); + 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 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..848492ab27f7 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); + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests); + int ret = allocator_->allocate(stream, bufferCount); if (ret < 0) return TestFail; diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index f0c3d7764027..c91c6434e2db 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) + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests); + if (allocator_->allocate(stream, bufferCount) < 0) return TestFail; if (camera_->start()) diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp index 5de8201e45f6..c6b9e5882045 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_); + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests); + int ret = allocator_->allocate(stream_, bufferCount); if (ret < 0) return TestFail; From patchwork Thu May 6 18:02:46 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: 12212 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 5DA4BBDE7F for ; Thu, 6 May 2021 18:04:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2055868923; Thu, 6 May 2021 20:04:01 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A7C668921 for ; Thu, 6 May 2021 20:03:59 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:995d:672b:100f:2fd9]) (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 4F5101F43B8F; Thu, 6 May 2021 19:03:57 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 6 May 2021 15:02:46 -0300 Message-Id: <20210506180249.318346-4-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210506180249.318346-1-nfraprado@collabora.com> References: <20210506180249.318346-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/6] lc-compliance: Factor common capture code into SimpleCapture 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" Factor common code from SimpleCapture* subclasses into the SimpleCapture parent class in order to avoid duplicated code. Signed-off-by: Nícolas F. R. A. Prado --- src/lc-compliance/simple_capture.cpp | 142 ++++++++++++++++----------- src/lc-compliance/simple_capture.h | 16 ++- 2 files changed, 95 insertions(+), 63 deletions(-) diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index f961e71f1815..bb173ef1f71a 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -73,6 +73,53 @@ void SimpleCapture::stop() allocator_->free(stream); } +bool SimpleCapture::captureCompleted() +{ + captureCount_++; + if (captureCount_ >= captureLimit_) { + loop_->exit(0); + return true; + } + return false; +} + +Results::Result SimpleCapture::queueRequests(Stream *stream, + std::vector> &requests, + const std::vector> &buffers) +{ + for (const std::unique_ptr &buffer : buffers) { + std::unique_ptr request = camera_->createRequest(); + if (!request) { + stop(); + return { Results::Fail, "Can't create request" }; + } + + if (request->addBuffer(stream, buffer.get())) { + stop(); + return { Results::Fail, "Can't set buffer for request" }; + } + + if (camera_->queueRequest(request.get()) < 0) { + stop(); + return { Results::Fail, "Failed to queue request" }; + } + + requests.push_back(std::move(request)); + } + + return { Results::Pass, "Succesfully queued requests" }; +} + +int SimpleCapture::runCaptureSession() +{ + loop_ = new EventLoop(); + int status = loop_->exec(); + stop(); + delete loop_; + + return status; +} + /* SimpleCaptureBalanced */ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr camera) @@ -80,6 +127,33 @@ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr camera) { } +Results::Result SimpleCaptureBalanced::queueRequests(Stream *stream, + std::vector> &requests, + const std::vector> &buffers) +{ + for (const std::unique_ptr &buffer : buffers) { + std::unique_ptr request = camera_->createRequest(); + if (!request) { + stop(); + return { Results::Fail, "Can't create request" }; + } + + if (request->addBuffer(stream, buffer.get())) { + stop(); + return { Results::Fail, "Can't set buffer for request" }; + } + + if (queueRequest(request.get()) < 0) { + stop(); + return { Results::Fail, "Failed to queue request" }; + } + + requests.push_back(std::move(request)); + } + + return { Results::Pass, "Succesfully queued requests" }; +} + Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) { Results::Result ret = start(); @@ -103,33 +177,12 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) captureCount_ = 0; captureLimit_ = numRequests; - /* Queue the recommended number of reqeuests. */ std::vector> requests; - for (const std::unique_ptr &buffer : buffers) { - std::unique_ptr request = camera_->createRequest(); - if (!request) { - stop(); - return { Results::Fail, "Can't create request" }; - } - - if (request->addBuffer(stream, buffer.get())) { - stop(); - return { Results::Fail, "Can't set buffer for request" }; - } - - if (queueRequest(request.get()) < 0) { - stop(); - return { Results::Fail, "Failed to queue request" }; - } + ret = queueRequests(stream, requests, buffers); + if (ret.first != Results::Pass) + return ret; - requests.push_back(std::move(request)); - } - - /* Run capture session. */ - loop_ = new EventLoop(); - loop_->exec(); - stop(); - delete loop_; + runCaptureSession(); if (captureCount_ != captureLimit_) return { Results::Fail, "Got " + std::to_string(captureCount_) + @@ -150,11 +203,8 @@ int SimpleCaptureBalanced::queueRequest(Request *request) void SimpleCaptureBalanced::requestComplete(Request *request) { - captureCount_++; - if (captureCount_ >= captureLimit_) { - loop_->exit(0); + if (captureCompleted()) return; - } request->reuse(Request::ReuseBuffers); if (queueRequest(request)) @@ -180,44 +230,20 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) captureCount_ = 0; captureLimit_ = numRequests; - /* Queue the recommended number of reqeuests. */ std::vector> requests; - for (const std::unique_ptr &buffer : buffers) { - std::unique_ptr request = camera_->createRequest(); - if (!request) { - stop(); - return { Results::Fail, "Can't create request" }; - } - - if (request->addBuffer(stream, buffer.get())) { - stop(); - return { Results::Fail, "Can't set buffer for request" }; - } - - if (camera_->queueRequest(request.get()) < 0) { - stop(); - return { Results::Fail, "Failed to queue request" }; - } - - requests.push_back(std::move(request)); - } + ret = queueRequests(stream, requests, buffers); + if (ret.first != Results::Pass) + return ret; - /* Run capture session. */ - loop_ = new EventLoop(); - int status = loop_->exec(); - stop(); - delete loop_; + int status = runCaptureSession(); return { status ? Results::Fail : Results::Pass, "Unbalanced capture of " + std::to_string(numRequests) + " requests" }; } void SimpleCaptureUnbalanced::requestComplete(Request *request) { - captureCount_++; - if (captureCount_ >= captureLimit_) { - loop_->exit(0); + if (captureCompleted()) return; - } request->reuse(Request::ReuseBuffers); if (camera_->queueRequest(request)) diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index 82e2c56a55f1..8d0f94f0c3e5 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -26,11 +26,19 @@ protected: Results::Result start(); void stop(); + Results::Result queueRequests(libcamera::Stream *stream, + std::vector> &requests, + const std::vector> &buffers); + bool captureCompleted(); + int runCaptureSession(); virtual void requestComplete(libcamera::Request *request) = 0; EventLoop *loop_; + unsigned int captureCount_; + unsigned int captureLimit_; + std::shared_ptr camera_; std::unique_ptr allocator_; std::unique_ptr config_; @@ -46,10 +54,11 @@ public: private: int queueRequest(libcamera::Request *request); void requestComplete(libcamera::Request *request) override; + Results::Result queueRequests(libcamera::Stream *stream, + std::vector> &requests, + const std::vector> &buffers); unsigned int queueCount_; - unsigned int captureCount_; - unsigned int captureLimit_; }; class SimpleCaptureUnbalanced : public SimpleCapture @@ -61,9 +70,6 @@ public: private: void requestComplete(libcamera::Request *request) override; - - unsigned int captureCount_; - unsigned int captureLimit_; }; #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */ From patchwork Thu May 6 18:02:47 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: 12213 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 DE043BDE7F for ; Thu, 6 May 2021 18:04:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A578568926; Thu, 6 May 2021 20:04:02 +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 30D8D68926 for ; Thu, 6 May 2021 20:04:01 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:995d:672b:100f:2fd9]) (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 6A3B11F43B94; Thu, 6 May 2021 19:03:59 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 6 May 2021 15:02:47 -0300 Message-Id: <20210506180249.318346-5-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210506180249.318346-1-nfraprado@collabora.com> References: <20210506180249.318346-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 4/6] lc-compliance: Add test to queue more requests than hardware depth 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" A pipeline handler should be able to handle an arbitrary number of simultaneous requests by submitting what it can to the video device and queuing the rest internally until resources are available. This isn't currently done by some pipeline handlers however [1]. Add a new test to lc-compliance that submits a lot of requests at once to check if the pipeline handler is behaving well in this situation. [1] https://bugs.libcamera.org/show_bug.cgi?id=24 Signed-off-by: Nícolas F. R. A. Prado --- src/lc-compliance/simple_capture.cpp | 48 ++++++++++++++++++++++++++++ src/lc-compliance/simple_capture.h | 12 +++++++ src/lc-compliance/single_stream.cpp | 31 +++++++++++++++++- 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index bb173ef1f71a..598c5aebe033 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -249,3 +249,51 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request) if (camera_->queueRequest(request)) loop_->exit(-EINVAL); } + +/* SimpleCaptureOverflow */ + +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr camera) + : SimpleCapture(camera) +{ +} + +Results::Result SimpleCaptureOverflow::capture() +{ + Results::Result ret = start(); + if (ret.first != Results::Pass) + return ret; + + Stream *stream = config_->at(0).stream(); + const std::vector> &buffers = allocator_->buffers(stream); + + captureCount_ = 0; + captureLimit_ = buffers.size(); + + std::vector> requests; + ret = queueRequests(stream, requests, buffers); + if (ret.first != Results::Pass) + return ret; + + runCaptureSession(); + + if (captureCount_ != captureLimit_) + return { Results::Fail, "Got " + std::to_string(captureCount_) + + " request, wanted " + std::to_string(captureLimit_) }; + + return { Results::Pass, "Overflow capture of " + + std::to_string(captureLimit_) + " requests" }; +} + +void SimpleCaptureOverflow::requestComplete([[maybe_unused]] Request *request) +{ + captureCompleted(); +} + +Results::Result SimpleCaptureOverflow::allocateBuffers(unsigned int count) +{ + Stream *stream = config_->at(0).stream(); + if (allocator_->allocate(stream, count) < 0) + return { Results::Fail, "Failed to allocate buffers" }; + + return { Results::Pass, "Allocated buffers" }; +} diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index 8d0f94f0c3e5..3b818c640477 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -72,4 +72,16 @@ private: void requestComplete(libcamera::Request *request) override; }; +class SimpleCaptureOverflow : public SimpleCapture +{ +public: + SimpleCaptureOverflow(std::shared_ptr camera); + + Results::Result allocateBuffers(unsigned int count); + Results::Result capture(); + +private: + void requestComplete(libcamera::Request *request) override; +}; + #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */ diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp index 649291c7bb73..7035d9cf5df5 100644 --- a/src/lc-compliance/single_stream.cpp +++ b/src/lc-compliance/single_stream.cpp @@ -12,6 +12,23 @@ using namespace libcamera; +static const unsigned int MAX_SIMULTANEOUS_REQUESTS = 8; + +Results::Result testRequestOverflow(std::shared_ptr camera, + StreamRole role, unsigned int numRequests) +{ + SimpleCaptureOverflow capture(camera); + + Results::Result ret = capture.configure(role); + if (ret.first != Results::Pass) + return ret; + capture.allocateBuffers(numRequests); + if (ret.first != Results::Pass) + return ret; + + return capture.capture(); +} + Results::Result testRequestBalance(std::shared_ptr camera, StreamRole role, unsigned int startCycles, unsigned int numRequests) @@ -61,7 +78,7 @@ Results testSingleStream(std::shared_ptr camera) }; static const std::vector numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; - Results results(numRequests.size() * roles.size() * 3); + Results results(numRequests.size() * roles.size() * 3 + roles.size()); for (const auto &role : roles) { std::cout << "= Test role " << role.first << std::endl; @@ -97,6 +114,18 @@ Results testSingleStream(std::shared_ptr camera) std::cout << "* Test unbalanced stop" << std::endl; for (unsigned int num : numRequests) results.add(testRequestUnbalance(camera, role.second, num)); + + /* + * Test overflowing pipeline with requests + * + * Makes sure that the camera supports receiving a large number + * of requests at once. Example failure is a camera that doesn't + * check if there are available resources (free internal + * buffers, free buffers in the video devices) before handling a + * request. + */ + std::cout << "* Test overflowing requests" << std::endl; + results.add(testRequestOverflow(camera, role.second, MAX_SIMULTANEOUS_REQUESTS)); } return results; From patchwork Thu May 6 18:02:48 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: 12214 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 174EBBDE7F for ; Thu, 6 May 2021 18:04:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DCD526891B; Thu, 6 May 2021 20:04:04 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 572DF68901 for ; Thu, 6 May 2021 20:04:03 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:995d:672b:100f:2fd9]) (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 874041F43B91; Thu, 6 May 2021 19:04:01 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 6 May 2021 15:02:48 -0300 Message-Id: <20210506180249.318346-6-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210506180249.318346-1-nfraprado@collabora.com> References: <20210506180249.318346-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 5/6] libcamera: pipeline: Don't rely on bufferCount 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" Pipelines have relied on bufferCount to decide on the number of buffers to allocate internally through allocateBuffers() and on the number of V4L2 buffer slots to reserve through importBuffers(). Instead, the number of internal buffers should be the minimum required by the algorithms to avoid wasting memory, and the number of V4L2 buffer slots should overallocate to avoid trashing dmabuf mappings. For now, just set them to constants and stop relying on bufferCount, to allow for its removal. Signed-off-by: Nícolas F. R. A. Prado --- src/libcamera/pipeline/ipu3/imgu.cpp | 12 ++++++------ src/libcamera/pipeline/ipu3/imgu.h | 5 ++++- src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +-------- .../pipeline/raspberrypi/raspberrypi.cpp | 15 +++++---------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++------- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 3 +++ src/libcamera/pipeline/simple/converter.cpp | 4 ++-- src/libcamera/pipeline/simple/converter.h | 3 +++ src/libcamera/pipeline/simple/simple.cpp | 6 ++---- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++-- src/libcamera/pipeline/vimc/vimc.cpp | 5 +++-- 12 files changed, 35 insertions(+), 43 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index d5cf05b0c421..720b598d1077 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -543,22 +543,22 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, /** * \brief Allocate buffers for all the ImgU video devices */ -int ImgUDevice::allocateBuffers(unsigned int bufferCount) +int ImgUDevice::allocateBuffers() { /* Share buffers between CIO2 output and ImgU input. */ - int ret = input_->importBuffers(bufferCount); + int ret = input_->importBuffers(BUFFER_SLOT_COUNT); if (ret) { LOG(IPU3, Error) << "Failed to import ImgU input buffers"; return ret; } - ret = param_->allocateBuffers(bufferCount, ¶mBuffers_); + ret = param_->allocateBuffers(INTERNAL_BUFFER_COUNT, ¶mBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU param buffers"; goto error; } - ret = stat_->allocateBuffers(bufferCount, &statBuffers_); + ret = stat_->allocateBuffers(INTERNAL_BUFFER_COUNT, &statBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; @@ -569,13 +569,13 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount) * corresponding stream is active or inactive, as the driver needs * buffers to be requested on the V4L2 devices in order to operate. */ - ret = output_->importBuffers(bufferCount); + ret = output_->importBuffers(BUFFER_SLOT_COUNT); if (ret < 0) { LOG(IPU3, Error) << "Failed to import ImgU output buffers"; goto error; } - ret = viewfinder_->importBuffers(bufferCount); + ret = viewfinder_->importBuffers(BUFFER_SLOT_COUNT); if (ret < 0) { LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers"; goto error; diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index 9d4915116087..4163a8dc0875 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -61,7 +61,7 @@ public: outputFormat); } - int allocateBuffers(unsigned int bufferCount); + int allocateBuffers(); void freeBuffers(); int start(); @@ -86,6 +86,9 @@ private: static constexpr unsigned int PAD_VF = 3; static constexpr unsigned int PAD_STAT = 4; + static constexpr unsigned int INTERNAL_BUFFER_COUNT = 4; + static constexpr unsigned int BUFFER_SLOT_COUNT = 5; + int linkSetup(const std::string &source, unsigned int sourcePad, const std::string &sink, unsigned int sinkPad, bool enable); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index d9e0d8a32afb..857cafcbbb72 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -668,16 +668,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); ImgUDevice *imgu = data->imgu_; - unsigned int bufferCount; int ret; - bufferCount = std::max({ - data->outStream_.configuration().bufferCount, - data->vfStream_.configuration().bufferCount, - data->rawStream_.configuration().bufferCount, - }); - - ret = imgu->allocateBuffers(bufferCount); + ret = imgu->allocateBuffers(); if (ret < 0) return ret; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 56eaf77c7566..ca31e437a431 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1144,20 +1144,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); int ret; + constexpr unsigned int bufferCount = 4; /* - * Decide how many internal buffers to allocate. For now, simply look - * at how many external buffers will be provided. We'll need to improve - * this logic. However, we really must have all streams allocate the same - * number of buffers to simplify error handling in queueRequestDevice(). + * Allocate internal buffers. We really must have all streams allocate + * the same number of buffers to simplify error handling in + * queueRequestDevice(). */ - unsigned int maxBuffers = 0; - for (const Stream *s : camera->streams()) - if (static_cast(s)->isExternal()) - maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); - for (auto const stream : data->streams_) { - ret = stream->prepareBuffers(maxBuffers); + ret = stream->prepareBuffers(bufferCount); if (ret < 0) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index d8cc7c2c9fb5..f09e247a9ae1 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -689,16 +689,11 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) unsigned int ipaBufferId = 1; int ret; - unsigned int maxCount = std::max({ - data->mainPathStream_.configuration().bufferCount, - data->selfPathStream_.configuration().bufferCount, - }); - - ret = param_->allocateBuffers(maxCount, ¶mBuffers_); + ret = param_->allocateBuffers(INTERNAL_BUFFER_COUNT, ¶mBuffers_); if (ret < 0) goto error; - ret = stat_->allocateBuffers(maxCount, &statBuffers_); + ret = stat_->allocateBuffers(INTERNAL_BUFFER_COUNT, &statBuffers_); if (ret < 0) goto error; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 25f482eb8d8e..ec281fef316b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -172,7 +172,7 @@ int RkISP1Path::start() return -EBUSY; /* \todo Make buffer count user configurable. */ - ret = video_->importBuffers(RKISP1_BUFFER_COUNT); + ret = video_->importBuffers(BUFFER_SLOT_COUNT); if (ret) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 3b3e37d258d0..2133914228aa 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -26,6 +26,9 @@ class V4L2Subdevice; struct StreamConfiguration; struct V4L2SubdeviceFormat; +static constexpr unsigned int INTERNAL_BUFFER_COUNT = 4; +static constexpr unsigned int BUFFER_SLOT_COUNT = 5; + class RkISP1Path { public: diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index 68644ef6477f..bf491cf661eb 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -100,11 +100,11 @@ int SimpleConverter::Stream::exportBuffers(unsigned int count, int SimpleConverter::Stream::start() { - int ret = m2m_->output()->importBuffers(inputBufferCount_); + int ret = m2m_->output()->importBuffers(BUFFER_SLOT_COUNT); if (ret < 0) return ret; - ret = m2m_->capture()->importBuffers(outputBufferCount_); + ret = m2m_->capture()->importBuffers(BUFFER_SLOT_COUNT); if (ret < 0) { stop(); return ret; diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h index 480e528d2210..6a7031b78ebf 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/src/libcamera/pipeline/simple/converter.h @@ -29,6 +29,9 @@ class SizeRange; struct StreamConfiguration; class V4L2M2MDevice; +constexpr unsigned int INTERNAL_BUFFER_COUNT = 5; +constexpr unsigned int BUFFER_SLOT_COUNT = 5; + class SimpleConverter { public: diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index c949efe5c422..a6e432272364 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -792,12 +792,10 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL * When using the converter allocate a fixed number of internal * buffers. */ - ret = video->allocateBuffers(kNumInternalBuffers, + ret = video->allocateBuffers(INTERNAL_BUFFER_COUNT, &data->converterBuffers_); } else { - /* Otherwise, prepare for using buffers from the only stream. */ - Stream *stream = &data->streams_[0]; - ret = video->importBuffers(stream->configuration().bufferCount); + ret = video->importBuffers(BUFFER_SLOT_COUNT); } if (ret < 0) return ret; diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 1f77629bb8fc..1052b6f2fe5f 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -91,6 +91,8 @@ private: return static_cast( PipelineHandler::cameraData(camera)); } + + static constexpr unsigned int BUFFER_SLOT_COUNT = 5; }; UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data) @@ -236,9 +238,8 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) { UVCCameraData *data = cameraData(camera); - unsigned int count = data->stream_.configuration().bufferCount; - int ret = data->video_->importBuffers(count); + int ret = data->video_->importBuffers(BUFFER_SLOT_COUNT); if (ret < 0) return ret; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 82e520dd7162..161d8ba6368d 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -102,6 +102,8 @@ private: return static_cast( PipelineHandler::cameraData(camera)); } + + static constexpr unsigned int BUFFER_SLOT_COUNT = 5; }; namespace { @@ -312,9 +314,8 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] const ControlList *controls) { VimcCameraData *data = cameraData(camera); - unsigned int count = data->stream_.configuration().bufferCount; - int ret = data->video_->importBuffers(count); + int ret = data->video_->importBuffers(BUFFER_SLOT_COUNT); if (ret < 0) return ret; From patchwork Thu May 6 18:02:49 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: 12215 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 4F045BDE7F for ; Thu, 6 May 2021 18:04:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 16EC26891A; Thu, 6 May 2021 20:04:07 +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 93BE968901 for ; Thu, 6 May 2021 20:04:05 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:995d:672b:100f:2fd9]) (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 9F6571F43B93; Thu, 6 May 2021 19:04:03 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 6 May 2021 15:02:49 -0300 Message-Id: <20210506180249.318346-7-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210506180249.318346-1-nfraprado@collabora.com> References: <20210506180249.318346-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 6/6] libcamera: stream: Remove bufferCount 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" Now that the number of buffers allocated by the FrameBufferAllocator helper is passed through FrameBufferAllocator::allocate() and the pipelines no longer use bufferCount for internal buffer or V4L2 buffer slots allocation, we no longer need to have bufferCount in the StreamConfiguration, so remove it. Signed-off-by: Nícolas F. R. A. Prado --- include/libcamera/stream.h | 2 -- src/android/camera_stream.cpp | 2 +- src/libcamera/pipeline/ipu3/cio2.cpp | 1 - src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ------- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 ------ src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 -- src/libcamera/pipeline/simple/converter.cpp | 3 --- src/libcamera/pipeline/simple/converter.h | 3 --- src/libcamera/pipeline/simple/simple.cpp | 5 +---- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 --- src/libcamera/pipeline/vimc/vimc.cpp | 3 --- src/libcamera/stream.cpp | 9 ++------- src/v4l2/v4l2_camera.cpp | 14 ++++++++++---- src/v4l2/v4l2_camera.h | 5 +++-- src/v4l2/v4l2_camera_proxy.cpp | 8 +++----- test/camera/buffer_import.cpp | 10 +++++++--- test/libtest/buffer_source.cpp | 4 ++-- test/libtest/buffer_source.h | 2 +- test/v4l2_videodevice/buffer_cache.cpp | 4 ++-- 19 files changed, 32 insertions(+), 61 deletions(-) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index bb47c390f8a1..f36aeffd9540 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -45,8 +45,6 @@ struct StreamConfiguration { unsigned int stride; unsigned int frameSize; - unsigned int bufferCount; - Stream *stream() const { return stream_; } void setStream(Stream *stream) { stream_ = stream; } const StreamFormats &formats() const { return formats_; } diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 859c8807e585..3498ecb0bc0e 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -94,7 +94,7 @@ int CameraStream::configure() buffers_.push_back(frameBuffer.get()); } - camera3Stream_->max_buffers = configuration().bufferCount; + camera3Stream_->max_buffers = bufferCount; return 0; } diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 3cd777d1b742..1e110fe0c189 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -213,7 +213,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const cfg.size = sensorFormat.size; cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code); - cfg.bufferCount = CIO2_BUFFER_COUNT; return cfg; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 857cafcbbb72..b1bc421fc7aa 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -291,7 +291,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() /* Initialize the RAW stream with the CIO2 configuration. */ cfg->size = cio2Configuration_.size; cfg->pixelFormat = cio2Configuration_.pixelFormat; - cfg->bufferCount = cio2Configuration_.bufferCount; cfg->stride = info.stride(cfg->size.width, 0, 64); cfg->frameSize = info.frameSize(cfg->size, 64); cfg->setStream(const_cast(&data_->rawStream_)); @@ -335,7 +334,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() IMGU_OUTPUT_HEIGHT_ALIGN); cfg->pixelFormat = formats::NV12; - cfg->bufferCount = IPU3_BUFFER_COUNT; cfg->stride = info.stride(cfg->size.width, 0, 1); cfg->frameSize = info.frameSize(cfg->size, 1); @@ -403,7 +401,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, Size sensorResolution = data->cio2_.sensor()->resolution(); for (const StreamRole role : roles) { std::map> streamFormats; - unsigned int bufferCount; PixelFormat pixelFormat; Size size; @@ -424,7 +421,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, size.height = utils::alignDown(size.height - 1, IMGU_OUTPUT_HEIGHT_MARGIN); pixelFormat = formats::NV12; - bufferCount = IPU3_BUFFER_COUNT; streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } }; break; @@ -434,7 +430,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, data->cio2_.generateConfiguration(sensorResolution); pixelFormat = cio2Config.pixelFormat; size = cio2Config.size; - bufferCount = cio2Config.bufferCount; for (const PixelFormat &format : data->cio2_.formats()) streamFormats[format] = data->cio2_.sizes(); @@ -453,7 +448,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, .alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN, IMGU_OUTPUT_HEIGHT_ALIGN); pixelFormat = formats::NV12; - bufferCount = IPU3_BUFFER_COUNT; streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } }; break; @@ -470,7 +464,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, StreamConfiguration cfg(formats); cfg.size = size; cfg.pixelFormat = pixelFormat; - cfg.bufferCount = bufferCount; config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ca31e437a431..bf5fa8c158a3 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -471,7 +471,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, RPiCameraData *data = cameraData(camera); CameraConfiguration *config = new RPiCameraConfiguration(data); V4L2DeviceFormat sensorFormat; - unsigned int bufferCount; PixelFormat pixelFormat; V4L2VideoDevice::Formats fmts; Size size; @@ -489,7 +488,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, sensorFormat = findBestMode(fmts, size); pixelFormat = sensorFormat.fourcc.toPixelFormat(); ASSERT(pixelFormat.isValid()); - bufferCount = 2; rawCount++; break; @@ -498,7 +496,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, pixelFormat = formats::NV12; /* Return the largest sensor resolution. */ size = data->sensor_->resolution(); - bufferCount = 1; outCount++; break; @@ -514,7 +511,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, fmts = data->isp_[Isp::Output0].dev()->formats(); pixelFormat = formats::YUV420; size = { 1920, 1080 }; - bufferCount = 4; outCount++; break; @@ -522,7 +518,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, fmts = data->isp_[Isp::Output0].dev()->formats(); pixelFormat = formats::ARGB8888; size = { 800, 600 }; - bufferCount = 4; outCount++; break; @@ -552,7 +547,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, StreamConfiguration cfg(formats); cfg.size = size; cfg.pixelFormat = pixelFormat; - cfg.bufferCount = bufferCount; config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index ec281fef316b..42cdb0ff7be6 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -61,7 +61,6 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) StreamConfiguration cfg(formats); cfg.pixelFormat = formats::NV12; cfg.size = maxResolution; - cfg.bufferCount = RKISP1_BUFFER_COUNT; return cfg; } @@ -77,7 +76,6 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) cfg->size.boundTo(maxResolution_); cfg->size.expandTo(minResolution_); - cfg->bufferCount = RKISP1_BUFFER_COUNT; V4L2DeviceFormat format; format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index bf491cf661eb..09d5ecb14084 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -86,9 +86,6 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, return -EINVAL; } - inputBufferCount_ = inputCfg.bufferCount; - outputBufferCount_ = outputCfg.bufferCount; - return 0; } diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h index 6a7031b78ebf..1288b4717ff9 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/src/libcamera/pipeline/simple/converter.h @@ -87,9 +87,6 @@ private: SimpleConverter *converter_; unsigned int index_; std::unique_ptr m2m_; - - unsigned int inputBufferCount_; - unsigned int outputBufferCount_; }; std::string deviceNode_; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index a6e432272364..6be9587062e8 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -617,7 +617,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.size != pipeConfig_->captureSize) needConversion_ = true; - /* Set the stride, frameSize and bufferCount. */ + /* Set the stride and frameSize. */ if (needConversion_) { std::tie(cfg.stride, cfg.frameSize) = converter->strideAndFrameSize(cfg.pixelFormat, cfg.size); @@ -635,8 +635,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; } - - cfg.bufferCount = 3; } return status; @@ -759,7 +757,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) inputCfg.pixelFormat = pipeConfig->captureFormat; inputCfg.size = pipeConfig->captureSize; inputCfg.stride = captureFormat.planes[0].bpl; - inputCfg.bufferCount = kNumInternalBuffers; return converter_->configure(inputCfg, outputCfgs); } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 1052b6f2fe5f..eaeb94887c93 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -150,8 +150,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -193,7 +191,6 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, cfg.pixelFormat = formats.pixelformats().front(); cfg.size = formats.sizes(cfg.pixelFormat).back(); - cfg.bufferCount = 4; config->addConfiguration(cfg); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 161d8ba6368d..569f07c694b5 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -167,8 +167,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -224,7 +222,6 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, cfg.pixelFormat = formats::BGR888; cfg.size = { 1920, 1080 }; - cfg.bufferCount = 4; config->addConfiguration(cfg); diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index f7bafcf8fc97..be57abce4eb3 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -279,7 +279,7 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const * handlers provide StreamFormats. */ StreamConfiguration::StreamConfiguration() - : pixelFormat(0), stride(0), frameSize(0), bufferCount(0), + : pixelFormat(0), stride(0), frameSize(0), stream_(nullptr) { } @@ -288,7 +288,7 @@ StreamConfiguration::StreamConfiguration() * \brief Construct a configuration with stream formats */ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) - : pixelFormat(0), stride(0), frameSize(0), bufferCount(0), + : pixelFormat(0), stride(0), frameSize(0), stream_(nullptr), formats_(formats) { } @@ -323,11 +323,6 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) * validating the configuration with a call to CameraConfiguration::validate(). */ -/** - * \var StreamConfiguration::bufferCount - * \brief Requested number of buffers to allocate for the stream - */ - /** * \fn StreamConfiguration::stream() * \brief Retrieve the stream associated with the configuration diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 53d97f3e6b86..52f28960b3c2 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include "libcamera/internal/log.h" using namespace libcamera; @@ -107,14 +109,12 @@ void V4L2Camera::requestComplete(Request *request) } int V4L2Camera::configure(StreamConfiguration *streamConfigOut, - const Size &size, const PixelFormat &pixelformat, - unsigned int bufferCount) + const Size &size, const PixelFormat &pixelformat) { StreamConfiguration &streamConfig = config_->at(0); streamConfig.size.width = size.width; streamConfig.size.height = size.height; streamConfig.pixelFormat = pixelformat; - streamConfig.bufferCount = bufferCount; /* \todo memoryType (interval vs external) */ CameraConfiguration::Status validation = config_->validate(); @@ -146,7 +146,6 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat, StreamConfiguration &cfg = config->at(0); cfg.size = size; cfg.pixelFormat = pixelFormat; - cfg.bufferCount = 1; CameraConfiguration::Status validation = config->validate(); if (validation == CameraConfiguration::Invalid) @@ -299,3 +298,10 @@ bool V4L2Camera::isRunning() { return isRunning_; } + +unsigned int V4L2Camera::minNumRequests(unsigned int count) +{ + unsigned int min = camera_->properties().get(properties::MinNumRequests); + + return std::max(count, min); +} diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index d238046250e3..a5ad7dd35351 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -45,8 +45,7 @@ public: std::vector completedBuffers(); int configure(StreamConfiguration *streamConfigOut, - const Size &size, const PixelFormat &pixelformat, - unsigned int bufferCount); + const Size &size, const PixelFormat &pixelformat); int validateConfiguration(const PixelFormat &pixelformat, const Size &size, StreamConfiguration *streamConfigOut); @@ -65,6 +64,8 @@ public: bool isRunning(); + unsigned int minNumRequests(unsigned int count); + private: void requestComplete(Request *request); diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index f8bfe595e90e..48955f85b991 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -348,8 +348,7 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg) Size size(arg->fmt.pix.width, arg->fmt.pix.height); V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat); ret = vcam_->configure(&streamConfig_, size, - PixelFormatInfo::info(v4l2Format).format, - bufferCount_); + PixelFormatInfo::info(v4l2Format).format); if (ret < 0) return -EINVAL; @@ -490,14 +489,13 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf Size size(v4l2PixFormat_.width, v4l2PixFormat_.height); V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat); int ret = vcam_->configure(&streamConfig_, size, - PixelFormatInfo::info(v4l2Format).format, - arg->count); + PixelFormatInfo::info(v4l2Format).format); if (ret < 0) return -EINVAL; setFmtFromConfig(streamConfig_); - arg->count = streamConfig_.bufferCount; + arg->count = vcam_->minNumRequests(arg->count); bufferCount_ = arg->count; ret = vcam_->allocBuffers(arg->count); diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 61f4eb92ae95..403829efcfd3 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -12,6 +12,8 @@ #include #include +#include + #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/event_dispatcher.h" #include "libcamera/internal/media_device.h" @@ -91,10 +93,12 @@ protected: return TestFail; } + unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests); + Stream *stream = cfg.stream(); BufferSource source; - int ret = source.allocate(cfg); + int ret = source.allocate(cfg, bufferCount); if (ret != TestPass) return ret; @@ -138,10 +142,10 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ < cfg.bufferCount * 2) { + if (completeRequestsCount_ < bufferCount * 2) { std::cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg.bufferCount * 2 << ")" << std::endl; + << bufferCount * 2 << ")" << std::endl; return TestFail; } diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index 73563f2fc39d..c3d5286a2462 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -24,7 +24,7 @@ BufferSource::~BufferSource() media_->release(); } -int BufferSource::allocate(const StreamConfiguration &config) +int BufferSource::allocate(const StreamConfiguration &config, unsigned int count) { /* Locate and open the video device. */ std::string videoDeviceName = "vivid-000-vid-out"; @@ -77,7 +77,7 @@ int BufferSource::allocate(const StreamConfiguration &config) return TestFail; } - if (video->allocateBuffers(config.bufferCount, &buffers_) < 0) { + if (video->allocateBuffers(count, &buffers_) < 0) { std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h index 14b4770e8d8a..6a18e269a575 100644 --- a/test/libtest/buffer_source.h +++ b/test/libtest/buffer_source.h @@ -20,7 +20,7 @@ public: BufferSource(); ~BufferSource(); - int allocate(const StreamConfiguration &config); + int allocate(const StreamConfiguration &config, unsigned int count); const std::vector> &buffers(); private: diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp index b3f2bec11783..07fddfd2617c 100644 --- a/test/v4l2_videodevice/buffer_cache.cpp +++ b/test/v4l2_videodevice/buffer_cache.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include "buffer_source.h" @@ -145,10 +146,9 @@ public: StreamConfiguration cfg; cfg.pixelFormat = formats::YUYV; cfg.size = Size(600, 800); - cfg.bufferCount = numBuffers; BufferSource source; - int ret = source.allocate(cfg); + int ret = source.allocate(cfg, numBuffers); if (ret != TestPass) return ret;