From patchwork Tue Nov 26 23:35:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2349 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5854260C3D for ; Wed, 27 Nov 2019 00:39:26 +0100 (CET) X-Halon-ID: fa49fc55-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fa49fc55-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:24 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:51 +0100 Message-Id: <20191126233620.1695316-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 01/30] libcamera: pipelines: Align bookkeeping in queueRequest() 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:26 -0000 Expecting pipeline handler implementations of queueRequest() to call the base class queueRequest() at the correct point have lead to different behaviors between the pipelines. Fix this by splitting queueRequest() into a base class implementation which handles the bookkeeping and a new queueRequestHardware() that is to be implemented by pipeline handlers and only deals with committing the request to hardware. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/include/pipeline_handler.h | 4 ++- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- src/libcamera/pipeline/uvcvideo.cpp | 6 ++-- src/libcamera/pipeline/vimc.cpp | 6 ++-- src/libcamera/pipeline_handler.cpp | 35 +++++++++++++++++------- 6 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index a02e6e77dc9ce0e7..6ce8ea1c8d31b870 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -77,7 +77,7 @@ public: virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; - virtual int queueRequest(Camera *camera, Request *request); + int queueRequest(Camera *camera, Request *request); bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); void completeRequest(Camera *camera, Request *request); @@ -89,6 +89,8 @@ protected: std::unique_ptr data); void hotplugMediaDevice(MediaDevice *media); + virtual int queueRequestHardware(Camera *camera, Request *request) = 0; + CameraData *cameraData(const Camera *camera); CameraManager *manager_; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1c5fccf694289ae9..ad223d9101bdc6ed 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -220,7 +220,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera) data->rawBuffers_.clear(); } -int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) +int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) { int error = 0; @@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) error = ret; } - PipelineHandler::queueRequest(camera, request); - return error; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4a583a7a1d7ef1fd..f75b25c6787e40df 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -184,7 +184,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera) activeCamera_ = nullptr; } -int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) +int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, + Request *request) { RkISP1CameraData *data = cameraData(camera); Stream *stream = &data->stream_; - PipelineHandler::queueRequest(camera, request); - RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, stream); if (!info) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 522229241ffb9e8a..3a76653ff6dc2b5e 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -72,7 +72,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) return ret; } -int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) +int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); Buffer *buffer = request->findBuffer(&data->stream_); @@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) if (ret < 0) return ret; - PipelineHandler::queueRequest(camera, request); - return 0; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 06664fed42e796e4..f5550a1723668106 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -90,7 +90,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) return ret; } -int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) +int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); Buffer *buffer = request->findBuffer(&data->stream_); @@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) if (ret < 0) return ret; - PipelineHandler::queueRequest(camera, request); - return 0; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4349ca8957e403fe..c9e348b98da7b736 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -345,27 +345,42 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * \param[in] request The request to queue * * This method queues a capture request to the pipeline handler for processing. - * The request contains a set of buffers associated with streams and a set of - * parameters. The pipeline handler shall program the device to ensure that the - * parameters will be applied to the frames captured in the buffers provided in - * the request. - * - * Pipeline handlers shall override this method. The base implementation in the - * PipelineHandler class keeps track of queued requests in order to ensure - * completion of all requests when the pipeline handler is stopped with stop(). - * Requests completion shall be signalled by the pipeline handler using the + * The method keeps track of queued requests in order to ensure completion of + * all requests when the pipeline handler is stopped with stop(). Requests + * completion shall be signalled by the pipeline handler using the * completeRequest() method. * * \return 0 on success or a negative error code otherwise */ int PipelineHandler::queueRequest(Camera *camera, Request *request) { + int ret; + CameraData *data = cameraData(camera); data->queuedRequests_.push_back(request); - return 0; + ret = queueRequestHardware(camera, request); + if (ret) + data->queuedRequests_.remove(request); + + return ret; } +/** + * \fn PipelineHandler::queueRequestHardware() + * \brief Queue a request to the hardware + * \param[in] camera The camera to queue the request to + * \param[in] request The request to queue + * + * This method queues a capture request to the hardware for processing. The + * request contains a set of buffers associated with streams and a set of + * parameters. The pipeline handler shall program the device to ensure that the + * parameters will be applied to the frames captured in the buffers provided in + * the request. + * + * \return 0 on success or a negative error code otherwise + */ + /** * \brief Complete a buffer for a request * \param[in] camera The camera the request belongs to From patchwork Tue Nov 26 23:35:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2350 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 11EB960C21 for ; Wed, 27 Nov 2019 00:39:27 +0100 (CET) X-Halon-ID: fad60a2c-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fad60a2c-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:25 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:52 +0100 Message-Id: <20191126233620.1695316-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 02/30] libcamera: Remove buffer index from logging 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:27 -0000 The buffer index is a V4L2 concept that will be hidden from users when the switch to FrameBuffer is complete, in preparation remove it from logging. Keep and move one debug log message where the index is available as the V4L2 buffer is being dequeued for the video device and it's useful when debugging. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/cam/capture.cpp | 3 +-- src/libcamera/v4l2_videodevice.cpp | 3 +-- src/qcam/main_window.cpp | 3 +-- test/v4l2_videodevice/buffer_sharing.cpp | 8 ++++---- test/v4l2_videodevice/capture_async.cpp | 2 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 4 ++-- 6 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index e665d819fb777a90..4b65b1d0a2dbed35 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -155,7 +155,6 @@ void Capture::requestComplete(Request *request) const std::string &name = streamName_[stream]; info << " " << name - << " (" << buffer->index() << ")" << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() << " bytesused: " << buffer->bytesused(); @@ -182,7 +181,7 @@ void Capture::requestComplete(Request *request) std::unique_ptr newBuffer = stream->createBuffer(index); if (!newBuffer) { - std::cerr << "Can't create buffer " << index << std::endl; + std::cerr << "Can't create buffer" << std::endl; return; } diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 992130751286994c..dbb5c3982334e243 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1103,6 +1103,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer() } ASSERT(buf.index < bufferPool_->count()); + LOG(V4L2, Debug) << "Buffer " << buf.index << " is available"; auto it = queuedBuffers_.find(buf.index); Buffer *buffer = it->second; @@ -1138,8 +1139,6 @@ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) if (!buffer) return; - LOG(V4L2, Debug) << "Buffer " << buffer->index() << " is available"; - /* Notify anyone listening to the device. */ bufferReady.emit(buffer); } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index cca7365ae75687f9..0c7ca61ac12ec41c 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -264,7 +264,6 @@ void MainWindow::requestComplete(Request *request) lastBufferTime_ = buffer->timestamp(); std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " buf: " << buffer->index() << " bytesused: " << buffer->bytesused() << " timestamp: " << buffer->timestamp() << " fps: " << std::fixed << std::setprecision(2) << fps @@ -285,7 +284,7 @@ void MainWindow::requestComplete(Request *request) std::unique_ptr newBuffer = stream->createBuffer(index); if (!newBuffer) { - std::cerr << "Can't create buffer " << index << std::endl; + std::cerr << "Can't create buffer" << std::endl; return; } diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 1629f34cfa6c79fe..d02c391b95933922 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -92,8 +92,8 @@ protected: void captureBufferReady(Buffer *buffer) { - std::cout << "Received capture buffer: " << buffer->index() - << " sequence " << buffer->sequence() << std::endl; + std::cout << "Received capture buffer sequence " + << buffer->sequence() << std::endl; if (buffer->status() != Buffer::BufferSuccess) return; @@ -104,8 +104,8 @@ protected: void outputBufferReady(Buffer *buffer) { - std::cout << "Received output buffer: " << buffer->index() - << " sequence " << buffer->sequence() << std::endl; + std::cout << "Received output buffer sequence " + << buffer->sequence() << std::endl; if (buffer->status() != Buffer::BufferSuccess) return; diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index 442a4fe56eace57e..0bc0067c50909c9d 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -22,7 +22,7 @@ public: void receiveBuffer(Buffer *buffer) { - std::cout << "Received buffer " << buffer->index() << std::endl; + std::cout << "Received buffer" << std::endl; frames++; /* Requeue the buffer for further use. */ diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index 4d3644c2d28792f1..442bcac5dc49cc59 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -31,7 +31,7 @@ public: void outputBufferComplete(Buffer *buffer) { - cout << "Received output buffer " << buffer->index() << endl; + cout << "Received output buffer" << endl; outputFrames_++; @@ -41,7 +41,7 @@ public: void receiveCaptureBuffer(Buffer *buffer) { - cout << "Received capture buffer " << buffer->index() << endl; + cout << "Received capture buffer" << endl; captureFrames_++; From patchwork Tue Nov 26 23:35:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2351 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C315861C62 for ; Wed, 27 Nov 2019 00:39:27 +0100 (CET) X-Halon-ID: fb3c3f54-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fb3c3f54-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:26 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:53 +0100 Message-Id: <20191126233620.1695316-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 03/30] libcamera: buffer: Add BufferInfo container for buffer metadata information 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:28 -0000 For FrameBuffers the metadata information gathered when a buffer is captured will not be stored directly in the buffer object but in its own container. Add the BufferInfo class to hold this information. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- include/libcamera/buffer.h | 30 +++++++++++++ src/libcamera/buffer.cpp | 92 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 80602124f24be4a0..7302b9f32ce8c50d 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -16,6 +16,36 @@ namespace libcamera { class Request; class Stream; +class BufferInfo +{ +public: + enum Status { + BufferSuccess, + BufferError, + BufferCancelled, + }; + + struct Plane { + unsigned int bytesused; + }; + + BufferInfo(); + + void update(Status status, unsigned int sequence, uint64_t timestamp, + const std::vector &planes); + + Status status() const { return status_; } + unsigned int sequence() const { return sequence_; } + uint64_t timestamp() const { return timestamp_; } + const std::vector &planes() const { return planes_; } + +private: + Status status_; + unsigned int sequence_; + uint64_t timestamp_; + std::vector planes_; +}; + class Plane final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 960eeb8f7d193ddd..4acff216cafba484 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -23,6 +23,98 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Buffer) +/** + * \class BufferInfo + * \brief Dynamic buffer metadata + * + * The BufferInfo class references a buffers dynamic metadata related to the + * frame contained in the buffer. + */ + +/** + * \enum BufferInfo::Status + * Buffer completion status + * \var BufferInfo::BufferSuccess + * The buffer has completed with success and contains valid data. All its other + * metadata (such as bytesused(), timestamp() or sequence() number) are valid. + * \var BufferInfo::BufferError + * The buffer has completed with an error and doesn't contain valid data. Its + * other metadata are valid. + * \var BufferInfo::BufferCancelled + * The buffer has been cancelled due to capture stop. Its other metadata are + * invalid and shall not be used. + */ + +/** + * \struct BufferInfo::Plane + * \brief Plane specific buffer information + */ + +/** + * \var BufferInfo::Plane::bytesused + * \brief Retrieve the number of bytes occupied by the data in the plane + * \return Number of bytes occupied in the plane + */ + +BufferInfo::BufferInfo() + : status_(BufferError), sequence_(0), timestamp_(0) +{ +} + +/** + * \brief Uptade the buffer information + * \param[in] status New buffer status + * \param[in] sequence New buffer sequence + * \param[in] timestamp New buffer timestamp + * \param[in] planes New buffer plane meta data + * + * Update the stored buffer meta data information. + */ +void BufferInfo::update(Status status, unsigned int sequence, + uint64_t timestamp, const std::vector &planes) +{ + status_ = status; + sequence_ = sequence; + timestamp_ = timestamp; + planes_ = planes; +} + +/** + * \fn BufferInfo::status() + * \brief Retrieve the buffer status + * + * The buffer status reports whether the buffer has completed successfully + * (BufferSuccess) or if an error occurred (BufferError). + * + * \return The buffer status + */ + +/** + * \fn BufferInfo::sequence() + * \brief Retrieve the buffer sequence number + * + * The sequence number is a monotonically increasing number assigned to the + * buffer processed by the stream. Gaps in the sequence numbers indicate + * dropped frames. + * + * \return Sequence number of the buffer + */ + +/** + * \fn BufferInfo::timestamp() + * \brief Retrieve the time when the buffer was processed + * + * The timestamp is expressed as a number of nanoseconds since the epoch. + * + * \return Timestamp when the buffer was processed + */ + +/** + * \fn BufferInfo::planes() + * \brief Retrieve the plane(s) information for the buffer + * \return A array holding all plane information for the buffer + */ + /** * \class Plane * \brief A memory region to store a single plane of a frame From patchwork Tue Nov 26 23:35:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2352 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 27AC860C3D for ; Wed, 27 Nov 2019 00:39:28 +0100 (CET) X-Halon-ID: fb9c075c-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fb9c075c-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:26 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:54 +0100 Message-Id: <20191126233620.1695316-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 04/30] libcamera: buffer: Add FileDecriptor to help deal with file descriptors 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:29 -0000 Add a helper to make it easier to pass file descriptors around. The helper class duplicates the fd which decouples it from the original fd which could be closed by its owner while the new FileDescriptor remains valid. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- include/libcamera/buffer.h | 12 ++++++++++++ src/libcamera/buffer.cpp | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 7302b9f32ce8c50d..33793b4ccf881eda 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -46,6 +46,18 @@ private: std::vector planes_; }; +class FileDescriptor final +{ +public: + FileDescriptor(int fd); + ~FileDescriptor(); + + int fd() const { return fd_; } + +private: + int fd_; +}; + class Plane final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 4acff216cafba484..0676586ae3be2a61 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -115,6 +115,46 @@ void BufferInfo::update(Status status, unsigned int sequence, * \return A array holding all plane information for the buffer */ +/** + * \class FileDescriptor + * \brief Life time management of a file descriptor + * + * The managed file descriptor (fd) is duplicated from the original one and + * is opened for the life time of the FileDescriptor object disregarding + * if the original one is closed. + */ + +/** + * \brief Create a life time managed file descriptor + * \param[in] fd File descriptor + */ +FileDescriptor::FileDescriptor(int fd) + : fd_(-1) +{ + if (fd < 0) + return; + + /* Failing to dup() a fd should not happen and is fatal. */ + fd_ = dup(fd); + if (fd_ == -1) { + int ret = -errno; + LOG(Buffer, Fatal) + << "Failed to dup() fd: " << strerror(-ret); + } +} + +FileDescriptor::~FileDescriptor() +{ + if (fd_ != -1) + close(fd_); +} + +/** + * \fn FileDescriptor::fd() + * \brief Retrieve the file descriptor or + * \return A valid file descriptor or a negative error code + */ + /** * \class Plane * \brief A memory region to store a single plane of a frame From patchwork Tue Nov 26 23:35:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2353 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D538761C59 for ; Wed, 27 Nov 2019 00:39:28 +0100 (CET) X-Halon-ID: fbea2f08-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fbea2f08-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:27 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:55 +0100 Message-Id: <20191126233620.1695316-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 05/30] libcamera: buffer: Add Dmabuf to describe a dma buffer 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:29 -0000 A FrameBuffer object that holds a frame captured from a sensor consists of one or more plane(s). The memory of each plane can be accessed by using a dma buffer. Add a class that describes a dmabuf to make it easy for applications and IPAs to interact with memory. Signed-off-by: Niklas Söderlund --- include/libcamera/buffer.h | 19 +++++++ src/libcamera/buffer.cpp | 113 +++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 33793b4ccf881eda..3c430afbfe8e9a05 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -58,6 +58,25 @@ private: int fd_; }; +class Dmabuf final +{ +public: + Dmabuf(int fd, unsigned int length); + ~Dmabuf(); + + int fd() const { return fd_.fd(); } + unsigned int length() const { return length_; } + void *mem(); + +private: + int mmap(); + int munmap(); + + FileDescriptor fd_; + unsigned int length_; + void *mem_; +}; + class Plane final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 0676586ae3be2a61..5516055b2ea885c2 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -177,6 +177,119 @@ FileDescriptor::~FileDescriptor() * an image may or may not be contiguous. */ +/** + * \class Dmabuf + * \brief A memory region to store a single plane of a frame + * + * Planar pixel formats use multiple memory regions to store planes + * corresponding to the different colour components of a frame. The Dmabuf class + * tracks the specific details of a memory region used to store a single plane + * for a given frame and provides the means to access the memory, both for the + * application and for DMA. A Buffer then contains one or multiple planes + * depending on its pixel format. + * + * To support DMA access, planes are associated with dmabuf objects represented + * by file handles. Each plane carries a dmabuf file handle and an offset within + * the buffer. Those file handles may refer to the same dmabuf object, depending + * on whether the devices accessing the memory regions composing the image + * support non-contiguous DMA to planes ore require DMA-contiguous memory. + * + * To support CPU access, planes carry the CPU address of their backing memory. + * Similarly to the dmabuf file handles, the CPU addresses for planes composing + * an image may or may not be contiguous. + */ + +/** + * \brief Set the dmabuf file handle backing the buffer + * \param[in] fd The dmabuf file handle + * \param[in] length The size of the memory region + * + * The \a fd dmabuf file handle is duplicated and stored. + */ +Dmabuf::Dmabuf(int fd, unsigned int length) + : fd_(fd), length_(length), mem_(nullptr) +{ +} + +Dmabuf::~Dmabuf() +{ + munmap(); +} + +/** + * \fn Dmabuf::fd() + * \brief Get the dmabuf file handle backing the buffer + */ + +/** + * \fn Dmabuf::length() + * \brief Retrieve the length of the memory region + * \return The length of the memory region + */ + +/** + * \fn Dmabuf::mem() + * \brief Retrieve the CPU accessible memory address of the Dmabuf + * \return The CPU accessible memory address on success or nullptr otherwise. + */ +void *Dmabuf::mem() +{ + if (!mem_) + mmap(); + + return mem_; +} + +/** + * \brief Map the plane memory data to a CPU accessible address + * + * \return 0 on success or a negative error code otherwise + */ +int Dmabuf::mmap() +{ + void *map; + + if (mem_) + return 0; + + map = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd_.fd(), 0); + if (map == MAP_FAILED) { + int ret = -errno; + LOG(Buffer, Error) + << "Failed to mmap plane: " << strerror(-ret); + return ret; + } + + mem_ = map; + + return 0; +} + +/** + * \brief Unmap any existing CPU accessible mapping + * + * Unmap the memory mapped by an earlier call to mmap(). + * + * \return 0 on success or a negative error code otherwise + */ +int Dmabuf::munmap() +{ + int ret = 0; + + if (mem_) + ret = ::munmap(mem_, length_); + + if (ret) { + ret = -errno; + LOG(Buffer, Warning) + << "Failed to unmap plane: " << strerror(-ret); + } else { + mem_ = 0; + } + + return ret; +} + Plane::Plane() : fd_(-1), length_(0), mem_(0) { From patchwork Tue Nov 26 23:35:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2354 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E3EF61C59 for ; Wed, 27 Nov 2019 00:39:29 +0100 (CET) X-Halon-ID: fc4de60d-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fc4de60d-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:27 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:56 +0100 Message-Id: <20191126233620.1695316-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 06/30] libcamera: buffer: Add FrameBuffer interface 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:29 -0000 Add the FrameBuffer interface buffer implementation. This change just adds the new interface, future patches will migrate all parts of libcamera to use this and replace the old Buffer interface. This change needs to clarify the const for Plane::length() as to not get Doxygen confused with FrameBuffer::Plane::length added in this chance. Signed-off-by: Niklas Söderlund --- include/libcamera/buffer.h | 33 ++++++++++ src/libcamera/buffer.cpp | 130 ++++++++++++++++++++++++++++++++++++- 2 files changed, 162 insertions(+), 1 deletion(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 3c430afbfe8e9a05..fe5195327b540f5c 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -171,6 +171,39 @@ private: Stream *stream_; }; +class FrameBuffer final +{ +public: + struct Plane { + int fd; + unsigned int length; + }; + + FrameBuffer(std::vector planes, unsigned int cookie = 0); + ~FrameBuffer(); + + Request *request() const { return request_; } + const BufferInfo &info() const { return info_; }; + const std::vector &dmabufs() { return dmabufs_; } + + const std::vector &planes() const { return planes_; } + + unsigned int cookie() const { return cookie_; } + void setCookie(unsigned int cookie) { cookie_ = cookie; } + +private: + friend class Request; /* Needed to update request_. */ + friend class V4L2VideoDevice; /* Needed to update info_. */ + + Request *request_; + BufferInfo info_; + std::vector dmabufs_; + + std::vector planes_; + + unsigned int cookie_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_BUFFER_H__ */ diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 5516055b2ea885c2..07647124a2cd9c62 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -412,7 +412,7 @@ void *Plane::mem() } /** - * \fn Plane::length() + * \fn Plane::length() const * \brief Retrieve the length of the memory region * \return The length of the memory region */ @@ -645,4 +645,132 @@ void Buffer::cancel() * The intended callers are Request::prepare() and Request::completeBuffer(). */ +/** + * \class FrameBuffer + * \brief A buffer handle and dynamic metadata + * + * The FrameBuffer class references a buffer memory and associates dynamic + * metadata related to the frame contained in the buffer. It allows referencing + * buffer memory. + * + * A FrameBuffer object can be created from both internal and externally + * allocated dmabuf. + */ + +/** + * \struct FrameBuffer::Plane + * \brief Describe a DMA buffer using low level data + * + * The plane description contains a file descriptor and a length, together + * they represent a dmabuf. + */ + +/** + * \var FrameBuffer::Plane::fd + * \brief The dmabuf file handle + */ + +/** + * \var FrameBuffer::Plane::length + * \brief The dmabuf length + */ + +/** + * \brief Create a libcamera FrameBuffer object from an array of dmabufs + * \param[in] planes dmabufs described as planes + * \param[in] cookie Opaque cookie for application use + * + * Buffers used by libcamera might be allocated externally or internally to + * libcamera, the FrameBuffer object handles both cases. + * + * The \a cookie is stored in the buffer and is accessible through the + * cookie() method at any time. It is typically used by user to map the + * buffer to an external resource, and is completely opaque to the FrameBuffer + * object. + */ +FrameBuffer::FrameBuffer(std::vector planes, unsigned int cookie) + : request_(nullptr), cookie_(cookie) +{ + /* Clone all file descriptors and create dmabufs. */ + for (Plane plane : planes) + dmabufs_.push_back(new Dmabuf(plane.fd, plane.length)); + + /* Cache the new plane description for this frame buffer. */ + for (const Dmabuf *dmabuf : dmabufs_) { + Plane plane = { + .fd = dmabuf->fd(), + .length = dmabuf->length() + }; + + planes_.emplace_back(plane); + } +} + +FrameBuffer::~FrameBuffer() +{ + for (Dmabuf *dmabuf : dmabufs_) + delete dmabuf; +} + +/** + * \fn FrameBuffer::request() + * \brief Retrieve the request this buffer belongs to + * + * The intended callers of this method are buffer completion handlers that + * need to associate a buffer to the request it belongs to. + * + * A Buffer is associated to a request by Request::prepare() and the + * association is valid until the buffer completes. The returned request + * pointer is valid only during that interval. + * + * \return The Request the Buffer belongs to, or nullptr if the buffer is + * either completed or not associated with a request + */ + +/** + * \fn FrameBuffer::info() + * \brief Retrieve the buffer metadata information + * + * The buffer metadata information is update every time the buffer contained + * are changed, for example when it is dequeued from hardware. + * + * \return Metadata of the buffer + */ + +/** + * \fn FrameBuffer::dmabufs() + * \brief Retrieve the Dmabuf(s) from the buffer + * + * This is intended for applications who wish to access the frame buffer + * content. The array returned is one item for each plane in the frame buffer. + * + * \return Array of Dmabuf(s) + */ + +/** + * \fn FrameBuffer::planes() + * \brief Retrieve the Dmabuf(s) as plane descriptions + * + * This is intended for internal libcamera use-cases where a frame buffer needs + * to be sent over an IPC barrier. Since IPC may be in the hot-path of capture + * a dedicated cached method is provided, the result might otherwise be + * constructed from information retrieved from dmabuf(). + * + * The array returned is one item for each plane in the frame buffer. + * + * \return Array of Dmabuf(s) as plane descriptions + */ + +/** + * \fn FrameBuffer::cookie() + * \brief Retrieve the cookie associated with the buffer + * \return The cookie + */ + +/** + * \fn FrameBuffer::setCookie() + * \brief Set the cookie associated with the buffer + * \param[in] cookie The cookie to set on the request + */ + } /* namespace libcamera */ From patchwork Tue Nov 26 23:35:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2355 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CB7B61C7D for ; Wed, 27 Nov 2019 00:39:30 +0100 (CET) X-Halon-ID: fcbca656-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fcbca656-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:28 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:57 +0100 Message-Id: <20191126233620.1695316-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 07/30] ipa: Switch to FrameBuffer interface 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:31 -0000 Switch the IPA interfaces and implementations to use the Framebuffer interface. - The IPA interface is switch to use the simpler FrameBuffer::Plane container when carrying dmabuf descriptions (fd and length) over the pipeline/IPA boundary. - The RkISP1 IPA implementation takes advantage of the new simpler and safer (better control over file descriptors) FrameBuffer interface. The biggest change can be observed in the test case where it is demonstrated that FrameBuffer objects only carry the necessary number of planes and not a fixed number. As a consequence of this the test needs to verify the file descriptors it injects and not depend on the library to handle uninitialized fields. Signed-off-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 2 +- src/ipa/libipa/ipa_interface_wrapper.cpp | 7 ++--- src/ipa/rkisp1/rkisp1.cpp | 14 +++++---- src/libcamera/ipa_context_wrapper.cpp | 8 ++--- src/libcamera/ipa_interface.cpp | 7 ++--- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 ++++++++-- test/ipa/ipa_wrappers_test.cpp | 38 ++++++++++++------------ 7 files changed, 52 insertions(+), 40 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 0ea53d120fe10acf..229d1124b19ec1c9 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -105,7 +105,7 @@ struct IPAStream { struct IPABuffer { unsigned int id; - BufferMemory memory; + std::vector planes; }; struct IPAOperationData { diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp index 6a389dfa714ab535..a1aa88979d73446f 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.cpp +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -144,15 +144,14 @@ void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx, for (unsigned int i = 0; i < num_buffers; ++i) { const struct ipa_buffer &_buffer = _buffers[i]; IPABuffer &buffer = buffers[i]; - std::vector &planes = buffer.memory.planes(); + std::vector &planes = buffer.planes; buffer.id = _buffer.id; planes.resize(_buffer.num_planes); for (unsigned int j = 0; j < _buffer.num_planes; ++j) { - if (_buffer.planes[j].dmabuf != -1) - planes[j].setDmabuf(_buffer.planes[j].dmabuf, - _buffer.planes[j].length); + planes[j].fd = _buffer.planes[j].dmabuf; + planes[j].length = _buffer.planes[j].length; /** \todo Create a Dmabuf class to implement RAII. */ ::close(_buffer.planes[j].dmabuf); } diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 744a16ae5b9aa420..7f29d0351ba58e6d 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -48,7 +48,7 @@ private: void setControls(unsigned int frame); void metadataReady(unsigned int frame, unsigned int aeState); - std::map bufferInfo_; + std::map bufferInfo_; ControlInfoMap ctrls_; @@ -102,15 +102,17 @@ void IPARkISP1::configure(const std::map &streamConfig, void IPARkISP1::mapBuffers(const std::vector &buffers) { for (const IPABuffer &buffer : buffers) { - bufferInfo_[buffer.id] = buffer.memory; - bufferInfo_[buffer.id].planes()[0].mem(); + bufferInfo_[buffer.id] = new FrameBuffer(buffer.planes); + bufferInfo_[buffer.id]->dmabufs()[0]->mem(); } } void IPARkISP1::unmapBuffers(const std::vector &ids) { - for (unsigned int id : ids) + for (unsigned int id : ids) { + delete bufferInfo_[id]; bufferInfo_.erase(id); + } } void IPARkISP1::processEvent(const IPAOperationData &event) @@ -121,7 +123,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event) unsigned int bufferId = event.data[1]; const rkisp1_stat_buffer *stats = - static_cast(bufferInfo_[bufferId].planes()[0].mem()); + static_cast(bufferInfo_[bufferId]->dmabufs()[0]->mem()); updateStatistics(frame, stats); break; @@ -131,7 +133,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event) unsigned int bufferId = event.data[1]; rkisp1_isp_params_cfg *params = - static_cast(bufferInfo_[bufferId].planes()[0].mem()); + static_cast(bufferInfo_[bufferId]->dmabufs()[0]->mem()); queueRequest(frame, params, event.controls[0]); break; diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 9603fdac87150aa0..978ead927d196f02 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -149,15 +149,15 @@ void IPAContextWrapper::mapBuffers(const std::vector &buffers) for (unsigned int i = 0; i < buffers.size(); ++i) { struct ipa_buffer &c_buffer = c_buffers[i]; const IPABuffer &buffer = buffers[i]; - const std::vector &planes = buffer.memory.planes(); + const std::vector &planes = buffer.planes; c_buffer.id = buffer.id; c_buffer.num_planes = planes.size(); for (unsigned int j = 0; j < planes.size(); ++j) { - const Plane &plane = planes[j]; - c_buffer.planes[j].dmabuf = plane.dmabuf(); - c_buffer.planes[j].length = plane.length(); + const FrameBuffer::Plane &plane = planes[j]; + c_buffer.planes[j].dmabuf = plane.fd; + c_buffer.planes[j].length = plane.length; } } diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index ee3e3622f39ae85f..2f86087ee4aa414f 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -334,11 +334,10 @@ namespace libcamera { */ /** - * \var IPABuffer::memory - * \brief The buffer memory description + * \var IPABuffer::planes + * \brief The buffer planes description * - * The memory field stores the dmabuf handle and size for each plane of the - * buffer. + * Stores the dmabuf handle and length for each plane of the buffer. */ /** diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index f75b25c6787e40df..00fa4d4f19cb4aa4 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -687,14 +687,26 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, } for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { + std::vector planes; + planes.push_back({ + .fd = paramPool_.buffers()[i].planes()[0].dmabuf(), + .length = paramPool_.buffers()[i].planes()[0].length(), + }); + data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, - .memory = paramPool_.buffers()[i] }); + .planes = planes }); paramBuffers_.push(new Buffer(i)); } for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { + std::vector planes; + planes.push_back({ + .fd = statPool_.buffers()[i].planes()[0].dmabuf(), + .length = statPool_.buffers()[i].planes()[0].length(), + }); + data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, - .memory = statPool_.buffers()[i] }); + .planes = planes }); statBuffers_.push(new Buffer(i)); } diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp index a1e34ad52317f47b..022db05172fcd548 100644 --- a/test/ipa/ipa_wrappers_test.cpp +++ b/test/ipa/ipa_wrappers_test.cpp @@ -115,28 +115,28 @@ public: return report(Op_mapBuffers, TestFail); } - if (buffers[0].memory.planes().size() != 3 || - buffers[1].memory.planes().size() != 3) { + if (buffers[0].planes.size() != 3 || + buffers[1].planes.size() != 3) { cerr << "mapBuffers(): Invalid number of planes" << endl; return report(Op_mapBuffers, TestFail); } - if (buffers[0].memory.planes()[0].length() != 4096 || - buffers[0].memory.planes()[1].length() != 0 || - buffers[0].memory.planes()[2].length() != 0 || - buffers[0].memory.planes()[0].length() != 4096 || - buffers[1].memory.planes()[1].length() != 4096 || - buffers[1].memory.planes()[2].length() != 0) { + if (buffers[0].planes[0].length != 4096 || + buffers[0].planes[1].length != 0 || + buffers[0].planes[2].length != 0 || + buffers[0].planes[0].length != 4096 || + buffers[1].planes[1].length != 4096 || + buffers[1].planes[2].length != 0) { cerr << "mapBuffers(): Invalid length" << endl; return report(Op_mapBuffers, TestFail); } - if (buffers[0].memory.planes()[0].dmabuf() == -1 || - buffers[0].memory.planes()[1].dmabuf() != -1 || - buffers[0].memory.planes()[2].dmabuf() != -1 || - buffers[0].memory.planes()[0].dmabuf() == -1 || - buffers[1].memory.planes()[1].dmabuf() == -1 || - buffers[1].memory.planes()[2].dmabuf() != -1) { + if (buffers[0].planes[0].fd == 0 || + buffers[0].planes[1].fd != 0 || + buffers[0].planes[2].fd != 0 || + buffers[0].planes[0].fd == 0 || + buffers[1].planes[1].fd == 0 || + buffers[1].planes[2].fd != 0) { cerr << "mapBuffers(): Invalid dmabuf" << endl; return report(Op_mapBuffers, TestFail); } @@ -287,13 +287,13 @@ protected: /* Test mapBuffers(). */ std::vector buffers(2); - buffers[0].memory.planes().resize(3); + buffers[0].planes.resize(3); buffers[0].id = 10; - buffers[0].memory.planes()[0].setDmabuf(fd_, 4096); + buffers[0].planes[0] = { .fd = fd_, .length = 4096 }; buffers[1].id = 11; - buffers[1].memory.planes().resize(3); - buffers[1].memory.planes()[0].setDmabuf(fd_, 4096); - buffers[1].memory.planes()[1].setDmabuf(fd_, 4096); + buffers[1].planes.resize(3); + buffers[1].planes[0] = { .fd = fd_, .length = 4096 }; + buffers[1].planes[1] = { .fd = fd_, .length = 4096 }; ret = INVOKE(mapBuffers, buffers); if (ret == TestFail) From patchwork Tue Nov 26 23:35:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2356 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5128A61C62 for ; Wed, 27 Nov 2019 00:39:31 +0100 (CET) X-Halon-ID: fd52620a-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fd52620a-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:29 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:58 +0100 Message-Id: <20191126233620.1695316-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 08/30] libcamera: buffer: Switch from Plane to Dmabuf 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:31 -0000 Dmabug and Plane serves similar functions in the two buffer interfaces Buffer and FrameBuffer and share many function signatures. Replace all usages of Plane with Dmabuf to prepare for dropping the Buffer interface. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 6 +++--- src/cam/buffer_writer.cpp | 6 +++--- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/stream.cpp | 3 +-- src/libcamera/v4l2_videodevice.cpp | 14 ++++++-------- src/qcam/main_window.cpp | 4 ++-- test/camera/buffer_import.cpp | 2 +- 7 files changed, 18 insertions(+), 21 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index fe5195327b540f5c..2dd5bcf3b49c4ee8 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -101,11 +101,11 @@ private: class BufferMemory final { public: - const std::vector &planes() const { return planes_; } - std::vector &planes() { return planes_; } + const std::vector &planes() const { return planes_; } + std::vector &planes() { return planes_; } private: - std::vector planes_; + std::vector planes_; }; class BufferPool final diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index c33e99c5f8173db8..5967efca07254666 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -43,9 +43,9 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) return -errno; BufferMemory *mem = buffer->mem(); - for (Plane &plane : mem->planes()) { - void *data = plane.mem(); - unsigned int length = plane.length(); + for (Dmabuf &dmabuf : mem->planes()) { + void *data = dmabuf.mem(); + unsigned int length = dmabuf.length(); ret = ::write(fd, data, length); if (ret < 0) { diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 00fa4d4f19cb4aa4..e8b6a278e97b0ba0 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -689,7 +689,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { std::vector planes; planes.push_back({ - .fd = paramPool_.buffers()[i].planes()[0].dmabuf(), + .fd = paramPool_.buffers()[i].planes()[0].fd(), .length = paramPool_.buffers()[i].planes()[0].length(), }); @@ -701,7 +701,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { std::vector planes; planes.push_back({ - .fd = statPool_.buffers()[i].planes()[0].dmabuf(), + .fd = statPool_.buffers()[i].planes()[0].fd(), .length = statPool_.buffers()[i].planes()[0].length(), }); diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 45f31ae1e2daeb53..e70a1e307ecaa5ba 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -577,8 +577,7 @@ int Stream::mapBuffer(const Buffer *buffer) if (dmabufs[i] == -1) break; - mem->planes().emplace_back(); - mem->planes().back().setDmabuf(dmabufs[i], 0); + mem->planes().emplace_back(dmabufs[i], 0); } /* Remove the buffer from the cache and return its index. */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index dbb5c3982334e243..166b0abc1b101f88 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -921,9 +921,7 @@ int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, return ret; } - buffer->planes().emplace_back(); - Plane &plane = buffer->planes().back(); - plane.setDmabuf(expbuf.fd, length); + buffer->planes().emplace_back(expbuf.fd, length); ::close(expbuf.fd); return 0; @@ -986,19 +984,19 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); BufferMemory *mem = &bufferPool_->buffers()[buf.index]; - const std::vector &planes = mem->planes(); + const std::vector &dmabufs = mem->planes(); if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { - for (unsigned int p = 0; p < planes.size(); ++p) - v4l2Planes[p].m.fd = planes[p].dmabuf(); + for (unsigned int p = 0; p < dmabufs.size(); ++p) + v4l2Planes[p].m.fd = dmabufs[p].fd(); } else { - buf.m.fd = planes[0].dmabuf(); + buf.m.fd = dmabufs[0].fd(); } } if (multiPlanar) { - buf.length = planes.size(); + buf.length = dmabufs.size(); buf.m.planes = v4l2Planes; } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 0c7ca61ac12ec41c..a828a176cbbf4854 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -300,8 +300,8 @@ int MainWindow::display(Buffer *buffer) if (mem->planes().size() != 1) return -EINVAL; - Plane &plane = mem->planes().front(); - unsigned char *raw = static_cast(plane.mem()); + Dmabuf &dmabuf = mem->planes().front(); + unsigned char *raw = static_cast(dmabuf.mem()); viewfinder_->display(raw, buffer->bytesused()); return 0; diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 3efe02704c02f691..8b0d1c167bd2bbe8 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -178,7 +178,7 @@ private: uint64_t cookie = index; BufferMemory &mem = pool_.buffers()[index]; - int dmabuf = mem.planes()[0].dmabuf(); + int dmabuf = mem.planes()[0].fd(); requestReady.emit(cookie, dmabuf); From patchwork Tue Nov 26 23:35:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2357 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 039C361C62 for ; Wed, 27 Nov 2019 00:39:31 +0100 (CET) X-Halon-ID: fdbb0c7d-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fdbb0c7d-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:30 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:35:59 +0100 Message-Id: <20191126233620.1695316-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 09/30] libcamera: buffers: Remove Plane class 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:32 -0000 There are no users left of the Plane class, drop it. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 21 ------ src/libcamera/buffer.cpp | 149 ------------------------------------- 2 files changed, 170 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 2dd5bcf3b49c4ee8..afcef805801a43b5 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -77,27 +77,6 @@ private: void *mem_; }; -class Plane final -{ -public: - Plane(); - ~Plane(); - - int dmabuf() const { return fd_; } - int setDmabuf(int fd, unsigned int length); - - void *mem(); - unsigned int length() const { return length_; } - -private: - int mmap(); - int munmap(); - - int fd_; - unsigned int length_; - void *mem_; -}; - class BufferMemory final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 07647124a2cd9c62..82b4799a2510d02f 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -155,28 +155,6 @@ FileDescriptor::~FileDescriptor() * \return A valid file descriptor or a negative error code */ -/** - * \class Plane - * \brief A memory region to store a single plane of a frame - * - * Planar pixel formats use multiple memory regions to store planes - * corresponding to the different colour components of a frame. The Plane class - * tracks the specific details of a memory region used to store a single plane - * for a given frame and provides the means to access the memory, both for the - * application and for DMA. A Buffer then contains one or multiple planes - * depending on its pixel format. - * - * To support DMA access, planes are associated with dmabuf objects represented - * by file handles. Each plane carries a dmabuf file handle and an offset within - * the buffer. Those file handles may refer to the same dmabuf object, depending - * on whether the devices accessing the memory regions composing the image - * support non-contiguous DMA to planes ore require DMA-contiguous memory. - * - * To support CPU access, planes carry the CPU address of their backing memory. - * Similarly to the dmabuf file handles, the CPU addresses for planes composing - * an image may or may not be contiguous. - */ - /** * \class Dmabuf * \brief A memory region to store a single plane of a frame @@ -290,133 +268,6 @@ int Dmabuf::munmap() return ret; } -Plane::Plane() - : fd_(-1), length_(0), mem_(0) -{ -} - -Plane::~Plane() -{ - munmap(); - - if (fd_ != -1) - close(fd_); -} - -/** - * \fn Plane::dmabuf() - * \brief Get the dmabuf file handle backing the buffer - */ - -/** - * \brief Set the dmabuf file handle backing the buffer - * \param[in] fd The dmabuf file handle - * \param[in] length The size of the memory region - * - * The \a fd dmabuf file handle is duplicated and stored. The caller may close - * the original file handle. - * - * \return 0 on success or a negative error code otherwise - */ -int Plane::setDmabuf(int fd, unsigned int length) -{ - if (fd < 0) { - LOG(Buffer, Error) << "Invalid dmabuf fd provided"; - return -EINVAL; - } - - if (fd_ != -1) { - close(fd_); - fd_ = -1; - } - - fd_ = dup(fd); - if (fd_ == -1) { - int ret = -errno; - LOG(Buffer, Error) - << "Failed to duplicate dmabuf: " << strerror(-ret); - return ret; - } - - length_ = length; - - return 0; -} - -/** - * \brief Map the plane memory data to a CPU accessible address - * - * The file descriptor to map the memory from must be set by a call to - * setDmaBuf() before calling this function. - * - * \sa setDmaBuf() - * - * \return 0 on success or a negative error code otherwise - */ -int Plane::mmap() -{ - void *map; - - if (mem_) - return 0; - - map = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd_, 0); - if (map == MAP_FAILED) { - int ret = -errno; - LOG(Buffer, Error) - << "Failed to mmap plane: " << strerror(-ret); - return ret; - } - - mem_ = map; - - return 0; -} - -/** - * \brief Unmap any existing CPU accessible mapping - * - * Unmap the memory mapped by an earlier call to mmap(). - * - * \return 0 on success or a negative error code otherwise - */ -int Plane::munmap() -{ - int ret = 0; - - if (mem_) - ret = ::munmap(mem_, length_); - - if (ret) { - ret = -errno; - LOG(Buffer, Warning) - << "Failed to unmap plane: " << strerror(-ret); - } else { - mem_ = 0; - } - - return ret; -} - -/** - * \fn Plane::mem() - * \brief Retrieve the CPU accessible memory address of the Plane - * \return The CPU accessible memory address on success or nullptr otherwise. - */ -void *Plane::mem() -{ - if (!mem_) - mmap(); - - return mem_; -} - -/** - * \fn Plane::length() const - * \brief Retrieve the length of the memory region - * \return The length of the memory region - */ - /** * \class BufferMemory * \brief A memory buffer to store an image From patchwork Tue Nov 26 23:36:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2358 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FFA861C8E for ; Wed, 27 Nov 2019 00:39:32 +0100 (CET) X-Halon-ID: fe29bf64-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fe29bf64-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:31 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:00 +0100 Message-Id: <20191126233620.1695316-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 10/30] libcamera: buffer: Drop private function setRequest() 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:33 -0000 There is no need to have a private helper function to access a private data member when a friend statement is needed anyhow. Remove the helper function to simplify the code and make it clear that a private member of Buffer is accessed. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 2 -- src/libcamera/buffer.cpp | 8 -------- src/libcamera/request.cpp | 4 ++-- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index afcef805801a43b5..d6db6138ca11d5fe 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -135,8 +135,6 @@ private: void cancel(); - void setRequest(Request *request) { request_ = request; } - unsigned int index_; std::array dmabuf_; BufferMemory *mem_; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 82b4799a2510d02f..7043345c3f3207cd 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -462,7 +462,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * * \return The Request the Buffer belongs to, or nullptr if the buffer is * either completed or not associated with a request - * \sa Buffer::setRequest() */ /** @@ -489,13 +488,6 @@ void Buffer::cancel() status_ = BufferCancelled; } -/** - * \fn Buffer::setRequest() - * \brief Set the request this buffer belongs to - * - * The intended callers are Request::prepare() and Request::completeBuffer(). - */ - /** * \class FrameBuffer * \brief A buffer handle and dynamic metadata diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index c14ed1a4d3ce55d0..c2854dc2e8caab2e 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -220,7 +220,7 @@ int Request::prepare() for (auto const &pair : bufferMap_) { Buffer *buffer = pair.second; - buffer->setRequest(this); + buffer->request_ = this; pending_.insert(buffer); } @@ -258,7 +258,7 @@ bool Request::completeBuffer(Buffer *buffer) int ret = pending_.erase(buffer); ASSERT(ret == 1); - buffer->setRequest(nullptr); + buffer->request_ = nullptr; if (buffer->status() == Buffer::BufferCancelled) cancelled_ = true; From patchwork Tue Nov 26 23:36:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2359 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 03B2C61C6A for ; Wed, 27 Nov 2019 00:39:32 +0100 (CET) X-Halon-ID: fe84577b-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fe84577b-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:31 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:01 +0100 Message-Id: <20191126233620.1695316-12-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 11/30] libcamera: v4l2_videodevice: Align which type variable is used in queueBuffer() 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:33 -0000 Reading V4L2VideoDevice::queueBuffer() is confusing since buf.type is first set to bufferType_ but then both variables are used in V4L2 macros to operate based on which type of buffer is being processed. Aligen on only using buf.type since it have the most existing users. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/v4l2_videodevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 166b0abc1b101f88..7b6fa5347ef320f8 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1000,7 +1000,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) buf.m.planes = v4l2Planes; } - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) { + if (V4L2_TYPE_IS_OUTPUT(buf.type)) { buf.bytesused = buffer->bytesused_; buf.sequence = buffer->sequence_; buf.timestamp.tv_sec = buffer->timestamp_ / 1000000000; From patchwork Tue Nov 26 23:36:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2360 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 84CBB61C97 for ; Wed, 27 Nov 2019 00:39:33 +0100 (CET) X-Halon-ID: fece51f7-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id fece51f7-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:32 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:02 +0100 Message-Id: <20191126233620.1695316-13-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 12/30] libcamera: v4l2_videodevice: Remove assertion involving BufferPool 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:33 -0000 The BufferPool class will be removed when the buffer logic is reworked. During the transition some functions will be shared between the new and old buffer code and removing this assert allows sharing of the dequeueBuffer() function. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/v4l2_videodevice.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 7b6fa5347ef320f8..644e4545a2f33b2e 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1100,7 +1100,6 @@ Buffer *V4L2VideoDevice::dequeueBuffer() return nullptr; } - ASSERT(buf.index < bufferPool_->count()); LOG(V4L2, Debug) << "Buffer " << buf.index << " is available"; auto it = queuedBuffers_.find(buf.index); From patchwork Tue Nov 26 23:36:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2361 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 16C6461CAA for ; Wed, 27 Nov 2019 00:39:34 +0100 (CET) X-Halon-ID: ff16ac83-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id ff16ac83-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:32 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:03 +0100 Message-Id: <20191126233620.1695316-14-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 13/30] libcamera: v4l2_videodevice: Extract exportDmaBuffer() to export DMA buffer 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:35 -0000 The part in createPlane() that exports a dma buffer from a video device will be used directly by the FrameBuffer interface. Break it out to a own function. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/include/v4l2_videodevice.h | 1 + src/libcamera/v4l2_videodevice.cpp | 41 +++++++++++++++--------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index fdf11b3a6ec9b702..34bbff41760753bd 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -179,6 +179,7 @@ private: int requestBuffers(unsigned int count); int createPlane(BufferMemory *buffer, unsigned int index, unsigned int plane, unsigned int length); + int exportDmaBuffer(unsigned int index, unsigned int plane); Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 644e4545a2f33b2e..cc0a1c9382a2b1ed 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -901,28 +901,19 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool) int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, unsigned int planeIndex, unsigned int length) { - struct v4l2_exportbuffer expbuf = {}; - int ret; + int fd; LOG(V4L2, Debug) << "Buffer " << index << " plane " << planeIndex << ": length=" << length; - expbuf.type = bufferType_; - expbuf.index = index; - expbuf.plane = planeIndex; - expbuf.flags = O_RDWR; - - ret = ioctl(VIDIOC_EXPBUF, &expbuf); - if (ret < 0) { - LOG(V4L2, Error) - << "Failed to export buffer: " << strerror(-ret); - return ret; - } + fd = exportDmaBuffer(index, planeIndex); + if (fd < 0) + return fd; - buffer->planes().emplace_back(expbuf.fd, length); - ::close(expbuf.fd); + buffer->planes().emplace_back(fd, length); + ::close(fd); return 0; } @@ -948,6 +939,26 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool) return 0; } +int V4L2VideoDevice::exportDmaBuffer(unsigned int index, unsigned int plane) +{ + struct v4l2_exportbuffer expbuf = {}; + int ret; + + expbuf.type = bufferType_; + expbuf.index = index; + expbuf.plane = plane; + expbuf.flags = O_RDWR; + + ret = ioctl(VIDIOC_EXPBUF, &expbuf); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to export buffer: " << strerror(-ret); + return ret; + } + + return expbuf.fd; +} + /** * \brief Release all internally allocated buffers */ From patchwork Tue Nov 26 23:36:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2362 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C4D846136B for ; Wed, 27 Nov 2019 00:39:34 +0100 (CET) X-Halon-ID: ff6ce578-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id ff6ce578-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:33 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:04 +0100 Message-Id: <20191126233620.1695316-15-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 14/30] libcamera: request: In addBuffer() do not fetch stream from Buffer 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:35 -0000 In the FrameBuffer interface the stream will not be available from the buffer object as the buffer might be allocated externally. The application needs to explicitly state which stream the buffer is being added for to the request. Extend the addBuffer() function to get this information explicitly from the caller. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/request.h | 2 +- src/android/camera_device.cpp | 2 +- src/cam/capture.cpp | 4 ++-- src/libcamera/request.cpp | 4 ++-- src/qcam/main_window.cpp | 4 ++-- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 4 ++-- test/camera/statemachine.cpp | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 2d5a5964e99eb75f..a8708010ec1247a2 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -39,7 +39,7 @@ public: ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } const std::map &buffers() const { return bufferMap_; } - int addBuffer(std::unique_ptr buffer); + int addBuffer(Stream *stream, std::unique_ptr buffer); Buffer *findBuffer(Stream *stream) const; uint64_t cookie() const { return cookie_; } diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 065e0292e186c2ad..09588c16a6301649 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -754,7 +754,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque Request *request = camera_->createRequest(reinterpret_cast(descriptor)); - request->addBuffer(std::move(buffer)); + request->addBuffer(stream, std::move(buffer)); int ret = camera_->queueRequest(request); if (ret) { diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 4b65b1d0a2dbed35..1a4dbe7ce4a15a2d 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -95,7 +95,7 @@ int Capture::capture(EventLoop *loop) Stream *stream = cfg.stream(); std::unique_ptr buffer = stream->createBuffer(i); - ret = request->addBuffer(std::move(buffer)); + ret = request->addBuffer(stream, std::move(buffer)); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; @@ -185,7 +185,7 @@ void Capture::requestComplete(Request *request) return; } - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, std::move(newBuffer)); } camera_->queueRequest(request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index c2854dc2e8caab2e..3b49e52510918eee 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -113,6 +113,7 @@ Request::~Request() /** * \brief Store a Buffer with its associated Stream in the Request + * \param[in] stream The stream the buffer belongs to * \param[in] buffer The Buffer to store in the request * * Ownership of the buffer is passed to the request. It will be deleted when @@ -125,9 +126,8 @@ Request::~Request() * \retval -EEXIST The request already contains a buffer for the stream * \retval -EINVAL The buffer does not reference a valid Stream */ -int Request::addBuffer(std::unique_ptr buffer) +int Request::addBuffer(Stream *stream, std::unique_ptr buffer) { - Stream *stream = buffer->stream(); if (!stream) { LOG(Request, Error) << "Invalid stream reference"; return -EINVAL; diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index a828a176cbbf4854..4cecf7e351214f3d 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -190,7 +190,7 @@ int MainWindow::startCapture() goto error; } - ret = request->addBuffer(std::move(buffer)); + ret = request->addBuffer(stream, std::move(buffer)); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; goto error; @@ -288,7 +288,7 @@ void MainWindow::requestComplete(Request *request) return; } - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, std::move(newBuffer)); } camera_->queueRequest(request); diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 8b0d1c167bd2bbe8..dae907f9f41841c9 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -268,7 +268,7 @@ public: Request *request = camera_->createRequest(cookie); std::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 }); - request->addBuffer(move(buffer)); + request->addBuffer(stream_, move(buffer)); camera_->queueRequest(request); } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 7cb76038f557d461..ca1ebe419946dd4d 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -49,7 +49,7 @@ protected: std::unique_ptr newBuffer = stream->createBuffer(buffer->index()); request = camera_->createRequest(); - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, std::move(newBuffer)); camera_->queueRequest(request); } @@ -101,7 +101,7 @@ protected: return TestFail; } - if (request->addBuffer(std::move(buffer))) { + if (request->addBuffer(stream, std::move(buffer))) { cout << "Failed to associating buffer with request" << endl; return TestFail; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index afa13ba77b0b7fe0..f627b8f37422350e 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -219,7 +219,7 @@ protected: Stream *stream = *camera_->streams().begin(); std::unique_ptr buffer = stream->createBuffer(0); - if (request->addBuffer(std::move(buffer))) + if (request->addBuffer(stream, std::move(buffer))) return TestFail; if (camera_->queueRequest(request)) From patchwork Tue Nov 26 23:36:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2363 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B40246136B for ; Wed, 27 Nov 2019 00:39:35 +0100 (CET) X-Halon-ID: ffd6d11a-10a5-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id ffd6d11a-10a5-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:33 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:05 +0100 Message-Id: <20191126233620.1695316-16-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 15/30] libcamera: buffer: Move capture information to BufferInfo 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:36 -0000 Move the metadata information retrieved when dequeuing a V4L2 buffer into a BufferInfo. This is done as a step to migrate to the FrameBuffer interface as the functions added to Buffer around BufferInfo matches the one in FrameBuffer. Signed-off-by: Niklas Söderlund --- include/libcamera/buffer.h | 2 ++ src/android/camera_device.cpp | 4 +-- src/cam/buffer_writer.cpp | 2 +- src/cam/capture.cpp | 8 ++++-- src/libcamera/buffer.cpp | 34 +++++++++++++++++------- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +++--- src/libcamera/request.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 23 +++++++++------- src/qcam/main_window.cpp | 13 ++++----- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 2 +- test/v4l2_videodevice/buffer_sharing.cpp | 12 ++++++--- 13 files changed, 72 insertions(+), 44 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index d6db6138ca11d5fe..2e5376fb8b53a4c5 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -122,6 +122,7 @@ public: unsigned int bytesused() const { return bytesused_; } uint64_t timestamp() const { return timestamp_; } unsigned int sequence() const { return sequence_; } + const BufferInfo &info() const { return info_; }; Status status() const { return status_; } Request *request() const { return request_; } @@ -142,6 +143,7 @@ private: unsigned int bytesused_; uint64_t timestamp_; unsigned int sequence_; + BufferInfo info_; Status status_; Request *request_; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 09588c16a6301649..55b29a9a41ab8943 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -803,11 +803,11 @@ void CameraDevice::requestComplete(Request *request) if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, - libcameraBuffer->timestamp()); + libcameraBuffer->info().timestamp()); captureResult.partial_result = 1; resultMetadata = getResultMetadata(descriptor->frameNumber, - libcameraBuffer->timestamp()); + libcameraBuffer->info().timestamp()); captureResult.result = resultMetadata->get(); } diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index 5967efca07254666..b6b40baeee661df6 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -32,7 +32,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) if (pos != std::string::npos) { std::stringstream ss; ss << streamName << "-" << std::setw(6) - << std::setfill('0') << buffer->sequence(); + << std::setfill('0') << buffer->info().sequence(); filename.replace(pos, 1, ss.str()); } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 1a4dbe7ce4a15a2d..a4fa88a8d99669bc 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -155,8 +155,12 @@ void Capture::requestComplete(Request *request) const std::string &name = streamName_[stream]; info << " " << name - << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused(); + << " seq: " << std::setw(6) << std::setfill('0') << buffer->info().sequence(); + + unsigned int nplane = 0; + for (const BufferInfo::Plane &plane : buffer->info().planes()) + info << " bytesused(" << nplane++ << "): " + << plane.bytesused; if (writer_) writer_->write(buffer, name); diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 7043345c3f3207cd..d5a4815a0bb8c528 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -375,15 +375,22 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) status_(Buffer::BufferSuccess), request_(nullptr), stream_(nullptr) { + unsigned int sequence; + uint64_t timestamp; + unsigned int bytesused; + if (metadata) { - bytesused_ = metadata->bytesused_; - sequence_ = metadata->sequence_; - timestamp_ = metadata->timestamp_; + bytesused = metadata->info().planes()[0].bytesused; + sequence = metadata->info().sequence(); + timestamp = metadata->info().timestamp(); } else { - bytesused_ = 0; - sequence_ = 0; - timestamp_ = 0; + bytesused = 0; + sequence = 0; + timestamp = 0; } + + info_.update(BufferInfo::BufferSuccess, sequence, timestamp, + { { bytesused } }); } /** @@ -439,6 +446,16 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * \return Sequence number of the buffer */ +/** + * \fn Buffer::info() + * \brief Retrieve the buffer metadata information + * + * The buffer metadata information is update every time the buffer contained + * are changed, for example when it is dequeued from hardware. + * + * \return Metadata of the buffer + */ + /** * \fn Buffer::status() * \brief Retrieve the buffer status @@ -482,10 +499,7 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) */ void Buffer::cancel() { - bytesused_ = 0; - timestamp_ = 0; - sequence_ = 0; - status_ = BufferCancelled; + info_.update(BufferInfo::BufferCancelled, 0, 0, { {} }); } /** diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ad223d9101bdc6ed..8ba08351c950f5e2 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -928,7 +928,7 @@ int PipelineHandlerIPU3::registerCameras() void IPU3CameraData::imguInputBufferReady(Buffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->info().status() == BufferInfo::BufferCancelled) return; cio2_.output_->queueBuffer(buffer); @@ -962,7 +962,7 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) void IPU3CameraData::cio2BufferReady(Buffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->info().status() == BufferInfo::BufferCancelled) return; imgu_->input_->queueBuffer(buffer); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index e8b6a278e97b0ba0..6ad9b57d8353896c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -100,10 +100,10 @@ public: ASSERT(frameOffset(SOE) == 0); utils::time_point soe = std::chrono::time_point() - + std::chrono::nanoseconds(buffer->timestamp()) + + std::chrono::nanoseconds(buffer->info().timestamp()) + timeOffset(SOE); - notifyStartOfExposure(buffer->sequence(), soe); + notifyStartOfExposure(buffer->info().sequence(), soe); } void setDelay(unsigned int type, int frame, int msdelay) @@ -1006,8 +1006,8 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) data->timeline_.bufferReady(buffer); - if (data->frame_ <= buffer->sequence()) - data->frame_ = buffer->sequence() + 1; + if (data->frame_ <= buffer->info().sequence()) + data->frame_ = buffer->info().sequence() + 1; completeBuffer(activeCamera_, request, buffer); tryCompleteRequest(request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 3b49e52510918eee..7593bf9dfa546401 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -260,7 +260,7 @@ bool Request::completeBuffer(Buffer *buffer) buffer->request_ = nullptr; - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->info().status() == BufferInfo::BufferCancelled) cancelled_ = true; return !hasPendingBuffers(); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index cc0a1c9382a2b1ed..8f962c7e9d0c7d01 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1012,10 +1012,12 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) } if (V4L2_TYPE_IS_OUTPUT(buf.type)) { - buf.bytesused = buffer->bytesused_; - buf.sequence = buffer->sequence_; - buf.timestamp.tv_sec = buffer->timestamp_ / 1000000000; - buf.timestamp.tv_usec = (buffer->timestamp_ / 1000) % 1000000; + const BufferInfo &info = buffer->info(); + + buf.bytesused = info.planes()[0].bytesused; + buf.sequence = info.sequence(); + buf.timestamp.tv_sec = info.timestamp() / 1000000000; + buf.timestamp.tv_usec = (info.timestamp() / 1000) % 1000000; } LOG(V4L2, Debug) << "Queueing buffer " << buf.index; @@ -1121,12 +1123,13 @@ Buffer *V4L2VideoDevice::dequeueBuffer() fdEvent_->setEnabled(false); buffer->index_ = buf.index; - buffer->bytesused_ = buf.bytesused; - buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL - + buf.timestamp.tv_usec * 1000ULL; - buffer->sequence_ = buf.sequence; - buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR - ? Buffer::BufferError : Buffer::BufferSuccess; + + BufferInfo::Status status = buf.flags & V4L2_BUF_FLAG_ERROR + ? BufferInfo::BufferError : BufferInfo::BufferSuccess; + uint64_t timestamp = buf.timestamp.tv_sec * 1000000000ULL + + buf.timestamp.tv_usec * 1000ULL; + + buffer->info_.update(status, buf.sequence, timestamp, { { buf.bytesused } }); return buffer; } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 4cecf7e351214f3d..771020112f09b1ef 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -258,14 +258,15 @@ void MainWindow::requestComplete(Request *request) framesCaptured_++; Buffer *buffer = buffers.begin()->second; + const BufferInfo &info = buffer->info(); - double fps = buffer->timestamp() - lastBufferTime_; + double fps = info.timestamp() - lastBufferTime_; fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0; - lastBufferTime_ = buffer->timestamp(); + lastBufferTime_ = info.timestamp(); - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused() - << " timestamp: " << buffer->timestamp() + std::cout << "seq: " << std::setw(6) << std::setfill('0') << info.sequence() + << " bytesused: " << info.planes()[0].bytesused + << " timestamp: " << info.timestamp() << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; @@ -302,7 +303,7 @@ int MainWindow::display(Buffer *buffer) Dmabuf &dmabuf = mem->planes().front(); unsigned char *raw = static_cast(dmabuf.mem()); - viewfinder_->display(raw, buffer->bytesused()); + viewfinder_->display(raw, buffer->info().planes()[0].bytesused); return 0; } diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index dae907f9f41841c9..5dbaed9255d3d60c 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -275,7 +275,7 @@ public: protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + if (buffer->info().status() != BufferInfo::BufferSuccess) return; unsigned int index = buffer->index(); diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index ca1ebe419946dd4d..8307ea2629801679 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -28,7 +28,7 @@ protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + if (buffer->info().status() != BufferInfo::BufferSuccess) return; completeBuffersCount_++; diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index d02c391b95933922..6b71caef111693d6 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -92,10 +92,12 @@ protected: void captureBufferReady(Buffer *buffer) { + const BufferInfo &info = buffer->info(); + std::cout << "Received capture buffer sequence " - << buffer->sequence() << std::endl; + << info.sequence() << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (info.status() != BufferInfo::BufferSuccess) return; output_->queueBuffer(buffer); @@ -104,10 +106,12 @@ protected: void outputBufferReady(Buffer *buffer) { + const BufferInfo &info = buffer->info(); + std::cout << "Received output buffer sequence " - << buffer->sequence() << std::endl; + << info.sequence() << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (info.status() != BufferInfo::BufferSuccess) return; capture_->queueBuffer(buffer); From patchwork Tue Nov 26 23:36:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2364 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7421C61C95 for ; Wed, 27 Nov 2019 00:39:36 +0100 (CET) X-Halon-ID: 0072a09f-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 0072a09f-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:34 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:06 +0100 Message-Id: <20191126233620.1695316-17-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 16/30] libcamera: buffer: Buffer remove metadata information 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:37 -0000 All metadata information is now stored in the associated BufferInfo object, remove all metadata information from the Buffer object. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- include/libcamera/buffer.h | 14 ---------- src/libcamera/buffer.cpp | 53 +------------------------------------- 2 files changed, 1 insertion(+), 66 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 2e5376fb8b53a4c5..acc876eec7d93873 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -105,12 +105,6 @@ private: class Buffer final { public: - enum Status { - BufferSuccess, - BufferError, - BufferCancelled, - }; - Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); Buffer(const Buffer &) = delete; Buffer &operator=(const Buffer &) = delete; @@ -119,12 +113,8 @@ public: const std::array &dmabufs() const { return dmabuf_; } BufferMemory *mem() { return mem_; } - unsigned int bytesused() const { return bytesused_; } - uint64_t timestamp() const { return timestamp_; } - unsigned int sequence() const { return sequence_; } const BufferInfo &info() const { return info_; }; - Status status() const { return status_; } Request *request() const { return request_; } Stream *stream() const { return stream_; } @@ -140,12 +130,8 @@ private: std::array dmabuf_; BufferMemory *mem_; - unsigned int bytesused_; - uint64_t timestamp_; - unsigned int sequence_; BufferInfo info_; - Status status_; Request *request_; Stream *stream_; }; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index d5a4815a0bb8c528..ab28e0d76b8c40f7 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -349,20 +349,6 @@ void BufferPool::destroyBuffers() * deleted automatically after the request complete handler returns. */ -/** - * \enum Buffer::Status - * Buffer completion status - * \var Buffer::BufferSuccess - * The buffer has completed with success and contains valid data. All its other - * metadata (such as bytesused(), timestamp() or sequence() number) are valid. - * \var Buffer::BufferError - * The buffer has completed with an error and doesn't contain valid data. Its - * other metadata are valid. - * \var Buffer::BufferCancelled - * The buffer has been cancelled due to capture stop. Its other metadata are - * invalid and shall not be used. - */ - /** * \brief Construct a buffer not associated with any stream * @@ -371,8 +357,7 @@ void BufferPool::destroyBuffers() * for a stream with Stream::createBuffer(). */ Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), dmabuf_({ -1, -1, -1 }), - status_(Buffer::BufferSuccess), request_(nullptr), + : index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr), stream_(nullptr) { unsigned int sequence; @@ -420,32 +405,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * \return The BufferMemory this buffer is associated with */ -/** - * \fn Buffer::bytesused() - * \brief Retrieve the number of bytes occupied by the data in the buffer - * \return Number of bytes occupied in the buffer - */ - -/** - * \fn Buffer::timestamp() - * \brief Retrieve the time when the buffer was processed - * - * The timestamp is expressed as a number of nanoseconds since the epoch. - * - * \return Timestamp when the buffer was processed - */ - -/** - * \fn Buffer::sequence() - * \brief Retrieve the buffer sequence number - * - * The sequence number is a monotonically increasing number assigned to the - * buffer processed by the stream. Gaps in the sequence numbers indicate - * dropped frames. - * - * \return Sequence number of the buffer - */ - /** * \fn Buffer::info() * \brief Retrieve the buffer metadata information @@ -456,16 +415,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * \return Metadata of the buffer */ -/** - * \fn Buffer::status() - * \brief Retrieve the buffer status - * - * The buffer status reports whether the buffer has completed successfully - * (BufferSuccess) or if an error occurred (BufferError). - * - * \return The buffer status - */ - /** * \fn Buffer::request() * \brief Retrieve the request this buffer belongs to From patchwork Tue Nov 26 23:36:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2365 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B6656136C for ; Wed, 27 Nov 2019 00:39:38 +0100 (CET) X-Halon-ID: 00df5d30-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 00df5d30-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:36 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:07 +0100 Message-Id: <20191126233620.1695316-18-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 17/30] libcamera: v4l2_videodevice: Add support for multi plane output buffers 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:38 -0000 When queuing an output buffer it was assumed it only had one plane. With the recent rework of the buffer code it's now trivial to add support multi plane output buffers, add support for it. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/v4l2_videodevice.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 8f962c7e9d0c7d01..a05dd6a1f7d86eaa 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1014,7 +1014,17 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) if (V4L2_TYPE_IS_OUTPUT(buf.type)) { const BufferInfo &info = buffer->info(); - buf.bytesused = info.planes()[0].bytesused; + if (multiPlanar) { + unsigned int nplane = 0; + for (const BufferInfo::Plane &plane : info.planes()) { + v4l2Planes[nplane].bytesused = plane.bytesused; + nplane++; + } + } else { + if (info.planes().size()) + buf.bytesused = info.planes()[0].bytesused; + } + buf.sequence = info.sequence(); buf.timestamp.tv_sec = info.timestamp() / 1000000000; buf.timestamp.tv_usec = (info.timestamp() / 1000) % 1000000; From patchwork Tue Nov 26 23:36:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2366 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A561C61C6F for ; Wed, 27 Nov 2019 00:39:38 +0100 (CET) X-Halon-ID: 01c2c0f2-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 01c2c0f2-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:37 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:08 +0100 Message-Id: <20191126233620.1695316-19-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 18/30] libcamera: v4l2_videodevice: Add V4L2BufferCache to deal with index mapping 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:39 -0000 In preparation for the FrameBuffer interface add a class which will deal with keeping the cache between dmafds and V4L2 video device buffer indexes. This initial implement ensures that no hot association is lost while its eviction strategy could be improved in the future. Signed-off-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 20 ++++- src/libcamera/v4l2_videodevice.cpp | 105 ++++++++++++++++++++++- 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 34bbff41760753bd..254f8797af42dd8a 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -12,6 +12,7 @@ #include +#include #include #include #include @@ -22,7 +23,6 @@ namespace libcamera { -class Buffer; class BufferMemory; class BufferPool; class EventNotifier; @@ -105,6 +105,24 @@ struct V4L2Capability final : v4l2_capability { } }; +class V4L2BufferCache +{ +public: + V4L2BufferCache(unsigned int size); + V4L2BufferCache(const std::vector buffers); + + int fetch(const FrameBuffer *buffer); + void put(unsigned int index); + +private: + struct CacheInfo { + bool free; + std::vector last; + }; + + std::vector cache_; +}; + class V4L2DeviceFormat { public: diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index a05dd6a1f7d86eaa..c82f2829601bd14c 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -19,7 +19,6 @@ #include -#include #include #include "log.h" @@ -134,6 +133,110 @@ LOG_DECLARE_CATEGORY(V4L2) * \return True if the video device provides Streaming I/O IOCTLs */ +/** + * \class V4L2BufferCache + * \brief Hot cache of associations between V4L2 index and FrameBuffer + * + * There is performance to be gained if the same V4L2 buffer index can be + * reused for the same FrameBuffer object as the kernel don't have to redo + * the mapping. The V4L2BufferCache tries to keep a hot-cache of mappings + * between the two. + * + * If there is a cache miss is not critical, everything still works as expected. + */ + +/** + * \brief Create a empty cache of a given size + * \param[in] size Size of cache to create + * + * Create a cold cache with \a size entries. The cache will be populated as + * it's being used. + */ +V4L2BufferCache::V4L2BufferCache(unsigned int size) +{ + cache_.resize(size, { .free = true, .last = {} }); +} + +/** + * \brief Create a pre-populated cache + * \param[in] buffers Array of buffers to pre-populated with + * + * Create a warm cache from \a buffers. + */ +V4L2BufferCache::V4L2BufferCache(const std::vector buffers) +{ + for (const FrameBuffer *buffer : buffers) + cache_.push_back({ .free = true, .last = buffer->planes() }); +} + +/** + * \brief Fetch a index from the cache + * \param[in] buffer FrameBuffer to match + * + * Try to find \a buffer in cache and if it's free reuse the last used index + * for this buffer. If the buffer have never been seen or if have been evinced + * from the cache the first free index is pieced instead. Likewise if the last + * used index is in use a new free index is picked. + * + * When an index is picked it is marked as in-use and returned to the caller. + * The association is also recorded so it if possible can reused the next time + * the FrameBuffer is seen. + * + * \return V4L2 buffer index + */ +int V4L2BufferCache::fetch(const FrameBuffer *buffer) +{ + int use = -1; + + for (unsigned int index = 0; index < cache_.size(); index++) { + if (!cache_[index].free) + continue; + + if (use < 0) + use = index; + + /* Try to find a cache hit by comparing the planes. */ + std::vector planes = buffer->planes(); + if (cache_[index].last.size() != planes.size()) + continue; + + bool match = true; + for (unsigned int i = 0; i < planes.size(); i++) { + if (cache_[index].last[i].fd != planes[i].fd || + cache_[index].last[i].length != planes[i].length) { + match = false; + break; + } + } + + if (!match) + continue; + + use = index; + break; + } + + if (use < 0) + return -ENOENT; + + cache_[use].free = false; + cache_[use].last = buffer->planes(); + + return use; +} + +/** + * \brief Pit a V4L2 index back in the cache + * \param[in] index V4L2 index + * + * Mark the \a index as free in the cache so it can be reused. + */ +void V4L2BufferCache::put(unsigned int index) +{ + ASSERT(index < cache_.size()); + cache_[index].free = true; +} + /** * \class V4L2DeviceFormat * \brief The V4L2 video device image format and sizes From patchwork Tue Nov 26 23:36:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2367 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7ABB26136C for ; Wed, 27 Nov 2019 00:39:39 +0100 (CET) X-Halon-ID: 022e8134-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 022e8134-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:37 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:09 +0100 Message-Id: <20191126233620.1695316-20-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 19/30] libcamera: v4l2_videodevice: Add new buffer interface 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:39 -0000 Extend V4L2VideoDevice with two new functions, one to deal with allocating buffers from the video device and one to prepare the video device to use external buffers. The two new functions intend to replace exportBuffers() and importBuffers() once the transition to the FrameBuffer interface is complete. Signed-off-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 4 + src/libcamera/v4l2_videodevice.cpp | 126 ++++++++++++++++++++++- 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 254f8797af42dd8a..f4cbcfbd26ea540c 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -162,6 +162,8 @@ public: int exportBuffers(BufferPool *pool); int importBuffers(BufferPool *pool); + int allocateBuffers(unsigned int count, std::vector *buffers); + int externalBuffers(unsigned int count); int releaseBuffers(); int queueBuffer(Buffer *buffer); @@ -197,6 +199,7 @@ private: int requestBuffers(unsigned int count); int createPlane(BufferMemory *buffer, unsigned int index, unsigned int plane, unsigned int length); + FrameBuffer *createBuffer(struct v4l2_buffer buf); int exportDmaBuffer(unsigned int index, unsigned int plane); Buffer *dequeueBuffer(); @@ -208,6 +211,7 @@ private: enum v4l2_memory memoryType_; BufferPool *bufferPool_; + V4L2BufferCache *cache_; std::map queuedBuffers_; EventNotifier *fdEvent_; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index c82f2829601bd14c..9fe66018ec502626 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -377,7 +377,8 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr) + : V4L2Device(deviceNode), bufferPool_(nullptr), cache_(nullptr), + fdEvent_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -1042,6 +1043,100 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool) return 0; } +/** + * \brief Operate using buffers allocated from local video device + * \param[in] count Number of buffers to allocate + * \param[out] buffers Vector to store local buffers + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::allocateBuffers(unsigned int count, std::vector *buffers) +{ + unsigned int i; + int ret; + + if (cache_) { + LOG(V4L2, Error) << "Can't allocate buffers when cache exists"; + return -EINVAL; + } + + memoryType_ = V4L2_MEMORY_MMAP; + + ret = requestBuffers(count); + if (ret < 0) + return ret; + + for (i = 0; i < count; ++i) { + struct v4l2_buffer buf = {}; + struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; + + buf.index = i; + buf.type = bufferType_; + buf.memory = memoryType_; + buf.length = VIDEO_MAX_PLANES; + buf.m.planes = planes; + + ret = ioctl(VIDIOC_QUERYBUF, &buf); + if (ret < 0) { + LOG(V4L2, Error) + << "Unable to query buffer " << i << ": " + << strerror(-ret); + goto err_buf; + } + + FrameBuffer *buffer = createBuffer(buf); + if (!buffer) { + LOG(V4L2, Error) << "Unable to create buffer"; + ret = -EINVAL; + goto err_buf; + } + + buffers->push_back(buffer); + } + + cache_ = new V4L2BufferCache(*buffers); + + return count; +err_buf: + requestBuffers(0); + + for (FrameBuffer *buffer : *buffers) + delete buffer; + + buffers->clear(); + + return ret; +} + +FrameBuffer *V4L2VideoDevice::createBuffer(struct v4l2_buffer buf) +{ + const unsigned int numPlanes = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ? buf.length : 1; + std::vector planes; + FrameBuffer *frame = nullptr; + + for (unsigned int nplane = 0; nplane < numPlanes; nplane++) { + int fd = exportDmaBuffer(buf.index, nplane); + if (fd < 0) + goto out; + + FrameBuffer::Plane plane; + plane.fd = fd; + plane.length = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ? + buf.m.planes[nplane].length : buf.length; + + planes.emplace_back(plane); + } + + if (!planes.empty()) + frame = new FrameBuffer(planes); + else + LOG(V4L2, Error) << "Failed to get planes"; +out: + for (FrameBuffer::Plane &plane : planes) + ::close(plane.fd); + + return frame; +} + int V4L2VideoDevice::exportDmaBuffer(unsigned int index, unsigned int plane) { struct v4l2_exportbuffer expbuf = {}; @@ -1062,6 +1157,35 @@ int V4L2VideoDevice::exportDmaBuffer(unsigned int index, unsigned int plane) return expbuf.fd; } +/** + * \brief Operate using buffers imported from external source + * \param[in] count Number of buffers to prepare for + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::externalBuffers(unsigned int count) +{ + /* + * We can only prepare the device to use external buffers when no + * internal buffers have been allocated. + */ + if (cache_) + return 0; + + int ret; + + memoryType_ = V4L2_MEMORY_DMABUF; + + ret = requestBuffers(count); + if (ret) + return ret; + + cache_ = new V4L2BufferCache(count); + + LOG(V4L2, Debug) << "provided for " << count << " external buffers"; + + return 0; +} + /** * \brief Release all internally allocated buffers */ From patchwork Tue Nov 26 23:36:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2368 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A10F61CC9 for ; Wed, 27 Nov 2019 00:39:40 +0100 (CET) X-Halon-ID: 02b0592e-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 02b0592e-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:38 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:10 +0100 Message-Id: <20191126233620.1695316-21-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 20/30] libcamera: stream: Add prototypes for new interface 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:40 -0000 The buffer allocation rework will remove most of the methods in the Stream class. This change adds the prototypes for the FrameBuffer interface with stub default implementations. Once the new buffer allocation work is completed the prototypes added in this change will be turned into pure virtual functions preventing the base Stream class from being instantiated. Signed-off-by: Niklas Söderlund --- include/libcamera/stream.h | 11 ++++++++ src/libcamera/stream.cpp | 55 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index a404eccf34d9c93b..395148e45d342d92 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_STREAM_H__ #define __LIBCAMERA_STREAM_H__ +#include #include #include #include @@ -74,6 +75,7 @@ class Stream { public: Stream(); + virtual ~Stream(){}; std::unique_ptr createBuffer(unsigned int index); std::unique_ptr createBuffer(const std::array &fds); @@ -86,6 +88,15 @@ public: protected: friend class Camera; + virtual int allocateBuffers(const StreamConfiguration &config, + std::vector *buffers) + { + return -EINVAL; + } + virtual void releaseBuffers() { return; } + virtual int start() { return -EINVAL; } + virtual void stop() { return; } + int mapBuffer(const Buffer *buffer); void unmapBuffer(const Buffer *buffer); diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index e70a1e307ecaa5ba..ba3f571b08cb0c41 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -520,6 +520,61 @@ std::unique_ptr Stream::createBuffer(const std::array &fds) * \return The memory type used by the stream */ +/** + * \fn Stream::allocateBuffers() + * \brief Allocate buffers from the stream + * \param[in] config A configuration describing the buffer(s) to be allocated + * \param[out] buffers Array of buffers successfully allocated + * + * Allocate buffers matching exactly what is described in \a config and return + * then in \a buffers. If buffers matching \a config can't be allocated an error + * shall be returned and no buffers returned in \a buffers. + * + * This is a helper which may be used by libcamera helper classes to allocate + * buffers from the stream itself. The allocated buffers may then be treated + * in the same way as if they where externally allocated. + * + * The only intended caller is buffer allocator helpers and the function must + * be implemeted by all subclasses of Stream. + * + * \return 0 on success or a negative error code otherwise + */ + +/** + * \fn Stream::releaseBuffers() + * \brief Relase buffers allocated from the stram + * + * This is a helper which release buffers allocated using allocateBuffers(). The + * only intended caller is buffer allocator helpers. + * + * The only intended caller is buffer allocator helpers and the function must + * be implemeted by all subclasses of Stream. + */ + +/** + * \fn Stream::start() + * \brief Prepare a stream for capture + * + * The subclss shall prepare the stream for capture and if needed allocate + * resources to allow for that. No buffers may be allocated from the stream + * using allocateBuffers() after a stream have been started. + * + * The only intended caller is the camera base class and the function must be + * implemeted by all subclasses of Stream. + * + * \return 0 on success or a negative error code otherwise + */ + +/** + * \fn Stream::stop() + * \brief Stop a stream after capture + * + * The subclass shall free all resources allocated in start(). + * + * The only intended caller is the camera base class and the function must be + * implemeted by all subclasses of Stream. + */ + /** * \brief Map a Buffer to a buffer memory index * \param[in] buffer The buffer to map to a buffer memory index From patchwork Tue Nov 26 23:36:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2369 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C0E260C21 for ; Wed, 27 Nov 2019 00:39:41 +0100 (CET) X-Halon-ID: 03200304-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 03200304-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:39 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:11 +0100 Message-Id: <20191126233620.1695316-22-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 21/30] libcamera: pipelines: Explicitly allocate streams 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:41 -0000 Prepare for sub-classing the Stream class with a V4L2 specific implementation which will need to be constructed later when knowledge about the V4L2 video device is available. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 64 +++++++++++++----------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++--- src/libcamera/pipeline/uvcvideo.cpp | 13 +++-- src/libcamera/pipeline/vimc.cpp | 14 ++++-- 4 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8ba08351c950f5e2..094c4db6bc32b683 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -138,8 +138,8 @@ public: class IPU3Stream : public Stream { public: - IPU3Stream() - : active_(false), device_(nullptr) + IPU3Stream(ImgUDevice::ImgUOutput *device, const std::string &name) + : active_(false), name_(name), device_(device) { } @@ -152,10 +152,16 @@ class IPU3CameraData : public CameraData { public: IPU3CameraData(PipelineHandler *pipe) - : CameraData(pipe) + : CameraData(pipe), outStream_(nullptr), vfStream_(nullptr) { } + ~IPU3CameraData() + { + delete outStream_; + delete vfStream_; + } + void imguOutputBufferReady(Buffer *buffer); void imguInputBufferReady(Buffer *buffer); void cio2BufferReady(Buffer *buffer); @@ -163,8 +169,8 @@ public: CIO2Device cio2_; ImgUDevice *imgu_; - IPU3Stream outStream_; - IPU3Stream vfStream_; + IPU3Stream *outStream_; + IPU3Stream *vfStream_; std::vector> rawBuffers_; }; @@ -343,8 +349,8 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() * stream otherwise. */ std::set availableStreams = { - &data_->outStream_, - &data_->vfStream_, + data_->outStream_, + data_->vfStream_, }; streams_.clear(); @@ -357,9 +363,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() const IPU3Stream *stream; if (cfg.size == sensorFormat_.size) - stream = &data_->outStream_; + stream = data_->outStream_; else - stream = &data_->vfStream_; + stream = data_->vfStream_; if (availableStreams.find(stream) == availableStreams.end()) stream = *availableStreams.begin(); @@ -367,7 +373,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() LOG(IPU3, Debug) << "Assigned '" << stream->name_ << "' to stream " << i; - bool scale = stream == &data_->vfStream_; + bool scale = stream == data_->vfStream_; adjustStream(config_[i], scale); if (cfg.pixelFormat != pixelFormat || cfg.size != size) { @@ -395,8 +401,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, IPU3CameraData *data = cameraData(camera); IPU3CameraConfiguration *config; std::set streams = { - &data->outStream_, - &data->vfStream_, + data->outStream_, + data->vfStream_, }; config = new IPU3CameraConfiguration(camera, data); @@ -414,10 +420,10 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * and VideoRecording roles are not allowed on * the output stream. */ - if (streams.find(&data->outStream_) != streams.end()) { - stream = &data->outStream_; - } else if (streams.find(&data->vfStream_) != streams.end()) { - stream = &data->vfStream_; + if (streams.find(data->outStream_) != streams.end()) { + stream = data->outStream_; + } else if (streams.find(data->vfStream_) != streams.end()) { + stream = data->vfStream_; } else { LOG(IPU3, Error) << "No stream available for requested role " @@ -447,14 +453,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * \todo This is an artificial limitation until we * figure out the exact capabilities of the hardware. */ - if (streams.find(&data->vfStream_) == streams.end()) { + if (streams.find(data->vfStream_) == streams.end()) { LOG(IPU3, Error) << "No stream available for requested role " << role; break; } - stream = &data->vfStream_; + stream = data->vfStream_; /* * Align the default viewfinder size to the maximum @@ -495,8 +501,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) IPU3CameraConfiguration *config = static_cast(c); IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = &data->outStream_; - IPU3Stream *vfStream = &data->vfStream_; + IPU3Stream *outStream = data->outStream_; + IPU3Stream *vfStream = data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; int ret; @@ -630,8 +636,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, const std::set &streams) { IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = &data->outStream_; - IPU3Stream *vfStream = &data->vfStream_; + IPU3Stream *outStream = data->outStream_; + IPU3Stream *vfStream = data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; @@ -855,10 +861,6 @@ int PipelineHandlerIPU3::registerCameras() for (unsigned int id = 0; id < 4 && numCameras < 2; ++id) { std::unique_ptr data = utils::make_unique(this); - std::set streams = { - &data->outStream_, - &data->vfStream_, - }; CIO2Device *cio2 = &data->cio2_; ret = cio2->init(cio2MediaDev_, id); @@ -872,10 +874,8 @@ int PipelineHandlerIPU3::registerCameras() * second. */ data->imgu_ = numCameras ? &imgu1_ : &imgu0_; - data->outStream_.device_ = &data->imgu_->output_; - data->outStream_.name_ = "output"; - data->vfStream_.device_ = &data->imgu_->viewfinder_; - data->vfStream_.name_ = "viewfinder"; + data->outStream_ = new IPU3Stream(&data->imgu_->output_, "output"); + data->vfStream_ = new IPU3Stream(&data->imgu_->viewfinder_, "viewfinder"); /* * Connect video devices' 'bufferReady' signals to their @@ -895,6 +895,10 @@ int PipelineHandlerIPU3::registerCameras() &IPU3CameraData::imguOutputBufferReady); /* Create and register the Camera instance. */ + std::set streams = { + data->outStream_, + data->vfStream_, + }; std::string cameraName = cio2->sensor_->entity()->name() + " " + std::to_string(id); std::shared_ptr camera = Camera::create(this, diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 6ad9b57d8353896c..1ae50bec0d401b3f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -117,19 +117,20 @@ class RkISP1CameraData : public CameraData { public: RkISP1CameraData(PipelineHandler *pipe) - : CameraData(pipe), sensor_(nullptr), frame_(0), - frameInfo_(pipe) + : CameraData(pipe), stream_(nullptr), sensor_(nullptr), + frame_(0), frameInfo_(pipe) { } ~RkISP1CameraData() { + delete stream_; delete sensor_; } int loadIPA(); - Stream stream_; + Stream *stream_; CameraSensor *sensor_; unsigned int frame_; std::vector ipaBuffers_; @@ -651,7 +652,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) if (ret) return ret; - cfg.setStream(&data->stream_); + cfg.setStream(data->stream_); return 0; } @@ -785,8 +786,8 @@ int PipelineHandlerRkISP1::start(Camera *camera) /* Inform IPA of stream configuration and sensor controls. */ std::map streamConfig; streamConfig[0] = { - .pixelFormat = data->stream_.configuration().pixelFormat, - .size = data->stream_.configuration().size, + .pixelFormat = data->stream_->configuration().pixelFormat, + .size = data->stream_->configuration().size, }; std::map entityControls; @@ -826,7 +827,7 @@ int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, Request *request) { RkISP1CameraData *data = cameraData(camera); - Stream *stream = &data->stream_; + Stream *stream = data->stream_; RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, stream); @@ -887,6 +888,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::unique_ptr data = utils::make_unique(this); + data->stream_ = new Stream(); + ControlInfoMap::Map ctrls; ctrls.emplace(std::piecewise_construct, std::forward_as_tuple(&controls::AeEnable), @@ -903,7 +906,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) if (ret) return ret; - std::set streams{ &data->stream_ }; + std::set streams{ data->stream_ }; std::shared_ptr camera = Camera::create(this, sensor->name(), streams); registerCamera(std::move(camera), std::move(data)); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3a76653ff6dc2b5e..64fc488912e5a82f 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -31,12 +31,13 @@ class UVCCameraData : public CameraData { public: UVCCameraData(PipelineHandler *pipe) - : CameraData(pipe), video_(nullptr) + : CameraData(pipe), video_(nullptr), stream_(nullptr) { } ~UVCCameraData() { + delete stream_; delete video_; } @@ -44,7 +45,7 @@ public: void bufferReady(Buffer *buffer); V4L2VideoDevice *video_; - Stream stream_; + Stream *stream_; }; class UVCCameraConfiguration : public CameraConfiguration @@ -187,7 +188,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) format.fourcc != data->video_->toV4L2Fourcc(cfg.pixelFormat)) return -EINVAL; - cfg.setStream(&data->stream_); + cfg.setStream(data->stream_); return 0; } @@ -265,7 +266,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(&data->stream_); + Buffer *buffer = request->findBuffer(data->stream_); if (!buffer) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; @@ -310,7 +311,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) } /* Create and register the camera. */ - std::set streams{ &data->stream_ }; + std::set streams{ data->stream_ }; std::shared_ptr camera = Camera::create(this, media->model(), streams); registerCamera(std::move(camera), std::move(data)); @@ -330,6 +331,8 @@ int UVCCameraData::init(MediaEntity *entity) if (ret) return ret; + stream_ = new Stream(); + video_->bufferReady.connect(this, &UVCCameraData::bufferReady); /* Initialise the supported controls. */ diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index f5550a1723668106..3f9e92163642f0c2 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -41,7 +41,8 @@ class VimcCameraData : public CameraData public: VimcCameraData(PipelineHandler *pipe) : CameraData(pipe), sensor_(nullptr), debayer_(nullptr), - scaler_(nullptr), video_(nullptr), raw_(nullptr) + scaler_(nullptr), video_(nullptr), raw_(nullptr), + stream_(nullptr) { } @@ -52,6 +53,7 @@ public: delete scaler_; delete video_; delete raw_; + delete stream_; } int init(MediaDevice *media); @@ -62,7 +64,7 @@ public: V4L2Subdevice *scaler_; V4L2VideoDevice *video_; V4L2VideoDevice *raw_; - Stream stream_; + Stream *stream_; }; class VimcCameraConfiguration : public CameraConfiguration @@ -254,7 +256,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) if (ret) return ret; - cfg.setStream(&data->stream_); + cfg.setStream(data->stream_); return 0; } @@ -326,7 +328,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(&data->stream_); + Buffer *buffer = request->findBuffer(data->stream_); if (!buffer) { LOG(VIMC, Error) << "Attempt to queue request with invalid stream"; @@ -376,7 +378,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) return false; /* Create and register the camera. */ - std::set streams{ &data->stream_ }; + std::set streams{ data->stream_ }; std::shared_ptr camera = Camera::create(this, "VIMC Sensor B", streams); registerCamera(std::move(camera), std::move(data)); @@ -418,6 +420,8 @@ int VimcCameraData::init(MediaDevice *media) if (video_->open()) return -ENODEV; + stream_ = new Stream(); + video_->bufferReady.connect(this, &VimcCameraData::bufferReady); raw_ = new V4L2VideoDevice(media->getEntityByName("Raw Capture 1")); From patchwork Tue Nov 26 23:36:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2370 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B2F361C65 for ; Wed, 27 Nov 2019 00:39:43 +0100 (CET) X-Halon-ID: 03c6c4dd-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 03c6c4dd-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:41 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:12 +0100 Message-Id: <20191126233620.1695316-23-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 22/30] libcamera: v4l2_videodevice: Add V4L2Stream to facilitate buffers 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:43 -0000 In the FrameBuffer interface the streams and not the pipelines abstracts the knowledge about how and if buffers needs to be allocated or prepared before a capture session is started. The rational behind this model is the V4L2 API where if applications needs to allocate memory for buffers it do that using a video node and not using a external allocator. As the video node needs to be prepared in different ways depending on if it have been used for memory allocation or not each streams facing applications needs not only be available to be used to allocated buffers but track if they have been so in order to be started correctly. This change implements the interface for cameras backed by V4L2 video devices to function with the FrameBuffer interface. Signed-off-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 24 +++++++ src/libcamera/v4l2_videodevice.cpp | 90 ++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index f4cbcfbd26ea540c..0e94c3f71d0b1208 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -16,6 +16,7 @@ #include #include #include +#include #include "formats.h" #include "log.h" @@ -236,6 +237,29 @@ private: V4L2VideoDevice *capture_; }; +class V4L2Stream : public Stream +{ +public: + V4L2Stream(V4L2VideoDevice *video); + +protected: + int allocateBuffers(const StreamConfiguration &config, + std::vector *buffers) override; + void releaseBuffers() override; + int start() override; + void stop() override; + +private: + enum BufferType { + NoBuffers, + AllocatedBuffers, + ExternalBuffers, + }; + + V4L2VideoDevice *video_; + BufferType bufferType_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 9fe66018ec502626..f5810956b2040ce6 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1711,4 +1711,94 @@ void V4L2M2MDevice::close() output_->close(); } +/** + * \class V4L2Stream + * \brief V4L2 Video stream for a camera + * + * The V4l2Stream implemets the interfaces mandated by Stream to allow buffers + * to be allocoated from a video device and presented to the user as if they + * where allocated externaly. + */ + +/** + * \brief Create a stream around a V4L2 video device + * \param[in] video V4L2 video device to serve + * + * Create a stream which will manage the application visible buffers for a V4L2 + * video device. + */ +V4L2Stream::V4L2Stream(V4L2VideoDevice *video) + : Stream(), video_(video), bufferType_(NoBuffers) +{ +} + +int V4L2Stream::allocateBuffers(const StreamConfiguration &config, + std::vector *buffers) +{ + if (bufferType_ != NoBuffers) + return -EINVAL; + + if (!buffers) + return -EINVAL; + + /* + * \todo: Extend V4L2VideoDevice to support VIDIOC_CREATE_BUFS which + * can take the whole configuration into account, until then use as + * much as possible (number of buffers) with VIDIOC_QUERYBUF. + * + * For this reason check that the format of the video device is the + * same as the one request or fail. This will allow for a applicaiton + * facing API that will work with VIDIOC_CREATE_BUFS. + */ + V4L2DeviceFormat format; + int ret = video_->getFormat(&format); + if (ret) + return ret; + + if (config.size != format.size || + config.pixelFormat != V4L2VideoDevice::toPixelFormat(format.fourcc)) + return -EINVAL; + + ret = video_->allocateBuffers(config.bufferCount, buffers); + if (ret) + return ret; + + bufferType_ = AllocatedBuffers; + + return ret; +} + +void V4L2Stream::releaseBuffers() +{ + if (bufferType_ == NoBuffers) + return; + + video_->releaseBuffers(); + bufferType_ = NoBuffers; +} + +int V4L2Stream::start() +{ + /* If internal buffers is already allocated nothing to do. */ + if (bufferType_ == AllocatedBuffers) + return 0; + + int ret = video_->externalBuffers(configuration_.bufferCount); + if (ret) + return ret; + + bufferType_ = ExternalBuffers; + + return ret; +} + +void V4L2Stream::stop() +{ + /* Only need to clean up if external buffers is used. */ + if (bufferType_ != ExternalBuffers) + return; + + releaseBuffers(); +} + } /* namespace libcamera */ From patchwork Tue Nov 26 23:36:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2371 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 22C7961C58 for ; Wed, 27 Nov 2019 00:39:44 +0100 (CET) X-Halon-ID: 050961bd-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 050961bd-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:42 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:13 +0100 Message-Id: <20191126233620.1695316-24-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 23/30] libcamera: pipelines: Switch to V4L2Stream 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:44 -0000 Prepare to switch to the FrameBuffer interface by using V4L2Stream. This does not break current operations as changes in applications are needed to switch interface. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 094c4db6bc32b683..5080ac6c4d0abe3b 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -135,11 +135,12 @@ public: BufferPool pool_; }; -class IPU3Stream : public Stream +class IPU3Stream : public V4L2Stream { public: IPU3Stream(ImgUDevice::ImgUOutput *device, const std::string &name) - : active_(false), name_(name), device_(device) + : V4L2Stream(device->dev), active_(false), name_(name), + device_(device) { } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 1ae50bec0d401b3f..ca3d92c7ad637c3a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -888,7 +888,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::unique_ptr data = utils::make_unique(this); - data->stream_ = new Stream(); + data->stream_ = new V4L2Stream(video_); ControlInfoMap::Map ctrls; ctrls.emplace(std::piecewise_construct, diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 64fc488912e5a82f..9cc90bf454cb4392 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -331,7 +331,7 @@ int UVCCameraData::init(MediaEntity *entity) if (ret) return ret; - stream_ = new Stream(); + stream_ = new V4L2Stream(video_); video_->bufferReady.connect(this, &UVCCameraData::bufferReady); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 3f9e92163642f0c2..b1222bd21fb629a0 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -420,7 +420,7 @@ int VimcCameraData::init(MediaDevice *media) if (video_->open()) return -ENODEV; - stream_ = new Stream(); + stream_ = new V4L2Stream(video_); video_->bufferReady.connect(this, &VimcCameraData::bufferReady); From patchwork Tue Nov 26 23:36:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2372 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A1ABB61C59 for ; Wed, 27 Nov 2019 00:39:44 +0100 (CET) X-Halon-ID: 0564c944-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 0564c944-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:43 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:14 +0100 Message-Id: <20191126233620.1695316-25-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 24/30] libcamera: stream: Make FrameBuffer support mandatory 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:44 -0000 All pipelines now uses V4L2Streams, make using a sub-class of Stream which implements the FrameBuffer interface helpers mandatory. Remove the fallback implementations in the Stream base class to prevent the base class to be instantiated directly. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/stream.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 395148e45d342d92..a3c692c347340382 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -7,7 +7,6 @@ #ifndef __LIBCAMERA_STREAM_H__ #define __LIBCAMERA_STREAM_H__ -#include #include #include #include @@ -89,13 +88,10 @@ protected: friend class Camera; virtual int allocateBuffers(const StreamConfiguration &config, - std::vector *buffers) - { - return -EINVAL; - } - virtual void releaseBuffers() { return; } - virtual int start() { return -EINVAL; } - virtual void stop() { return; } + std::vector *buffers) = 0; + virtual void releaseBuffers() = 0; + virtual int start() = 0; + virtual void stop() = 0; int mapBuffer(const Buffer *buffer); void unmapBuffer(const Buffer *buffer); From patchwork Tue Nov 26 23:36:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2373 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 607CF61C59 for ; Wed, 27 Nov 2019 00:39:45 +0100 (CET) X-Halon-ID: 05b440e8-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 05b440e8-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:43 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:15 +0100 Message-Id: <20191126233620.1695316-26-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 25/30] libcamera: allocator: Add BufferAllocator to help applications allocate buffers 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:45 -0000 The FrameBuffer interface is based on the idea that all buffers are allocated outside libcamera and are only used by it. This is done to make the API simpler and so that both internal and external buffers are handled in the same way. As V4L2 do not provide a way to allocate buffers without the help of a video device add an application facing helper which hide this. This allow applications to allocate buffers and then use them with cameras. Signed-off-by: Niklas Söderlund --- include/libcamera/allocator.h | 39 ++++++++++ include/libcamera/meson.build | 1 + include/libcamera/stream.h | 1 + src/libcamera/allocator.cpp | 141 ++++++++++++++++++++++++++++++++++ src/libcamera/meson.build | 1 + 5 files changed, 183 insertions(+) create mode 100644 include/libcamera/allocator.h create mode 100644 src/libcamera/allocator.cpp diff --git a/include/libcamera/allocator.h b/include/libcamera/allocator.h new file mode 100644 index 0000000000000000..36fce38491b5f3ef --- /dev/null +++ b/include/libcamera/allocator.h @@ -0,0 +1,39 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * allocator.h - Buffer allocator + */ +#ifndef __LIBCAMERA_ALLOCATOR_H__ +#define __LIBCAMERA_ALLOCATOR_H__ + +#include +#include +#include + +namespace libcamera { + +class Camera; +class FrameBuffer; +class Stream; +struct StreamConfiguration; + +class BufferAllocator +{ +public: + BufferAllocator(std::shared_ptr camera); + ~BufferAllocator(); + + int allocate(Stream *stream, const StreamConfiguration &config); + void release(Stream *stream); + + const std::vector &buffers(Stream *stream) const; + +private: + std::shared_ptr camera_; + std::map> buffers_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_ALLOCATOR_H__ */ diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 99abf06099407c1f..0d0ba2248fd8a679 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -1,4 +1,5 @@ libcamera_api = files([ + 'allocator.h', 'bound_method.h', 'buffer.h', 'camera.h', diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index a3c692c347340382..8e653408fd54cf74 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -85,6 +85,7 @@ public: MemoryType memoryType() const { return memoryType_; } protected: + friend class BufferAllocator; /* To allocate and release buffers. */ friend class Camera; virtual int allocateBuffers(const StreamConfiguration &config, diff --git a/src/libcamera/allocator.cpp b/src/libcamera/allocator.cpp new file mode 100644 index 0000000000000000..8b517c809c05cbcd --- /dev/null +++ b/src/libcamera/allocator.cpp @@ -0,0 +1,141 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * allocator.cpp - Buffer allocator + */ + +#include + +#include +#include +#include +#include + +#include +#include + +#include "log.h" + +/** + * \file allocator.h + * \brief Buffer allocator + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(Allocator) + +/** + * \class BufferAllocator + * \brief Buffer allocator for applications + * + * All buffers are to be treated as if they where allocated outside of the + * camera. In some situations however the only source applications can allocate + * buffers from is the camera. The BufferAllocator is the interface applications + * shall use in these situations. + * + * The buffer allocator creates buffers using resources from a camera and/or + * a stream. The buffers allocated this way are owned by the application and it + * is responsible for their lifetime management. But just as buffers allocated + * external from cameras and streams it's not valid to free a buffer while it's + * queue to a camera. + * + * All buffers allocator are automatically freed when the allocator object is + * deleted. It is the applications responsibility to make sure this do not + * happen while one or more of the buffers are queued to a camera. + * + * If buffers are allocated outside the scope of libcamera by the application it + * do not need to interact with the buffer allocator. + */ + +/** + * \brief Create a BufferAllocator serving a camera + * \param[in] camera Camera the allocator shall serve + */ + +BufferAllocator::BufferAllocator(std::shared_ptr camera) + : camera_(camera) +{ +} + +BufferAllocator::~BufferAllocator() +{ + for (auto &value : buffers_) { + Stream *stream = value.first; + std::vector &buffers = value.second; + + for (FrameBuffer *buffer : buffers) + delete buffer; + + buffers.clear(); + + stream->releaseBuffers(); + } + + buffers_.clear(); +} + +/** + * \brief Allocate buffers from a stream + * \param[in] stream The stream to allocate buffers from + * \param[in] config The configuration described the buffers to be allocated + * + * Allocate buffers matching exactly what is described in \a config. If buffers + * matching \a config can't be allocated an error is returned and no buffers are + * allocated. + * + * \return 0 on success or a negative error code otherwise + */ +int BufferAllocator::allocate(Stream *stream, const StreamConfiguration &config) +{ + auto iter = camera_->streams().find(stream); + if (iter != camera_->streams().end()) + return stream->allocateBuffers(config, &buffers_[stream]); + + LOG(Allocator, Error) << "Stream do not belong to " << camera_->name(); + return -EINVAL; +} + +/** + * \brief Free allocated buffers + * \param[in] stream The stream to free buffers for + * + * Free buffers allocated with allocate(). + */ +void BufferAllocator::release(Stream *stream) +{ + auto iter = buffers_.find(stream); + if (iter == buffers_.end()) + return; + + std::vector &buffers = iter->second; + + for (FrameBuffer *buffer : buffers) + delete buffer; + + buffers.clear(); + + stream->releaseBuffers(); + + buffers_.erase(iter); +} + +/** + * \brief Retrive array of allocated buffers + * \param[in] stream The stream to retrive buffers for + * + * \return Array of buffers + */ +const std::vector &BufferAllocator::buffers(Stream *stream) const +{ + static std::vector empty; + + auto iter = buffers_.find(stream); + if (iter == buffers_.end()) + return empty; + + return iter->second; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index c4f965bd7413b37e..b7041e003fdb1d49 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -1,4 +1,5 @@ libcamera_sources = files([ + 'allocator.cpp', 'bound_method.cpp', 'buffer.cpp', 'byte_stream_buffer.cpp', From patchwork Tue Nov 26 23:36:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2374 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DC49061C91 for ; Wed, 27 Nov 2019 00:39:45 +0100 (CET) X-Halon-ID: 06248b26-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 06248b26-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:44 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:16 +0100 Message-Id: <20191126233620.1695316-27-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 26/30] libcamera: camera: Start streams before pipeline 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:46 -0000 The FrameBuffer interface require individual streams to be started explicitly. This is needed so that the streams may perform any preparations before the pipeline and this the camera is started. The only supported stream at the moment is V4L2 streams and they need to be started so they can prepare for the use of external buffers in the case they are used. Signed-off-by: Niklas Söderlund --- src/libcamera/camera.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index e810fb725d81350d..6b1b3fb64f8b2c0b 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -856,15 +856,23 @@ int Camera::queueRequest(Request *request) */ int Camera::start() { + int ret; + if (disconnected_) return -ENODEV; if (!stateIs(CameraPrepared)) return -EACCES; + for (Stream *stream : activeStreams_) { + ret = stream->start(); + if (ret < 0) + return ret; + } + LOG(Camera, Debug) << "Starting capture"; - int ret = pipe_->start(this); + ret = pipe_->start(this); if (ret) return ret; @@ -899,6 +907,9 @@ int Camera::stop() pipe_->stop(this); + for (Stream *stream : activeStreams_) + stream->stop(); + return 0; } From patchwork Tue Nov 26 23:36:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2375 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F66260C3D for ; Wed, 27 Nov 2019 00:39:49 +0100 (CET) X-Halon-ID: 0676cd8e-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 0676cd8e-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:44 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:17 +0100 Message-Id: <20191126233620.1695316-28-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 27/30] libcamera: Switch to FrameBuffer interface 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:49 -0000 Switch to the FrameBuffer interface where all buffers are treated as external buffers and are allocated outside the camera. Applications allocating buffers using libcamera are switched to use the BufferAllocator helper. A follow up changes to this one will finalize the transition to the new FrameBuffer interface by removing code that is left unused after this change. Signed-off-by: Niklas Söderlund --- include/libcamera/camera.h | 4 +- include/libcamera/request.h | 14 +- src/android/camera_device.cpp | 19 +- src/cam/buffer_writer.cpp | 9 +- src/cam/buffer_writer.h | 3 +- src/cam/capture.cpp | 38 +- src/cam/capture.h | 4 +- src/libcamera/camera.cpp | 34 -- src/libcamera/include/pipeline_handler.h | 5 +- src/libcamera/include/v4l2_videodevice.h | 9 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 223 +++------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 120 +++-- src/libcamera/pipeline/uvcvideo.cpp | 20 +- src/libcamera/pipeline/vimc.cpp | 20 +- src/libcamera/pipeline_handler.cpp | 2 +- src/libcamera/request.cpp | 16 +- src/libcamera/v4l2_videodevice.cpp | 80 +--- src/qcam/main_window.cpp | 47 +- src/qcam/main_window.h | 6 +- test/camera/buffer_import.cpp | 415 +++++------------- test/camera/capture.cpp | 31 +- test/camera/statemachine.cpp | 12 +- test/v4l2_videodevice/buffer_sharing.cpp | 23 +- test/v4l2_videodevice/capture_async.cpp | 14 +- test/v4l2_videodevice/request_buffers.cpp | 11 +- test/v4l2_videodevice/stream_on_off.cpp | 6 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 40 +- test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- 28 files changed, 415 insertions(+), 812 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index ef6a37bb142c83a6..02d047b9649f53d0 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -19,7 +19,7 @@ namespace libcamera { -class Buffer; +class FrameBuffer; class PipelineHandler; class Request; @@ -77,7 +77,7 @@ public: const std::string &name() const; - Signal bufferCompleted; + Signal bufferCompleted; Signal requestCompleted; Signal disconnected; diff --git a/include/libcamera/request.h b/include/libcamera/request.h index a8708010ec1247a2..02c0ef5f6b028bd4 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -17,9 +17,9 @@ namespace libcamera { -class Buffer; class Camera; class CameraControlValidator; +class FrameBuffer; class Stream; class Request @@ -38,9 +38,9 @@ public: ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } - const std::map &buffers() const { return bufferMap_; } - int addBuffer(Stream *stream, std::unique_ptr buffer); - Buffer *findBuffer(Stream *stream) const; + const std::map &buffers() const { return bufferMap_; } + int addBuffer(Stream *stream, FrameBuffer *buffer); + FrameBuffer *findBuffer(Stream *stream) const; uint64_t cookie() const { return cookie_; } Status status() const { return status_; } @@ -54,14 +54,14 @@ private: int prepare(); void complete(); - bool completeBuffer(Buffer *buffer); + bool completeBuffer(FrameBuffer *buffer); Camera *camera_; CameraControlValidator *validator_; ControlList *controls_; ControlList *metadata_; - std::map bufferMap_; - std::unordered_set pending_; + std::map bufferMap_; + std::unordered_set pending_; const uint64_t cookie_; Status status_; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 55b29a9a41ab8943..708cf95b0c3c25c2 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -739,13 +739,13 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque * and (currently) only supported request buffer. */ const buffer_handle_t camera3Handle = *camera3Buffers[0].buffer; - std::array fds = { - camera3Handle->data[0], - camera3Handle->data[1], - camera3Handle->data[2], - }; - std::unique_ptr buffer = stream->createBuffer(fds); + std::vector planes; + planes.push_back({ .fd = camera3Handle->data[0], .length = 0 }); + planes.push_back({ .fd = camera3Handle->data[1], .length = 0 }); + planes.push_back({ .fd = camera3Handle->data[2], .length = 0 }); + + FrameBuffer *buffer = new FrameBuffer(planes); if (!buffer) { LOG(HAL, Error) << "Failed to create buffer"; delete descriptor; @@ -754,7 +754,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque Request *request = camera_->createRequest(reinterpret_cast(descriptor)); - request->addBuffer(stream, std::move(buffer)); + request->addBuffer(stream, buffer); int ret = camera_->queueRequest(request); if (ret) { @@ -771,8 +771,8 @@ error: void CameraDevice::requestComplete(Request *request) { - const std::map &buffers = request->buffers(); - Buffer *libcameraBuffer = buffers.begin()->second; + const std::map &buffers = request->buffers(); + FrameBuffer *libcameraBuffer = buffers.begin()->second; camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; std::unique_ptr resultMetadata; @@ -825,6 +825,7 @@ void CameraDevice::requestComplete(Request *request) callbacks_->process_capture_result(callbacks_, &captureResult); delete descriptor; + delete libcameraBuffer; } void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp) diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index b6b40baeee661df6..5cead75a16d0923a 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -21,7 +21,7 @@ BufferWriter::BufferWriter(const std::string &pattern) { } -int BufferWriter::write(Buffer *buffer, const std::string &streamName) +int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) { std::string filename; size_t pos; @@ -42,10 +42,9 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) if (fd == -1) return -errno; - BufferMemory *mem = buffer->mem(); - for (Dmabuf &dmabuf : mem->planes()) { - void *data = dmabuf.mem(); - unsigned int length = dmabuf.length(); + for (Dmabuf *dmabuf : buffer->dmabufs()) { + void *data = dmabuf->mem(); + unsigned int length = dmabuf->length(); ret = ::write(fd, data, length); if (ret < 0) { diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 7bf785d1e83235ff..5917a7dfb5e28106 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -16,7 +16,8 @@ class BufferWriter public: BufferWriter(const std::string &pattern = "frame-#.bin"); - int write(libcamera::Buffer *buffer, const std::string &streamName); + int write(libcamera::FrameBuffer *buffer, + const std::string &streamName); private: std::string pattern_; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index a4fa88a8d99669bc..154e6e461ee2b96b 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -57,7 +57,10 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) writer_ = new BufferWriter(); } - ret = capture(loop); + + BufferAllocator allocator(camera_); + + ret = capture(loop, allocator); if (options.isSet(OptFile)) { delete writer_; @@ -69,14 +72,22 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) return ret; } -int Capture::capture(EventLoop *loop) +int Capture::capture(EventLoop *loop, BufferAllocator &allocator) { int ret; /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (StreamConfiguration &cfg : *config_) - nbuffers = std::min(nbuffers, cfg.bufferCount); + for (StreamConfiguration &cfg : *config_) { + ret = allocator.allocate(cfg.stream(), cfg); + 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); + } /* * TODO: make cam tool smarter to support still capture by for @@ -93,9 +104,9 @@ int Capture::capture(EventLoop *loop) for (StreamConfiguration &cfg : *config_) { Stream *stream = cfg.stream(); - std::unique_ptr buffer = stream->createBuffer(i); + FrameBuffer *buffer = allocator.buffers(stream)[i]; - ret = request->addBuffer(stream, std::move(buffer)); + ret = request->addBuffer(stream, buffer); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; @@ -138,7 +149,7 @@ void Capture::requestComplete(Request *request) if (request->status() == Request::RequestCancelled) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); double fps = std::chrono::duration_cast(now - last_).count(); @@ -151,7 +162,7 @@ void Capture::requestComplete(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; - Buffer *buffer = it->second; + FrameBuffer *buffer = it->second; const std::string &name = streamName_[stream]; info << " " << name @@ -180,16 +191,9 @@ void Capture::requestComplete(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; - Buffer *buffer = it->second; - unsigned int index = buffer->index(); - - std::unique_ptr newBuffer = stream->createBuffer(index); - if (!newBuffer) { - std::cerr << "Can't create buffer" << std::endl; - return; - } + FrameBuffer *buffer = it->second; - request->addBuffer(stream, std::move(newBuffer)); + request->addBuffer(stream, buffer); } camera_->queueRequest(request); diff --git a/src/cam/capture.h b/src/cam/capture.h index c692d48918f2de1d..3a8842fbd2080c2c 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -10,6 +10,8 @@ #include #include +#include +#include #include #include #include @@ -26,7 +28,7 @@ public: int run(EventLoop *loop, const OptionsParser::Options &options); private: - int capture(EventLoop *loop); + int capture(EventLoop *loop, libcamera::BufferAllocator &allocator); void requestComplete(libcamera::Request *request); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 6b1b3fb64f8b2c0b..05fdcaab8f918afa 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -678,12 +678,6 @@ int Camera::configure(CameraConfiguration *config) stream->configuration_ = cfg; activeStreams_.insert(stream); - - /* - * Allocate buffer objects in the pool. - * Memory will be allocated and assigned later. - */ - stream->createBuffers(cfg.memoryType, cfg.bufferCount); } state_ = CameraConfigured; @@ -739,14 +733,6 @@ int Camera::freeBuffers() if (!stateIs(CameraPrepared)) return -EACCES; - for (Stream *stream : activeStreams_) { - /* - * All mappings must be destroyed before buffers can be freed - * by the V4L2 device that has allocated them. - */ - stream->destroyBuffers(); - } - state_ = CameraConfigured; return pipe_->freeBuffers(this, activeStreams_); @@ -812,24 +798,11 @@ int Camera::queueRequest(Request *request) for (auto const &it : request->buffers()) { Stream *stream = it.first; - Buffer *buffer = it.second; if (activeStreams_.find(stream) == activeStreams_.end()) { LOG(Camera, Error) << "Invalid request"; return -EINVAL; } - - if (stream->memoryType() == ExternalMemory) { - int index = stream->mapBuffer(buffer); - if (index < 0) { - LOG(Camera, Error) << "No buffer memory available"; - return -ENOMEM; - } - - buffer->index_ = index; - } - - buffer->mem_ = &stream->buffers()[buffer->index_]; } int ret = request->prepare(); @@ -923,13 +896,6 @@ int Camera::stop() */ void Camera::requestComplete(Request *request) { - for (auto it : request->buffers()) { - Stream *stream = it.first; - Buffer *buffer = it.second; - if (stream->memoryType() == ExternalMemory) - stream->unmapBuffer(buffer); - } - requestCompleted.emit(request); delete request; } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 6ce8ea1c8d31b870..e4588f10bac0228c 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -20,12 +20,12 @@ namespace libcamera { -class Buffer; class Camera; class CameraConfiguration; class CameraManager; class DeviceEnumerator; class DeviceMatch; +class FrameBuffer; class MediaDevice; class PipelineHandler; class Request; @@ -79,7 +79,8 @@ public: int queueRequest(Camera *camera, Request *request); - bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); + bool completeBuffer(Camera *camera, Request *request, + FrameBuffer *buffer); void completeRequest(Camera *camera, Request *request); const char *name() const { return name_; } diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 0e94c3f71d0b1208..ced3cb229c509ad2 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -167,9 +167,8 @@ public: int externalBuffers(unsigned int count); int releaseBuffers(); - int queueBuffer(Buffer *buffer); - std::vector> queueAllBuffers(); - Signal bufferReady; + int queueBuffer(FrameBuffer *buffer); + Signal bufferReady; int streamOn(); int streamOff(); @@ -203,7 +202,7 @@ private: FrameBuffer *createBuffer(struct v4l2_buffer buf); int exportDmaBuffer(unsigned int index, unsigned int plane); - Buffer *dequeueBuffer(); + FrameBuffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); V4L2Capability caps_; @@ -213,7 +212,7 @@ private: BufferPool *bufferPool_; V4L2BufferCache *cache_; - std::map queuedBuffers_; + std::map queuedBuffers_; EventNotifier *fdEvent_; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 5080ac6c4d0abe3b..8d9420aea3eb0b21 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -31,6 +31,8 @@ namespace libcamera { LOG_DEFINE_CATEGORY(IPU3) +class IPU3CameraData; + class ImgUDevice { public: @@ -44,7 +46,7 @@ public: V4L2VideoDevice *dev; unsigned int pad; std::string name; - BufferPool *pool; + std::vector buffers; }; ImgUDevice() @@ -70,10 +72,9 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); - int importInputBuffers(BufferPool *pool); - int importOutputBuffers(ImgUOutput *output, BufferPool *pool); - int exportOutputBuffers(ImgUOutput *output, BufferPool *pool); - void freeBuffers(); + int importInputBuffers(unsigned int count); + int exportOutputBuffers(ImgUOutput *output, unsigned int count); + void freeBuffers(IPU3CameraData *data); int start(); int stop(); @@ -93,10 +94,6 @@ public: ImgUOutput viewfinder_; ImgUOutput stat_; /* \todo Add param video device for 3A tuning */ - - BufferPool vfPool_; - BufferPool statPool_; - BufferPool outPool_; }; class CIO2Device @@ -120,10 +117,10 @@ public: int configure(const Size &size, V4L2DeviceFormat *outputFormat); - BufferPool *exportBuffers(); + int exportBuffers(); void freeBuffers(); - int start(std::vector> *buffers); + int start(); int stop(); static int mediaBusToFormat(unsigned int code); @@ -132,7 +129,8 @@ public: V4L2Subdevice *csi2_; CameraSensor *sensor_; - BufferPool pool_; +private: + std::vector buffers_; }; class IPU3Stream : public V4L2Stream @@ -163,17 +161,15 @@ public: delete vfStream_; } - void imguOutputBufferReady(Buffer *buffer); - void imguInputBufferReady(Buffer *buffer); - void cio2BufferReady(Buffer *buffer); + void imguOutputBufferReady(FrameBuffer *buffer); + void imguInputBufferReady(FrameBuffer *buffer); + void cio2BufferReady(FrameBuffer *buffer); CIO2Device cio2_; ImgUDevice *imgu_; IPU3Stream *outStream_; IPU3Stream *vfStream_; - - std::vector> rawBuffers_; }; class IPU3CameraConfiguration : public CameraConfiguration @@ -637,70 +633,42 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, const std::set &streams) { IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = data->outStream_; - IPU3Stream *vfStream = data->vfStream_; - CIO2Device *cio2 = &data->cio2_; - ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; int ret; - /* Share buffers between CIO2 output and ImgU input. */ - BufferPool *pool = cio2->exportBuffers(); - if (!pool) - return -ENOMEM; + ret = data->cio2_.exportBuffers(); + if (ret < 0) + return ret; - ret = imgu->importInputBuffers(pool); - if (ret) - goto error; + bufferCount = ret; - /* - * Use for the stat's internal pool the same number of buffer as - * for the input pool. - * \todo To be revised when we'll actually use the stat node. - */ - bufferCount = pool->count(); - imgu->stat_.pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(&imgu->stat_, imgu->stat_.pool); + ret = data->imgu_->importInputBuffers(bufferCount); if (ret) goto error; - /* Allocate buffers for each active stream. */ - for (Stream *s : streams) { - IPU3Stream *stream = static_cast(s); - ImgUDevice::ImgUOutput *dev = stream->device_; - - if (stream->memoryType() == InternalMemory) - ret = imgu->exportOutputBuffers(dev, &stream->bufferPool()); - else - ret = imgu->importOutputBuffers(dev, &stream->bufferPool()); - if (ret) - goto error; - } + ret = data->imgu_->exportOutputBuffers(&data->imgu_->stat_, bufferCount); + if (ret < 0) + goto error; /* * Allocate buffers also on non-active outputs; use the same number * of buffers as the active ones. */ - if (!outStream->active_) { - bufferCount = vfStream->configuration().bufferCount; - outStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(outStream->device_, - outStream->device_->pool); - if (ret) + if (!data->outStream_->active_) { + ret = data->imgu_->exportOutputBuffers(data->outStream_->device_, + bufferCount); + if (ret < 0) goto error; } - if (!vfStream->active_) { - bufferCount = outStream->configuration().bufferCount; - vfStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(vfStream->device_, - vfStream->device_->pool); - if (ret) + if (!data->vfStream_->active_) { + ret = data->imgu_->exportOutputBuffers(data->vfStream_->device_, + bufferCount); + if (ret < 0) goto error; } return 0; - error: freeBuffers(camera, streams); @@ -713,7 +681,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, IPU3CameraData *data = cameraData(camera); data->cio2_.freeBuffers(); - data->imgu_->freeBuffers(); + data->imgu_->freeBuffers(data); return 0; } @@ -729,7 +697,7 @@ int PipelineHandlerIPU3::start(Camera *camera) * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. */ - ret = cio2->start(&data->rawBuffers_); + ret = cio2->start(); if (ret) goto error; @@ -745,7 +713,6 @@ int PipelineHandlerIPU3::start(Camera *camera) error: LOG(IPU3, Error) << "Failed to start camera " << camera->name(); - data->rawBuffers_.clear(); return ret; } @@ -759,8 +726,6 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); - - data->rawBuffers_.clear(); } int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) @@ -769,7 +734,7 @@ int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) for (auto it : request->buffers()) { IPU3Stream *stream = static_cast(it.first); - Buffer *buffer = it.second; + FrameBuffer *buffer = it.second; int ret = stream->device_->dev->queueBuffer(buffer); if (ret < 0) @@ -930,7 +895,7 @@ int PipelineHandlerIPU3::registerCameras() * Buffers completed from the ImgU input are immediately queued back to the * CIO2 unit to continue frame capture. */ -void IPU3CameraData::imguInputBufferReady(Buffer *buffer) +void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ if (buffer->info().status() == BufferInfo::BufferCancelled) @@ -945,7 +910,7 @@ void IPU3CameraData::imguInputBufferReady(Buffer *buffer) * * Buffers completed from the ImgU output are directed to the application. */ -void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) +void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); @@ -964,7 +929,7 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) * Buffers completed from the CIO2 are immediately queued to the ImgU unit * for further processing. */ -void IPU3CameraData::cio2BufferReady(Buffer *buffer) +void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ if (buffer->info().status() == BufferInfo::BufferCancelled) @@ -1020,7 +985,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) output_.pad = PAD_OUTPUT; output_.name = "output"; - output_.pool = &outPool_; viewfinder_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " viewfinder"); @@ -1030,7 +994,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) viewfinder_.pad = PAD_VF; viewfinder_.name = "viewfinder"; - viewfinder_.pool = &vfPool_; stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); ret = stat_.dev->open(); @@ -1039,7 +1002,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) stat_.pad = PAD_STAT; stat_.name = "stat"; - stat_.pool = &statPool_; return 0; } @@ -1139,85 +1101,49 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; } -/** - * \brief Import buffers from \a pool into the ImgU input - * \param[in] pool The buffer pool to import - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::importInputBuffers(BufferPool *pool) +int ImgUDevice::importInputBuffers(unsigned int count) { - int ret = input_->importBuffers(pool); - if (ret) { + int ret = input_->externalBuffers(count); + if (ret) LOG(IPU3, Error) << "Failed to import ImgU input buffers"; - return ret; - } - return 0; + return ret; } -/** - * \brief Export buffers from \a output to the provided \a pool - * \param[in] output The ImgU output device - * \param[in] pool The buffer pool where to export buffers - * - * Export memory buffers reserved in the video device memory associated with - * \a output id to the buffer pool provided as argument. - * - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::exportOutputBuffers(ImgUOutput *output, BufferPool *pool) +int ImgUDevice::exportOutputBuffers(ImgUOutput *output, unsigned int count) { - int ret = output->dev->exportBuffers(pool); - if (ret) { - LOG(IPU3, Error) << "Failed to export ImgU " - << output->name << " buffers"; - return ret; - } - - return 0; -} + int ret; -/** - * \brief Reserve buffers in \a output from the provided \a pool - * \param[in] output The ImgU output device - * \param[in] pool The buffer pool used to reserve buffers in \a output - * - * Reserve a number of buffers equal to the number of buffers in \a pool - * in the \a output device. - * - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::importOutputBuffers(ImgUOutput *output, BufferPool *pool) -{ - int ret = output->dev->importBuffers(pool); - if (ret) { - LOG(IPU3, Error) - << "Failed to import buffer in " << output->name - << " ImgU device"; - return ret; - } + ret = output->dev->allocateBuffers(count, &output->buffers); + if (ret < 0) + LOG(IPU3, Error) << "Failed to allocate ImgU " + << output->name << " buffers"; - return 0; + return ret; } /** * \brief Release buffers for all the ImgU video devices */ -void ImgUDevice::freeBuffers() +void ImgUDevice::freeBuffers(IPU3CameraData *data) { int ret; - ret = output_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU output buffers"; + if (!data->outStream_->active_) { + ret = output_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU output buffers"; + } ret = stat_.dev->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; - ret = viewfinder_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; + if (!data->vfStream_->active_) { + ret = viewfinder_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; + } ret = input_->releaseBuffers(); if (ret) @@ -1452,38 +1378,33 @@ int CIO2Device::configure(const Size &size, return 0; } -/** - * \brief Allocate CIO2 memory buffers and export them in a BufferPool - * - * Allocate memory buffers in the CIO2 video device and export them to - * a buffer pool that can be imported by another device. - * - * \return The buffer pool with export buffers on success or nullptr otherwise - */ -BufferPool *CIO2Device::exportBuffers() +int CIO2Device::exportBuffers() { - pool_.createBuffers(CIO2_BUFFER_COUNT); - - int ret = output_->exportBuffers(&pool_); - if (ret) { + int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); + if (ret < 0) LOG(IPU3, Error) << "Failed to export CIO2 buffers"; - return nullptr; - } - return &pool_; + return ret; } void CIO2Device::freeBuffers() { + for (FrameBuffer *buffer : buffers_) + delete buffer; + if (output_->releaseBuffers()) LOG(IPU3, Error) << "Failed to release CIO2 buffers"; } -int CIO2Device::start(std::vector> *buffers) +int CIO2Device::start() { - *buffers = output_->queueAllBuffers(); - if (buffers->empty()) - return -EINVAL; + for (FrameBuffer *buffer : buffers_) { + int ret = output_->queueBuffer(buffer); + if (ret) { + LOG(IPU3, Error) << "Failed to queue CIO2 buffer"; + return ret; + } + } return output_->streamOn(); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ca3d92c7ad637c3a..96e863f0208fa748 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -32,8 +32,7 @@ #include "v4l2_subdevice.h" #include "v4l2_videodevice.h" -#define RKISP1_PARAM_BASE 0x100 -#define RKISP1_STAT_BASE 0x200 +#define RKISP1_BUFFER_COUNT 4 namespace libcamera { @@ -52,9 +51,9 @@ struct RkISP1FrameInfo { unsigned int frame; Request *request; - Buffer *paramBuffer; - Buffer *statBuffer; - Buffer *videoBuffer; + FrameBuffer *paramBuffer; + FrameBuffer *statBuffer; + FrameBuffer *videoBuffer; bool paramFilled; bool paramDequeued; @@ -70,7 +69,7 @@ public: int destroy(unsigned int frame); RkISP1FrameInfo *find(unsigned int frame); - RkISP1FrameInfo *find(Buffer *buffer); + RkISP1FrameInfo *find(FrameBuffer *buffer); RkISP1FrameInfo *find(Request *request); private: @@ -89,7 +88,7 @@ public: setDelay(QueueBuffers, -1, 10); } - void bufferReady(Buffer *buffer) + void bufferReady(FrameBuffer *buffer) { /* * Calculate SOE by taking the end of DMA set by the kernel and applying @@ -154,8 +153,6 @@ public: const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - /* * The RkISP1CameraData instance is guaranteed to be valid as long as the * corresponding Camera instance is valid. In order to borrow a @@ -203,9 +200,9 @@ private: int initLinks(); int createCamera(MediaEntity *sensor); void tryCompleteRequest(Request *request); - void bufferReady(Buffer *buffer); - void paramReady(Buffer *buffer); - void statReady(Buffer *buffer); + void bufferReady(FrameBuffer *buffer); + void paramReady(FrameBuffer *buffer); + void statReady(FrameBuffer *buffer); MediaDevice *media_; V4L2Subdevice *dphy_; @@ -214,11 +211,8 @@ private: V4L2VideoDevice *param_; V4L2VideoDevice *stat_; - BufferPool paramPool_; - BufferPool statPool_; - - std::queue paramBuffers_; - std::queue statBuffers_; + std::queue paramBuffers_; + std::queue statBuffers_; Camera *activeCamera_; }; @@ -234,15 +228,15 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre LOG(RkISP1, Error) << "Parameters buffer underrun"; return nullptr; } - Buffer *paramBuffer = pipe_->paramBuffers_.front(); + FrameBuffer *paramBuffer = pipe_->paramBuffers_.front(); if (pipe_->statBuffers_.empty()) { LOG(RkISP1, Error) << "Statisitc buffer underrun"; return nullptr; } - Buffer *statBuffer = pipe_->statBuffers_.front(); + FrameBuffer *statBuffer = pipe_->statBuffers_.front(); - Buffer *videoBuffer = request->findBuffer(stream); + FrameBuffer *videoBuffer = request->findBuffer(stream); if (!videoBuffer) { LOG(RkISP1, Error) << "Attempt to queue request with invalid stream"; @@ -295,7 +289,7 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) return nullptr; } -RkISP1FrameInfo *RkISP1Frames::find(Buffer *buffer) +RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) { for (auto &itInfo : frameInfo_) { RkISP1FrameInfo *info = itInfo.second; @@ -661,57 +655,42 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, const std::set &streams) { RkISP1CameraData *data = cameraData(camera); - Stream *stream = *streams.begin(); + std::vector paramBuffers, statBuffers; + unsigned int count = 1; int ret; - if (stream->memoryType() == InternalMemory) - ret = video_->exportBuffers(&stream->bufferPool()); - else - ret = video_->importBuffers(&stream->bufferPool()); - if (ret) - return ret; + ret = param_->allocateBuffers(RKISP1_BUFFER_COUNT, ¶mBuffers); + if (ret < 0) + goto err; - paramPool_.createBuffers(stream->configuration().bufferCount + 1); - ret = param_->exportBuffers(¶mPool_); - if (ret) { - video_->releaseBuffers(); - return ret; - } + ret = stat_->allocateBuffers(RKISP1_BUFFER_COUNT, &statBuffers); + if (ret < 0) + goto err; - statPool_.createBuffers(stream->configuration().bufferCount + 1); - ret = stat_->exportBuffers(&statPool_); - if (ret) { - param_->releaseBuffers(); - video_->releaseBuffers(); - return ret; + for (FrameBuffer *buffer : paramBuffers) { + buffer->setCookie(count++); + data->ipaBuffers_.push_back({ .id = buffer->cookie(), + .planes = buffer->planes() }); + paramBuffers_.push(buffer); } - for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - std::vector planes; - planes.push_back({ - .fd = paramPool_.buffers()[i].planes()[0].fd(), - .length = paramPool_.buffers()[i].planes()[0].length(), - }); - - data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, - .planes = planes }); - paramBuffers_.push(new Buffer(i)); + for (FrameBuffer *buffer : statBuffers) { + buffer->setCookie(count++); + data->ipaBuffers_.push_back({ .id = buffer->cookie(), + .planes = buffer->planes() }); + statBuffers_.push(buffer); } - for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - std::vector planes; - planes.push_back({ - .fd = statPool_.buffers()[i].planes()[0].fd(), - .length = statPool_.buffers()[i].planes()[0].length(), - }); + data->ipa_->mapBuffers(data->ipaBuffers_); - data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, - .planes = planes }); - statBuffers_.push(new Buffer(i)); - } + return 0; +err: + for (FrameBuffer *buffer : paramBuffers) + delete buffer; - data->ipa_->mapBuffers(data->ipaBuffers_); + for (FrameBuffer *buffer : statBuffers) + delete buffer; return ret; } @@ -744,9 +723,6 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera, if (stat_->releaseBuffers()) LOG(RkISP1, Error) << "Failed to release stat buffers"; - if (video_->releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release video buffers"; - return 0; } @@ -834,9 +810,11 @@ int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, if (!info) return -ENOENT; + unsigned int paramid = info->paramBuffer->cookie(); + IPAOperationData op; op.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST; - op.data = { data->frame_, RKISP1_PARAM_BASE | info->paramBuffer->index() }; + op.data = { data->frame_, paramid }; op.controls = { request->controls() }; data->ipa_->processEvent(op); @@ -996,12 +974,12 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) if (!info->paramDequeued) return; - completeRequest(activeCamera_, request); - data->frameInfo_.destroy(info->frame); + + completeRequest(activeCamera_, request); } -void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) +void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); @@ -1016,7 +994,7 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) tryCompleteRequest(request); } -void PipelineHandlerRkISP1::paramReady(Buffer *buffer) +void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); @@ -1027,7 +1005,7 @@ void PipelineHandlerRkISP1::paramReady(Buffer *buffer) tryCompleteRequest(info->request); } -void PipelineHandlerRkISP1::statReady(Buffer *buffer) +void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); @@ -1037,7 +1015,7 @@ void PipelineHandlerRkISP1::statReady(Buffer *buffer) return; unsigned int frame = info->frame; - unsigned int statid = RKISP1_STAT_BASE | info->statBuffer->index(); + unsigned int statid = info->statBuffer->cookie(); IPAOperationData op; op.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 9cc90bf454cb4392..c275b9f0f75429bd 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -42,7 +42,7 @@ public: } int init(MediaEntity *entity); - void bufferReady(Buffer *buffer); + void bufferReady(FrameBuffer *buffer); V4L2VideoDevice *video_; Stream *stream_; @@ -196,23 +196,13 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) int PipelineHandlerUVC::allocateBuffers(Camera *camera, const std::set &streams) { - UVCCameraData *data = cameraData(camera); - Stream *stream = *streams.begin(); - const StreamConfiguration &cfg = stream->configuration(); - - LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - - if (stream->memoryType() == InternalMemory) - return data->video_->exportBuffers(&stream->bufferPool()); - else - return data->video_->importBuffers(&stream->bufferPool()); + return 0; } int PipelineHandlerUVC::freeBuffers(Camera *camera, const std::set &streams) { - UVCCameraData *data = cameraData(camera); - return data->video_->releaseBuffers(); + return 0; } int PipelineHandlerUVC::start(Camera *camera) @@ -266,7 +256,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(data->stream_); + FrameBuffer *buffer = request->findBuffer(data->stream_); if (!buffer) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; @@ -373,7 +363,7 @@ int UVCCameraData::init(MediaEntity *entity) return 0; } -void UVCCameraData::bufferReady(Buffer *buffer) +void UVCCameraData::bufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index b1222bd21fb629a0..64793788c752c791 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -57,7 +57,7 @@ public: } int init(MediaDevice *media); - void bufferReady(Buffer *buffer); + void bufferReady(FrameBuffer *buffer); CameraSensor *sensor_; V4L2Subdevice *debayer_; @@ -264,23 +264,13 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) int PipelineHandlerVimc::allocateBuffers(Camera *camera, const std::set &streams) { - VimcCameraData *data = cameraData(camera); - Stream *stream = *streams.begin(); - const StreamConfiguration &cfg = stream->configuration(); - - LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - - if (stream->memoryType() == InternalMemory) - return data->video_->exportBuffers(&stream->bufferPool()); - else - return data->video_->importBuffers(&stream->bufferPool()); + return 0; } int PipelineHandlerVimc::freeBuffers(Camera *camera, const std::set &streams) { - VimcCameraData *data = cameraData(camera); - return data->video_->releaseBuffers(); + return 0; } int PipelineHandlerVimc::start(Camera *camera) @@ -328,7 +318,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(data->stream_); + FrameBuffer *buffer = request->findBuffer(data->stream_); if (!buffer) { LOG(VIMC, Error) << "Attempt to queue request with invalid stream"; @@ -459,7 +449,7 @@ int VimcCameraData::init(MediaDevice *media) return 0; } -void VimcCameraData::bufferReady(Buffer *buffer) +void VimcCameraData::bufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c9e348b98da7b736..b107e23258dee692 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -398,7 +398,7 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request) * otherwise */ bool PipelineHandler::completeBuffer(Camera *camera, Request *request, - Buffer *buffer) + FrameBuffer *buffer) { camera->bufferCompleted.emit(request, buffer); return request->completeBuffer(buffer); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 7593bf9dfa546401..a4b27e1cb08a7641 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -75,11 +75,6 @@ Request::Request(Camera *camera, uint64_t cookie) Request::~Request() { - for (auto it : bufferMap_) { - Buffer *buffer = it.second; - delete buffer; - } - delete metadata_; delete controls_; delete validator_; @@ -126,7 +121,7 @@ Request::~Request() * \retval -EEXIST The request already contains a buffer for the stream * \retval -EINVAL The buffer does not reference a valid Stream */ -int Request::addBuffer(Stream *stream, std::unique_ptr buffer) +int Request::addBuffer(Stream *stream, FrameBuffer *buffer) { if (!stream) { LOG(Request, Error) << "Invalid stream reference"; @@ -139,7 +134,7 @@ int Request::addBuffer(Stream *stream, std::unique_ptr buffer) return -EEXIST; } - bufferMap_[stream] = buffer.release(); + bufferMap_[stream] = buffer; return 0; } @@ -159,7 +154,7 @@ int Request::addBuffer(Stream *stream, std::unique_ptr buffer) * \return The buffer associated with the stream, or nullptr if the stream is * not part of this request */ -Buffer *Request::findBuffer(Stream *stream) const +FrameBuffer *Request::findBuffer(Stream *stream) const { auto it = bufferMap_.find(stream); if (it == bufferMap_.end()) @@ -219,8 +214,9 @@ int Request::prepare() } for (auto const &pair : bufferMap_) { - Buffer *buffer = pair.second; + FrameBuffer *buffer = pair.second; buffer->request_ = this; + pending_.insert(buffer); } @@ -253,7 +249,7 @@ void Request::complete() * \return True if all buffers contained in the request have completed, false * otherwise */ -bool Request::completeBuffer(Buffer *buffer) +bool Request::completeBuffer(FrameBuffer *buffer) { int ret = pending_.erase(buffer); ASSERT(ret == 1); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index f5810956b2040ce6..a29ed697468a7f59 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1191,9 +1191,11 @@ int V4L2VideoDevice::externalBuffers(unsigned int count) */ int V4L2VideoDevice::releaseBuffers() { - LOG(V4L2, Debug) << "Releasing bufferPool"; + LOG(V4L2, Debug) << "Releasing buffers"; bufferPool_ = nullptr; + delete cache_; + cache_ = nullptr; return requestBuffers(0); } @@ -1209,27 +1211,26 @@ int V4L2VideoDevice::releaseBuffers() * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::queueBuffer(Buffer *buffer) +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) { struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; struct v4l2_buffer buf = {}; int ret; - buf.index = buffer->index(); + buf.index = cache_->fetch(buffer); buf.type = bufferType_; buf.memory = memoryType_; buf.field = V4L2_FIELD_NONE; bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); - BufferMemory *mem = &bufferPool_->buffers()[buf.index]; - const std::vector &dmabufs = mem->planes(); + const std::vector &dmabufs = buffer->dmabufs(); if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { for (unsigned int p = 0; p < dmabufs.size(); ++p) - v4l2Planes[p].m.fd = dmabufs[p].fd(); + v4l2Planes[p].m.fd = dmabufs[p]->fd(); } else { - buf.m.fd = dmabufs[0].fd(); + buf.m.fd = dmabufs[0]->fd(); } } @@ -1275,52 +1276,6 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) return 0; } -/** - * \brief Queue all buffers into the video device - * - * When starting video capture users of the video device often need to queue - * all allocated buffers to the device. This helper method simplifies the - * implementation of the user by queuing all buffers and returning a vector of - * Buffer instances for each queued buffer. - * - * This method is meant to be used with video capture devices internal to a - * pipeline handler, such as ISP statistics capture devices, or raw CSI-2 - * receivers. For video capture devices facing applications, buffers shall - * instead be queued when requests are received, and for video output devices, - * buffers shall be queued when frames are ready to be output. - * - * The caller shall ensure that the returned buffers vector remains valid until - * all the queued buffers are dequeued, either during capture, or by stopping - * the video device. - * - * Calling this method on an output device or on a device that has buffers - * already queued is an error and will return an empty vector. - * - * \return A vector of queued buffers, which will be empty if an error occurs - */ -std::vector> V4L2VideoDevice::queueAllBuffers() -{ - int ret; - - if (!queuedBuffers_.empty()) - return {}; - - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) - return {}; - - std::vector> buffers; - - for (unsigned int i = 0; i < bufferPool_->count(); ++i) { - Buffer *buffer = new Buffer(i); - buffers.emplace_back(buffer); - ret = queueBuffer(buffer); - if (ret) - return {}; - } - - return buffers; -} - /** * \brief Dequeue the next available buffer from the video device * @@ -1329,7 +1284,7 @@ std::vector> V4L2VideoDevice::queueAllBuffers() * * \return A pointer to the dequeued buffer on success, or nullptr otherwise */ -Buffer *V4L2VideoDevice::dequeueBuffer() +FrameBuffer *V4L2VideoDevice::dequeueBuffer() { struct v4l2_buffer buf = {}; struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; @@ -1352,15 +1307,15 @@ Buffer *V4L2VideoDevice::dequeueBuffer() LOG(V4L2, Debug) << "Buffer " << buf.index << " is available"; + cache_->put(buf.index); + auto it = queuedBuffers_.find(buf.index); - Buffer *buffer = it->second; + FrameBuffer *buffer = it->second; queuedBuffers_.erase(it); if (queuedBuffers_.empty()) fdEvent_->setEnabled(false); - buffer->index_ = buf.index; - BufferInfo::Status status = buf.flags & V4L2_BUF_FLAG_ERROR ? BufferInfo::BufferError : BufferInfo::BufferSuccess; uint64_t timestamp = buf.timestamp.tv_sec * 1000000000ULL @@ -1383,7 +1338,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer() */ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) { - Buffer *buffer = dequeueBuffer(); + FrameBuffer *buffer = dequeueBuffer(); if (!buffer) return; @@ -1418,7 +1373,7 @@ int V4L2VideoDevice::streamOn() * \brief Stop the video stream * * Buffers that are still queued when the video stream is stopped are - * immediately dequeued with their status set to Buffer::BufferError, + * immediately dequeued with their status set to BufferInfo::BufferCancelled, * and the bufferReady signal is emitted for them. The order in which those * buffers are dequeued is not specified. * @@ -1437,11 +1392,10 @@ int V4L2VideoDevice::streamOff() /* Send back all queued buffers. */ for (auto it : queuedBuffers_) { - unsigned int index = it.first; - Buffer *buffer = it.second; + FrameBuffer *buffer = it.second; + + buffer->info_.update(BufferInfo::BufferCancelled, 0, 0, { {} }); - buffer->index_ = index; - buffer->cancel(); bufferReady.emit(buffer); } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 771020112f09b1ef..9650bd970f89aa41 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -22,7 +22,7 @@ using namespace libcamera; MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) - : options_(options), isCapturing_(false) + : options_(options), allocator_(nullptr), isCapturing_(false) { int ret; @@ -36,8 +36,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) adjustSize(); ret = openCamera(cm); - if (!ret) + if (!ret) { + allocator_ = new BufferAllocator(camera_); ret = startCapture(); + } if (ret < 0) QTimer::singleShot(0, QCoreApplication::instance(), @@ -51,6 +53,8 @@ MainWindow::~MainWindow() camera_->release(); camera_.reset(); } + + delete allocator_; } void MainWindow::updateTitle() @@ -175,8 +179,14 @@ int MainWindow::startCapture() return ret; } + ret = allocator_->allocate(stream, cfg); + if (ret < 0) { + std::cerr << "Failed to allocate internal buffers" << std::endl; + return ret; + } + std::vector requests; - for (unsigned int i = 0; i < cfg.bufferCount; ++i) { + for (FrameBuffer *buffer : allocator_->buffers(stream)) { Request *request = camera_->createRequest(); if (!request) { std::cerr << "Can't create request" << std::endl; @@ -184,13 +194,7 @@ int MainWindow::startCapture() goto error; } - std::unique_ptr buffer = stream->createBuffer(i); - if (!buffer) { - std::cerr << "Can't create buffer " << i << std::endl; - goto error; - } - - ret = request->addBuffer(stream, std::move(buffer)); + ret = request->addBuffer(stream, buffer); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; goto error; @@ -253,11 +257,11 @@ void MainWindow::requestComplete(Request *request) if (request->status() == Request::RequestCancelled) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); framesCaptured_++; - Buffer *buffer = buffers.begin()->second; + FrameBuffer *buffer = buffers.begin()->second; const BufferInfo &info = buffer->info(); double fps = info.timestamp() - lastBufferTime_; @@ -280,29 +284,20 @@ void MainWindow::requestComplete(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; - Buffer *buffer = it->second; - unsigned int index = buffer->index(); - - std::unique_ptr newBuffer = stream->createBuffer(index); - if (!newBuffer) { - std::cerr << "Can't create buffer" << std::endl; - return; - } + FrameBuffer *buffer = it->second; - request->addBuffer(stream, std::move(newBuffer)); + request->addBuffer(stream, buffer); } camera_->queueRequest(request); } -int MainWindow::display(Buffer *buffer) +int MainWindow::display(FrameBuffer *buffer) { - BufferMemory *mem = buffer->mem(); - if (mem->planes().size() != 1) + if (buffer->planes().size() != 1) return -EINVAL; - Dmabuf &dmabuf = mem->planes().front(); - unsigned char *raw = static_cast(dmabuf.mem()); + unsigned char *raw = static_cast(buffer->dmabufs()[0]->mem()); viewfinder_->display(raw, buffer->info().planes()[0].bytesused); return 0; diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 0786e915ec512255..b38aa2ac959f44aa 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -14,6 +14,8 @@ #include #include +#include +#include #include #include #include @@ -49,7 +51,7 @@ private: void stopCapture(); void requestComplete(Request *request); - int display(Buffer *buffer); + int display(FrameBuffer *buffer); QString title_; QTimer titleTimer_; @@ -57,6 +59,8 @@ private: const OptionsParser::Options &options_; std::shared_ptr camera_; + BufferAllocator *allocator_; + bool isCapturing_; std::unique_ptr config_; diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 5dbaed9255d3d60c..c98afeb9597e21c2 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -21,35 +21,48 @@ #include "test.h" using namespace libcamera; +using namespace std; -/* Keep SINK_BUFFER_COUNT > CAMERA_BUFFER_COUNT + 1 */ -static constexpr unsigned int SINK_BUFFER_COUNT = 8; -static constexpr unsigned int CAMERA_BUFFER_COUNT = 4; +namespace { -class FrameSink +class BufferSource { public: - FrameSink() + BufferSource() : video_(nullptr) { } - int init() + ~BufferSource() + { + if (video_) { + video_->releaseBuffers(); + video_->close(); + } + + delete video_; + video_ = nullptr; + + if (media_) + media_->release(); + } + + int allocate(const StreamConfiguration &config) { int ret; /* Locate and open the video device. */ - std::string videoDeviceName = "vivid-000-vid-out"; + string videoDeviceName = "vivid-000-vid-out"; - std::unique_ptr enumerator = + unique_ptr enumerator = DeviceEnumerator::create(); if (!enumerator) { - std::cout << "Failed to create device enumerator" << std::endl; + cout << "Failed to create device enumerator" << std::endl; return TestFail; } if (enumerator->enumerate()) { - std::cout << "Failed to enumerate media devices" << std::endl; + cout << "Failed to enumerate media devices" << std::endl; return TestFail; } @@ -58,13 +71,13 @@ public: media_ = enumerator->search(dm); if (!media_) { - std::cout << "No vivid output device available" << std::endl; + cout << "No vivid output device available" << std::endl; return TestSkip; } video_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName); if (!video_) { - std::cout << "Unable to open " << videoDeviceName << std::endl; + cout << "Unable to open " << videoDeviceName << std::endl; return TestFail; } @@ -72,366 +85,178 @@ public: return TestFail; /* Configure the format. */ - ret = video_->getFormat(&format_); + V4L2DeviceFormat format; + ret = video_->getFormat(&format); if (ret) { - std::cout << "Failed to get format on output device" << std::endl; + cout << "Failed to get format on output device" << std::endl; return ret; } - format_.size.width = 1920; - format_.size.height = 1080; - format_.fourcc = V4L2_PIX_FMT_RGB24; - format_.planesCount = 1; - format_.planes[0].size = 1920 * 1080 * 3; - format_.planes[0].bpl = 1920 * 3; - - if (video_->setFormat(&format_)) { - cleanup(); + format.size = config.size; + format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false); + if (video_->setFormat(&format)) return TestFail; - } - /* Export the buffers to a pool. */ - pool_.createBuffers(SINK_BUFFER_COUNT); - ret = video_->exportBuffers(&pool_); - if (ret) { - std::cout << "Failed to export buffers" << std::endl; - cleanup(); - return TestFail; - } - - /* Only use the first CAMERA_BUFFER_COUNT buffers to start with. */ - availableBuffers_.resize(CAMERA_BUFFER_COUNT); - std::iota(availableBuffers_.begin(), availableBuffers_.end(), 0); - - /* Connect the buffer ready signal. */ - video_->bufferReady.connect(this, &FrameSink::bufferComplete); - - return TestPass; + return video_->allocateBuffers(config.bufferCount, &buffers_); } - void cleanup() - { - if (video_) { - video_->streamOff(); - video_->releaseBuffers(); - video_->close(); - - delete video_; - video_ = nullptr; - } - - if (media_) - media_->release(); - } - - int start() - { - requestsCount_ = 0; - done_ = false; - - int ret = video_->streamOn(); - if (ret < 0) - return ret; - - /* Queue all the initial requests. */ - for (unsigned int index = 0; index < CAMERA_BUFFER_COUNT; ++index) - queueRequest(index); - - return 0; - } - - int stop() - { - return video_->streamOff(); - } - - void requestComplete(uint64_t cookie, const Buffer *metadata) - { - unsigned int index = cookie; - - Buffer *buffer = new Buffer(index, metadata); - int ret = video_->queueBuffer(buffer); - if (ret < 0) - std::cout << "Failed to queue buffer to sink" << std::endl; - } - - bool done() const { return done_; } - - PixelFormat format() const - { - return video_->toPixelFormat(format_.fourcc); - } - - const Size &size() const - { - return format_.size; - } - - Signal requestReady; + const vector &buffers() { return buffers_; }; private: - void queueRequest(unsigned int index) - { - auto it = std::find(availableBuffers_.begin(), - availableBuffers_.end(), index); - availableBuffers_.erase(it); - - uint64_t cookie = index; - BufferMemory &mem = pool_.buffers()[index]; - int dmabuf = mem.planes()[0].fd(); - - requestReady.emit(cookie, dmabuf); - - requestsCount_++; - } - - void bufferComplete(Buffer *buffer) - { - availableBuffers_.push_back(buffer->index()); - - /* - * Pick the buffer for the next request among the available - * buffers. - * - * For the first 20 frames, select the buffer that has just - * completed to keep the mapping of dmabuf fds to buffers - * unchanged in the camera. - * - * For the next 20 frames, cycle randomly over the available - * buffers. The mapping should still be kept unchanged, as the - * camera should map using the cached fds. - * - * For the last 20 frames, cycles through all buffers, which - * should trash the mappings. - */ - unsigned int index = buffer->index(); - delete buffer; - - std::cout << "Completed buffer, request=" << requestsCount_ - << ", available buffers=" << availableBuffers_.size() - << std::endl; - - if (requestsCount_ >= 60) { - if (availableBuffers_.size() == SINK_BUFFER_COUNT) - done_ = true; - return; - } - - if (requestsCount_ == 40) { - /* Add the remaining of the buffers. */ - for (unsigned int i = CAMERA_BUFFER_COUNT; - i < SINK_BUFFER_COUNT; ++i) - availableBuffers_.push_back(i); - } - - if (requestsCount_ >= 20) { - /* - * Wait until we have enough buffers to make this - * meaningful. Preferably half of the camera buffers, - * but no less than 2 in any case. - */ - const unsigned int min_pool_size = - std::min(CAMERA_BUFFER_COUNT / 2, 2U); - if (availableBuffers_.size() < min_pool_size) - return; - - /* Pick a buffer at random. */ - unsigned int pos = random_() % availableBuffers_.size(); - index = availableBuffers_[pos]; - } - - queueRequest(index); - } - - std::shared_ptr media_; + shared_ptr media_; V4L2VideoDevice *video_; - BufferPool pool_; - V4L2DeviceFormat format_; - - unsigned int requestsCount_; - std::vector availableBuffers_; - std::random_device random_; - - bool done_; + vector buffers_; }; -class BufferImportTest : public CameraTest, public Test +class BufferImport : public CameraTest, public Test { public: - BufferImportTest() + BufferImport() : CameraTest("VIMC Sensor B") { } - void queueRequest(uint64_t cookie, int dmabuf) +protected: + unsigned int completeBuffersCount_; + unsigned int completeRequestsCount_; + + void bufferComplete(Request *request, FrameBuffer *buffer) { - Request *request = camera_->createRequest(cookie); + if (buffer->info().status() != BufferInfo::BufferSuccess) + return; - std::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 }); - request->addBuffer(stream_, move(buffer)); - camera_->queueRequest(request); + completeBuffersCount_++; } -protected: - void bufferComplete(Request *request, Buffer *buffer) + void requestComplete(Request *request) { - if (buffer->info().status() != BufferInfo::BufferSuccess) + if (request->status() != Request::RequestComplete) return; - unsigned int index = buffer->index(); - int dmabuf = buffer->dmabufs()[0]; + const map &buffers = request->buffers(); - /* Record dmabuf to index remappings. */ - bool remapped = false; - if (bufferMappings_.find(index) != bufferMappings_.end()) { - if (bufferMappings_[index] != dmabuf) - remapped = true; - } + completeRequestsCount_++; - std::cout << "Completed request " << framesCaptured_ - << ": dmabuf fd " << dmabuf - << " -> index " << index - << " (" << (remapped ? 'R' : '-') << ")" - << std::endl; + /* Create a new request. */ + Stream *stream = buffers.begin()->first; + FrameBuffer *buffer = buffers.begin()->second; - if (remapped) - bufferRemappings_.push_back(framesCaptured_); + request = camera_->createRequest(); + request->addBuffer(stream, buffer); + camera_->queueRequest(request); + } - bufferMappings_[index] = dmabuf; - framesCaptured_++; + int init() override + { + if (status_ != TestPass) + return status_; - sink_.requestComplete(request->cookie(), buffer); + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config_ || config_->size() != 1) { + cout << "Failed to generate default configuration" << endl; + return TestFail; + } - if (framesCaptured_ == 60) - sink_.stop(); + return TestPass; } - int initCamera() + int run() override { - if (camera_->acquire()) { - std::cout << "Failed to acquire the camera" << std::endl; - return TestFail; - } + StreamConfiguration &cfg = config_->at(0); - /* - * Configure the Stream to work with externally allocated - * buffers by setting the memoryType to ExternalMemory. - */ - std::unique_ptr config; - config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config || config->size() != 1) { - std::cout << "Failed to generate configuration" << std::endl; + if (camera_->acquire()) { + cout << "Failed to acquire the camera" << endl; return TestFail; } - StreamConfiguration &cfg = config->at(0); - cfg.size = sink_.size(); - cfg.pixelFormat = sink_.format(); - cfg.bufferCount = CAMERA_BUFFER_COUNT; - cfg.memoryType = ExternalMemory; - - if (camera_->configure(config.get())) { - std::cout << "Failed to set configuration" << std::endl; + if (camera_->configure(config_.get())) { + cout << "Failed to set default configuration" << endl; return TestFail; } - stream_ = cfg.stream(); - - /* Allocate buffers. */ if (camera_->allocateBuffers()) { - std::cout << "Failed to allocate buffers" << std::endl; + cout << "Failed to allocate buffers" << endl; return TestFail; } - /* Connect the buffer completed signal. */ - camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete); + Stream *stream = cfg.stream(); - return TestPass; - } + BufferSource source; + int ret = source.allocate(cfg); + if (ret < 0) + return TestFail; - int init() - { - if (status_ != TestPass) - return status_; + vector requests; + for (FrameBuffer *buffer : source.buffers()) { + Request *request = camera_->createRequest(); + if (!request) { + cout << "Failed to create request" << endl; + return TestFail; + } - int ret = sink_.init(); - if (ret != TestPass) { - cleanup(); - return ret; - } + if (request->addBuffer(stream, buffer)) { + cout << "Failed to associating buffer with request" << endl; + return TestFail; + } - ret = initCamera(); - if (ret != TestPass) { - cleanup(); - return ret; + requests.push_back(request); } - sink_.requestReady.connect(this, &BufferImportTest::queueRequest); - return TestPass; - } - - int run() - { - int ret; + completeRequestsCount_ = 0; + completeBuffersCount_ = 0; - framesCaptured_ = 0; + camera_->bufferCompleted.connect(this, &BufferImport::bufferComplete); + camera_->requestCompleted.connect(this, &BufferImport::requestComplete); if (camera_->start()) { - std::cout << "Failed to start camera" << std::endl; + cout << "Failed to start camera" << endl; return TestFail; } - ret = sink_.start(); - if (ret < 0) { - std::cout << "Failed to start sink" << std::endl; - return TestFail; + for (Request *request : requests) { + if (camera_->queueRequest(request)) { + cout << "Failed to queue request" << endl; + return TestFail; + } } EventDispatcher *dispatcher = cm_->eventDispatcher(); Timer timer; - timer.start(5000); - while (timer.isRunning() && !sink_.done()) + timer.start(1000); + while (timer.isRunning()) dispatcher->processEvents(); - std::cout << framesCaptured_ << " frames captured, " - << bufferRemappings_.size() << " buffers remapped" - << std::endl; + unsigned int nbuffers = source.buffers().size(); - if (framesCaptured_ < 60) { - std::cout << "Too few frames captured" << std::endl; + if (completeRequestsCount_ <= nbuffers * 2) { + cout << "Failed to capture enough frames (got " + << completeRequestsCount_ << " expected at least " + << nbuffers * 2 << ")" << endl; return TestFail; } - if (bufferRemappings_.empty()) { - std::cout << "No buffer remappings" << std::endl; + if (completeRequestsCount_ != completeBuffersCount_) { + cout << "Number of completed buffers and requests differ" << endl; return TestFail; } - if (bufferRemappings_[0] < 40) { - std::cout << "Early buffer remapping" << std::endl; + if (camera_->stop()) { + cout << "Failed to stop camera" << endl; return TestFail; } - return TestPass; - } - - void cleanup() - { - sink_.cleanup(); + if (camera_->freeBuffers()) { + cout << "Failed to free buffers" << endl; + return TestFail; + } - camera_->stop(); - camera_->freeBuffers(); + return TestPass; } -private: - Stream *stream_; - - std::map bufferMappings_; - std::vector bufferRemappings_; - unsigned int framesCaptured_; - - FrameSink sink_; + unique_ptr config_; }; -TEST_REGISTER(BufferImportTest); +} /* namespace */ + +TEST_REGISTER(BufferImport); diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 8307ea2629801679..ee6d0a1bc1fa839d 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -26,7 +26,7 @@ protected: unsigned int completeBuffersCount_; unsigned int completeRequestsCount_; - void bufferComplete(Request *request, Buffer *buffer) + void bufferComplete(Request *request, FrameBuffer *buffer) { if (buffer->info().status() != BufferInfo::BufferSuccess) return; @@ -39,17 +39,16 @@ protected: if (request->status() != Request::RequestComplete) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); completeRequestsCount_++; /* Create a new request. */ Stream *stream = buffers.begin()->first; - Buffer *buffer = buffers.begin()->second; - std::unique_ptr newBuffer = stream->createBuffer(buffer->index()); + FrameBuffer *buffer = buffers.begin()->second; request = camera_->createRequest(); - request->addBuffer(stream, std::move(newBuffer)); + request->addBuffer(stream, buffer); camera_->queueRequest(request); } @@ -87,21 +86,21 @@ protected: } Stream *stream = cfg.stream(); + + BufferAllocator allocator(camera_); + int ret = allocator.allocate(stream, cfg); + if (ret < 0) + return TestFail; + std::vector requests; - for (unsigned int i = 0; i < cfg.bufferCount; ++i) { + for (FrameBuffer *buffer : allocator.buffers(stream)) { Request *request = camera_->createRequest(); if (!request) { cout << "Failed to create request" << endl; return TestFail; } - std::unique_ptr buffer = stream->createBuffer(i); - if (!buffer) { - cout << "Failed to create buffer " << i << endl; - return TestFail; - } - - if (request->addBuffer(stream, std::move(buffer))) { + if (request->addBuffer(stream, buffer)) { cout << "Failed to associating buffer with request" << endl; return TestFail; } @@ -134,10 +133,12 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ <= cfg.bufferCount * 2) { + unsigned int nbuffers = allocator.buffers(stream).size(); + + if (completeRequestsCount_ <= nbuffers * 2) { cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg.bufferCount * 2 << ")" << endl; + << nbuffers * 2 << ")" << endl; return TestFail; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index f627b8f37422350e..e9468d806f977a63 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -185,6 +185,12 @@ protected: if (camera_->allocateBuffers()) return TestFail; + /* Use internally allocated buffers. */ + allocator_ = new BufferAllocator(camera_); + Stream *stream = *camera_->streams().begin(); + if (allocator_->allocate(stream, defconf_->at(0)) < 0) + return TestFail; + if (camera_->start()) return TestFail; @@ -218,8 +224,7 @@ protected: return TestFail; Stream *stream = *camera_->streams().begin(); - std::unique_ptr buffer = stream->createBuffer(0); - if (request->addBuffer(stream, std::move(buffer))) + if (request->addBuffer(stream, allocator_->buffers(stream)[0])) return TestFail; if (camera_->queueRequest(request)) @@ -229,6 +234,8 @@ protected: if (camera_->stop()) return TestFail; + delete allocator_; + if (camera_->freeBuffers()) return TestFail; @@ -283,6 +290,7 @@ protected: } std::unique_ptr defconf_; + BufferAllocator *allocator_; }; } /* namespace */ diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 6b71caef111693d6..1b6ff756ac53a96a 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -73,16 +73,14 @@ protected: return TestFail; } - pool_.createBuffers(bufferCount); - - ret = capture_->exportBuffers(&pool_); - if (ret) { - std::cout << "Failed to export buffers" << std::endl; + ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) { + std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } - ret = output_->importBuffers(&pool_); - if (ret) { + ret = output_->externalBuffers(bufferCount); + if (ret < 0) { std::cout << "Failed to import buffers" << std::endl; return TestFail; } @@ -90,7 +88,7 @@ protected: return 0; } - void captureBufferReady(Buffer *buffer) + void captureBufferReady(FrameBuffer *buffer) { const BufferInfo &info = buffer->info(); @@ -101,10 +99,11 @@ protected: return; output_->queueBuffer(buffer); + framesCaptured_++; } - void outputBufferReady(Buffer *buffer) + void outputBufferReady(FrameBuffer *buffer) { const BufferInfo &info = buffer->info(); @@ -127,10 +126,8 @@ protected: capture_->bufferReady.connect(this, &BufferSharingTest::captureBufferReady); output_->bufferReady.connect(this, &BufferSharingTest::outputBufferReady); - std::vector> buffers; - buffers = capture_->queueAllBuffers(); - if (buffers.empty()) - return TestFail; + for (FrameBuffer *buffer : buffers_) + capture_->queueBuffer(buffer); ret = capture_->streamOn(); if (ret) { diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index 0bc0067c50909c9d..78ac34c4799e9e5a 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -20,7 +20,7 @@ public: CaptureAsyncTest() : V4L2VideoDeviceTest("vimc", "Raw Capture 0"), frames(0) {} - void receiveBuffer(Buffer *buffer) + void receiveBuffer(FrameBuffer *buffer) { std::cout << "Received buffer" << std::endl; frames++; @@ -38,18 +38,14 @@ protected: Timer timeout; int ret; - pool_.createBuffers(bufferCount); - - ret = capture_->exportBuffers(&pool_); - if (ret) + ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) return TestFail; capture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); - std::vector> buffers; - buffers = capture_->queueAllBuffers(); - if (buffers.empty()) - return TestFail; + for (FrameBuffer *buffer : buffers_) + capture_->queueBuffer(buffer); ret = capture_->streamOn(); if (ret) diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp index c4aedf7b3cd61e80..2f8dfe1cafb111df 100644 --- a/test/v4l2_videodevice/request_buffers.cpp +++ b/test/v4l2_videodevice/request_buffers.cpp @@ -16,17 +16,10 @@ public: protected: int run() { - /* - * TODO: - * Test invalid requests - * Test different buffer allocations - */ const unsigned int bufferCount = 8; - pool_.createBuffers(bufferCount); - - int ret = capture_->exportBuffers(&pool_); - if (ret) + int ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret != bufferCount) return TestFail; return TestPass; diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp index 7664adc4c1f07046..ce48310aa2b7c3a8 100644 --- a/test/v4l2_videodevice/stream_on_off.cpp +++ b/test/v4l2_videodevice/stream_on_off.cpp @@ -17,10 +17,8 @@ protected: { const unsigned int bufferCount = 8; - pool_.createBuffers(bufferCount); - - int ret = capture_->exportBuffers(&pool_); - if (ret) + int ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) return TestFail; ret = capture_->streamOn(); diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index 442bcac5dc49cc59..2dd4b9440b4d4d71 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -29,7 +29,7 @@ public: { } - void outputBufferComplete(Buffer *buffer) + void outputBufferComplete(FrameBuffer *buffer) { cout << "Received output buffer" << endl; @@ -39,7 +39,7 @@ public: vim2m_->output()->queueBuffer(buffer); } - void receiveCaptureBuffer(Buffer *buffer) + void receiveCaptureBuffer(FrameBuffer *buffer) { cout << "Received capture buffer" << endl; @@ -112,17 +112,14 @@ protected: return TestFail; } - capturePool_.createBuffers(bufferCount); - outputPool_.createBuffers(bufferCount); - - ret = capture->exportBuffers(&capturePool_); - if (ret) { + ret = capture->allocateBuffers(bufferCount, &captureBuffers_); + if (ret < 0) { cerr << "Failed to export Capture Buffers" << endl; return TestFail; } - ret = output->exportBuffers(&outputPool_); - if (ret) { + ret = output->allocateBuffers(bufferCount, &outputBuffers_); + if (ret < 0) { cerr << "Failed to export Output Buffers" << endl; return TestFail; } @@ -130,24 +127,11 @@ protected: capture->bufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer); output->bufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete); - std::vector> captureBuffers; - captureBuffers = capture->queueAllBuffers(); - if (captureBuffers.empty()) { - cerr << "Failed to queue all Capture Buffers" << endl; - return TestFail; - } + for (FrameBuffer *buffer : captureBuffers_) + capture->queueBuffer(buffer); - /* We can't "queueAllBuffers()" on an output device, so we do it manually */ - std::vector> outputBuffers; - for (unsigned int i = 0; i < outputPool_.count(); ++i) { - Buffer *buffer = new Buffer(i); - outputBuffers.emplace_back(buffer); - ret = output->queueBuffer(buffer); - if (ret) { - cerr << "Failed to queue output buffer" << i << endl; - return TestFail; - } - } + for (FrameBuffer *buffer : outputBuffers_) + output->queueBuffer(buffer); ret = capture->streamOn(); if (ret) { @@ -202,8 +186,8 @@ private: std::shared_ptr media_; V4L2M2MDevice *vim2m_; - BufferPool capturePool_; - BufferPool outputPool_; + std::vector captureBuffers_; + std::vector outputBuffers_; unsigned int outputFrames_; unsigned int captureFrames_; diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h index 34dd231c6d9d108d..02800c2a16f7b0f7 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.h +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h @@ -41,7 +41,7 @@ protected: CameraSensor *sensor_; V4L2Subdevice *debayer_; V4L2VideoDevice *capture_; - BufferPool pool_; + std::vector buffers_; }; #endif /* __LIBCAMERA_V4L2_DEVICE_TEST_H_ */ From patchwork Tue Nov 26 23:36:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2376 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 26D3D61C77 for ; Wed, 27 Nov 2019 00:39:51 +0100 (CET) X-Halon-ID: 088d65ae-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 088d65ae-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:48 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:18 +0100 Message-Id: <20191126233620.1695316-29-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 28/30] libcamera: Remove dead code after switch to FrameBuffer 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:51 -0000 Delete all dead code after switching to the FrameBuffer interface. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 59 ------ include/libcamera/stream.h | 23 --- src/android/camera_device.cpp | 1 - src/libcamera/buffer.cpp | 183 ----------------- src/libcamera/include/v4l2_videodevice.h | 7 - src/libcamera/stream.cpp | 247 +---------------------- src/libcamera/v4l2_videodevice.cpp | 110 +--------- 7 files changed, 3 insertions(+), 627 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index acc876eec7d93873..6be26a6400e91546 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -77,65 +77,6 @@ private: void *mem_; }; -class BufferMemory final -{ -public: - const std::vector &planes() const { return planes_; } - std::vector &planes() { return planes_; } - -private: - std::vector planes_; -}; - -class BufferPool final -{ -public: - ~BufferPool(); - - void createBuffers(unsigned int count); - void destroyBuffers(); - - unsigned int count() const { return buffers_.size(); } - std::vector &buffers() { return buffers_; } - -private: - std::vector buffers_; -}; - -class Buffer final -{ -public: - Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); - Buffer(const Buffer &) = delete; - Buffer &operator=(const Buffer &) = delete; - - unsigned int index() const { return index_; } - const std::array &dmabufs() const { return dmabuf_; } - BufferMemory *mem() { return mem_; } - - const BufferInfo &info() const { return info_; }; - - Request *request() const { return request_; } - Stream *stream() const { return stream_; } - -private: - friend class Camera; - friend class Request; - friend class Stream; - friend class V4L2VideoDevice; - - void cancel(); - - unsigned int index_; - std::array dmabuf_; - BufferMemory *mem_; - - BufferInfo info_; - - Request *request_; - Stream *stream_; -}; - class FrameBuffer final { public: diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 8e653408fd54cf74..a4a8b4d3105add4c 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -36,11 +36,6 @@ private: std::map> formats_; }; -enum MemoryType { - InternalMemory, - ExternalMemory, -}; - struct StreamConfiguration { StreamConfiguration(); StreamConfiguration(const StreamFormats &formats); @@ -48,7 +43,6 @@ struct StreamConfiguration { PixelFormat pixelFormat; Size size; - MemoryType memoryType; unsigned int bufferCount; Stream *stream() const { return stream_; } @@ -76,13 +70,7 @@ public: Stream(); virtual ~Stream(){}; - std::unique_ptr createBuffer(unsigned int index); - std::unique_ptr createBuffer(const std::array &fds); - - BufferPool &bufferPool() { return bufferPool_; } - std::vector &buffers() { return bufferPool_.buffers(); } const StreamConfiguration &configuration() const { return configuration_; } - MemoryType memoryType() const { return memoryType_; } protected: friend class BufferAllocator; /* To allocate and release buffers. */ @@ -94,18 +82,7 @@ protected: virtual int start() = 0; virtual void stop() = 0; - int mapBuffer(const Buffer *buffer); - void unmapBuffer(const Buffer *buffer); - - void createBuffers(MemoryType memory, unsigned int count); - void destroyBuffers(); - - BufferPool bufferPool_; StreamConfiguration configuration_; - MemoryType memoryType_; - -private: - std::vector, unsigned int>> bufferCache_; }; } /* namespace libcamera */ diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 708cf95b0c3c25c2..a6da658bec277933 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -641,7 +641,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) StreamConfiguration *streamConfiguration = &config_->at(0); streamConfiguration->size.width = camera3Stream->width; streamConfiguration->size.height = camera3Stream->height; - streamConfiguration->memoryType = ExternalMemory; /* * \todo We'll need to translate from Android defined pixel format codes diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index ab28e0d76b8c40f7..5abb8589ea58e9c0 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -268,189 +268,6 @@ int Dmabuf::munmap() return ret; } -/** - * \class BufferMemory - * \brief A memory buffer to store an image - * - * The BufferMemory class represents the memory buffers used to store full frame - * images, which may contain multiple separate memory Plane objects if the - * image format is multi-planar. - */ - -/** - * \fn BufferMemory::planes() const - * \brief Retrieve the planes within the buffer - * \return A const reference to a vector holding all Planes within the buffer - */ - -/** - * \fn BufferMemory::planes() - * \brief Retrieve the planes within the buffer - * \return A reference to a vector holding all Planes within the buffer - */ - -/** - * \class BufferPool - * \brief A pool of buffers - * - * The BufferPool class groups together a collection of Buffers to store frames. - * The buffers must be exported by a device before they can be imported into - * another device for further use. - */ - -BufferPool::~BufferPool() -{ - destroyBuffers(); -} - -/** - * \brief Create buffers in the Pool - * \param[in] count The number of buffers to create - */ -void BufferPool::createBuffers(unsigned int count) -{ - buffers_.resize(count); -} - -/** - * \brief Release all buffers from pool - * - * If no buffers have been created or if buffers have already been released no - * operation is performed. - */ -void BufferPool::destroyBuffers() -{ - buffers_.resize(0); -} - -/** - * \fn BufferPool::count() - * \brief Retrieve the number of buffers contained within the pool - * \return The number of buffers contained in the pool - */ - -/** - * \fn BufferPool::buffers() - * \brief Retrieve all the buffers in the pool - * \return A vector containing all the buffers in the pool. - */ - -/** - * \class Buffer - * \brief A buffer handle and dynamic metadata - * - * The Buffer class references a buffer memory and associates dynamic metadata - * related to the frame contained in the buffer. It allows referencing buffer - * memory through a single interface regardless of whether the memory is - * allocated internally in libcamera or provided externally through dmabuf. - * - * Buffer instances are allocated dynamically for a stream through - * Stream::createBuffer(), added to a request with Request::addBuffer() and - * deleted automatically after the request complete handler returns. - */ - -/** - * \brief Construct a buffer not associated with any stream - * - * This method constructs an orphaned buffer not associated with any stream. It - * is not meant to be called by applications, they should instead create buffers - * for a stream with Stream::createBuffer(). - */ -Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr), - stream_(nullptr) -{ - unsigned int sequence; - uint64_t timestamp; - unsigned int bytesused; - - if (metadata) { - bytesused = metadata->info().planes()[0].bytesused; - sequence = metadata->info().sequence(); - timestamp = metadata->info().timestamp(); - } else { - bytesused = 0; - sequence = 0; - timestamp = 0; - } - - info_.update(BufferInfo::BufferSuccess, sequence, timestamp, - { { bytesused } }); -} - -/** - * \fn Buffer::index() - * \brief Retrieve the Buffer index - * \return The buffer index - */ - -/** - * \fn Buffer::dmabufs() - * \brief Retrieve the dmabuf file descriptors for all buffer planes - * - * The dmabufs array contains one dmabuf file descriptor per plane. Unused - * entries are set to -1. - * - * \return The dmabuf file descriptors - */ - -/** - * \fn Buffer::mem() - * \brief Retrieve the BufferMemory this buffer is associated with - * - * The association between the buffer and a BufferMemory instance is valid from - * the time the request containing this buffer is queued to a camera to the end - * of that request's completion handler. - * - * \return The BufferMemory this buffer is associated with - */ - -/** - * \fn Buffer::info() - * \brief Retrieve the buffer metadata information - * - * The buffer metadata information is update every time the buffer contained - * are changed, for example when it is dequeued from hardware. - * - * \return Metadata of the buffer - */ - -/** - * \fn Buffer::request() - * \brief Retrieve the request this buffer belongs to - * - * The intended callers of this method are buffer completion handlers that - * need to associate a buffer to the request it belongs to. - * - * A Buffer is associated to a request by Request::prepare() and the - * association is valid until the buffer completes. The returned request - * pointer is valid only during that interval. - * - * \return The Request the Buffer belongs to, or nullptr if the buffer is - * either completed or not associated with a request - */ - -/** - * \fn Buffer::stream() - * \brief Retrieve the stream this buffer is associated with - * - * A Buffer is associated to the stream that created it with - * Stream::createBuffer() and the association is valid until the buffer is - * destroyed. Buffer instances that are created directly are not associated - * with any stream. - * - * \return The Stream the Buffer is associated with, or nullptr if the buffer - * is not associated with a stream - */ - -/** - * \brief Mark a buffer as cancel by setting its status to BufferCancelled - */ -void Buffer::cancel() -{ - info_.update(BufferInfo::BufferCancelled, 0, 0, { {} }); -} - /** * \class FrameBuffer * \brief A buffer handle and dynamic metadata diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index ced3cb229c509ad2..1c299d988515298e 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -24,8 +24,6 @@ namespace libcamera { -class BufferMemory; -class BufferPool; class EventNotifier; class MediaDevice; class MediaEntity; @@ -161,8 +159,6 @@ public: int setFormat(V4L2DeviceFormat *format); ImageFormats formats(); - int exportBuffers(BufferPool *pool); - int importBuffers(BufferPool *pool); int allocateBuffers(unsigned int count, std::vector *buffers); int externalBuffers(unsigned int count); int releaseBuffers(); @@ -197,8 +193,6 @@ private: std::vector enumSizes(unsigned int pixelFormat); int requestBuffers(unsigned int count); - int createPlane(BufferMemory *buffer, unsigned int index, - unsigned int plane, unsigned int length); FrameBuffer *createBuffer(struct v4l2_buffer buf); int exportDmaBuffer(unsigned int index, unsigned int plane); @@ -210,7 +204,6 @@ private: enum v4l2_buf_type bufferType_; enum v4l2_memory memoryType_; - BufferPool *bufferPool_; V4L2BufferCache *cache_; std::map queuedBuffers_; diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index ba3f571b08cb0c41..c5a4cca658527ef8 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -266,17 +266,6 @@ SizeRange StreamFormats::range(PixelFormat pixelformat) const return range; } -/** - * \enum MemoryType - * \brief Define the memory type used by a Stream - * \var MemoryType::InternalMemory - * The Stream uses memory allocated internally by the library and exported to - * applications. - * \var MemoryType::ExternalMemory - * The Stream uses memory allocated externally by application and imported in - * the library. - */ - /** * \struct StreamConfiguration * \brief Configuration parameters for a stream @@ -290,7 +279,7 @@ SizeRange StreamFormats::range(PixelFormat pixelformat) const * handlers provied StreamFormats. */ StreamConfiguration::StreamConfiguration() - : pixelFormat(0), memoryType(InternalMemory), stream_(nullptr) + : pixelFormat(0), stream_(nullptr) { } @@ -298,8 +287,7 @@ StreamConfiguration::StreamConfiguration() * \brief Construct a configuration with stream formats */ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) - : pixelFormat(0), memoryType(InternalMemory), stream_(nullptr), - formats_(formats) + : pixelFormat(0), stream_(nullptr), formats_(formats) { } @@ -313,11 +301,6 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) * \brief Stream pixel format */ -/** - * \var StreamConfiguration::memoryType - * \brief The memory type the stream shall use - */ - /** * \var StreamConfiguration::bufferCount * \brief Requested number of buffers to allocate for the stream @@ -420,106 +403,12 @@ Stream::Stream() { } -/** - * \brief Create a Buffer instance referencing the memory buffer \a index - * \param[in] index The desired buffer index - * - * This method creates a Buffer instance that references a BufferMemory from - * the stream's buffers pool by its \a index. The index shall be lower than the - * number of buffers in the pool. - * - * This method is only valid for streams that use the InternalMemory type. It - * will return a null pointer when called on streams using the ExternalMemory - * type. - * - * \return A newly created Buffer on success or nullptr otherwise - */ -std::unique_ptr Stream::createBuffer(unsigned int index) -{ - if (memoryType_ != InternalMemory) { - LOG(Stream, Error) << "Invalid stream memory type"; - return nullptr; - } - - if (index >= bufferPool_.count()) { - LOG(Stream, Error) << "Invalid buffer index " << index; - return nullptr; - } - - Buffer *buffer = new Buffer(); - buffer->index_ = index; - buffer->stream_ = this; - - return std::unique_ptr(buffer); -} - -/** - * \brief Create a Buffer instance that represents a memory area identified by - * dmabuf file descriptors - * \param[in] fds The dmabuf file descriptors for each plane - * - * This method creates a Buffer instance that references buffer memory - * allocated outside of libcamera through dmabuf file descriptors. The \a - * dmabuf array shall contain a file descriptor for each plane in the buffer, - * and unused entries shall be set to -1. - * - * The buffer is created without a valid index, as it does not yet map to any of - * the stream's BufferMemory instances. An index will be assigned at the time - * the buffer is queued to the camera in a request. Applications may thus - * create any number of Buffer instances, providing that no more than the - * number of buffers allocated for the stream are queued at any given time. - * - * This method is only valid for streams that use the ExternalMemory type. It - * will return a null pointer when called on streams using the InternalMemory - * type. - * - * \sa Stream::mapBuffer() - * - * \return A newly created Buffer on success or nullptr otherwise - */ -std::unique_ptr Stream::createBuffer(const std::array &fds) -{ - if (memoryType_ != ExternalMemory) { - LOG(Stream, Error) << "Invalid stream memory type"; - return nullptr; - } - - Buffer *buffer = new Buffer(); - buffer->dmabuf_ = fds; - buffer->stream_ = this; - - return std::unique_ptr(buffer); -} - -/** - * \fn Stream::bufferPool() - * \brief Retrieve the buffer pool for the stream - * - * The buffer pool handles the memory buffers used to store frames for the - * stream. It is initially created empty and shall be populated with - * buffers before being used. - * - * \return A reference to the buffer pool - */ - -/** - * \fn Stream::buffers() - * \brief Retrieve the memory buffers in the Stream's buffer pool - * \return The list of stream's memory buffers - */ - /** * \fn Stream::configuration() * \brief Retrieve the active configuration of the stream * \return The active configuration of the stream */ -/** - * \fn Stream::memoryType() - * \brief Retrieve the stream memory type - * \return The memory type used by the stream - */ - /** * \fn Stream::allocateBuffers() * \brief Allocate buffers from the stream @@ -575,133 +464,6 @@ std::unique_ptr Stream::createBuffer(const std::array &fds) * implemeted by all subclasses of Stream. */ -/** - * \brief Map a Buffer to a buffer memory index - * \param[in] buffer The buffer to map to a buffer memory index - * - * Streams configured to use externally allocated memory need to maintain a - * best-effort association between the memory area the \a buffer represents - * and the associated buffer memory in the Stream's pool. - * - * The buffer memory to use, once the \a buffer reaches the video device, - * is selected using the index assigned to the \a buffer and to minimize - * relocations in the V4L2 back-end, this operation provides a best-effort - * caching mechanism that associates to the dmabuf file descriptors contained - * in the \a buffer the index of the buffer memory that was lastly queued with - * those file descriptors set. - * - * If the Stream uses internally allocated memory, the index of the memory - * buffer to use will match the one request at Stream::createBuffer(unsigned int) - * time, and no mapping is thus required. - * - * \return The buffer memory index for the buffer on success, or a negative - * error code otherwise - * \retval -ENOMEM No buffer memory was available to map the buffer - */ -int Stream::mapBuffer(const Buffer *buffer) -{ - ASSERT(memoryType_ == ExternalMemory); - - if (bufferCache_.empty()) - return -ENOMEM; - - const std::array &dmabufs = buffer->dmabufs(); - - /* - * Try to find a previously mapped buffer in the cache. If we miss, use - * the oldest entry in the cache. - */ - auto map = std::find_if(bufferCache_.begin(), bufferCache_.end(), - [&](std::pair, unsigned int> &entry) { - return entry.first == dmabufs; - }); - if (map == bufferCache_.end()) - map = bufferCache_.begin(); - - /* - * Update the dmabuf file descriptors of the entry. We can't assume that - * identical file descriptor numbers refer to the same dmabuf object as - * it may have been closed and its file descriptor reused. We thus need - * to update the plane's internally cached mmap()ed memory. - */ - unsigned int index = map->second; - BufferMemory *mem = &bufferPool_.buffers()[index]; - mem->planes().clear(); - - for (unsigned int i = 0; i < dmabufs.size(); ++i) { - if (dmabufs[i] == -1) - break; - - mem->planes().emplace_back(dmabufs[i], 0); - } - - /* Remove the buffer from the cache and return its index. */ - bufferCache_.erase(map); - return index; -} - -/** - * \brief Unmap a Buffer from its buffer memory - * \param[in] buffer The buffer to unmap - * - * This method releases the buffer memory entry that was mapped by mapBuffer(), - * making it available for new mappings. - */ -void Stream::unmapBuffer(const Buffer *buffer) -{ - ASSERT(memoryType_ == ExternalMemory); - - bufferCache_.emplace_back(buffer->dmabufs(), buffer->index()); -} - -/** - * \brief Create buffers for the stream - * \param[in] count The number of buffers to create - * \param[in] memory The stream memory type - * - * Create \a count empty buffers in the Stream's buffer pool. - */ -void Stream::createBuffers(MemoryType memory, unsigned int count) -{ - destroyBuffers(); - if (count == 0) - return; - - memoryType_ = memory; - bufferPool_.createBuffers(count); - - /* Streams with internal memory usage do not need buffer mapping. */ - if (memoryType_ == InternalMemory) - return; - - /* - * Prepare for buffer mapping by adding all buffer memory entries to the - * cache. - */ - bufferCache_.clear(); - for (unsigned int i = 0; i < bufferPool_.count(); ++i) - bufferCache_.emplace_back(std::array{ -1, -1, -1 }, i); -} - -/** - * \brief Destroy buffers in the stream - * - * If no buffers have been created or if buffers have already been destroyed no - * operation is performed. - */ -void Stream::destroyBuffers() -{ - bufferPool_.destroyBuffers(); -} - -/** - * \var Stream::bufferPool_ - * \brief The pool of buffers associated with the stream - * - * The stream buffer pool is populated by the Camera class after a successful - * stream configuration. - */ - /** * \var Stream::configuration_ * \brief The stream configuration @@ -711,9 +473,4 @@ void Stream::destroyBuffers() * next call to Camera::configure() regardless of if it includes the stream. */ -/** - * \var Stream::memoryType_ - * \brief The stream memory type - */ - } /* namespace libcamera */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index a29ed697468a7f59..787cf8c0477b808c 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -377,8 +377,7 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), bufferPool_(nullptr), cache_(nullptr), - fdEvent_(nullptr) + : V4L2Device(deviceNode), cache_(nullptr), fdEvent_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -937,112 +936,6 @@ int V4L2VideoDevice::requestBuffers(unsigned int count) return 0; } -/** - * \brief Request buffers to be allocated from the video device and stored in - * the buffer pool provided. - * \param[out] pool BufferPool to populate with buffers - * \return 0 on success or a negative error code otherwise - */ -int V4L2VideoDevice::exportBuffers(BufferPool *pool) -{ - unsigned int i; - int ret; - - memoryType_ = V4L2_MEMORY_MMAP; - - ret = requestBuffers(pool->count()); - if (ret) - return ret; - - /* Map the buffers. */ - for (i = 0; i < pool->count(); ++i) { - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; - struct v4l2_buffer buf = {}; - BufferMemory &buffer = pool->buffers()[i]; - - buf.index = i; - buf.type = bufferType_; - buf.memory = memoryType_; - buf.length = VIDEO_MAX_PLANES; - buf.m.planes = planes; - - ret = ioctl(VIDIOC_QUERYBUF, &buf); - if (ret < 0) { - LOG(V4L2, Error) - << "Unable to query buffer " << i << ": " - << strerror(-ret); - break; - } - - if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) { - for (unsigned int p = 0; p < buf.length; ++p) { - ret = createPlane(&buffer, i, p, - buf.m.planes[p].length); - if (ret) - break; - } - } else { - ret = createPlane(&buffer, i, 0, buf.length); - } - - if (ret) { - LOG(V4L2, Error) << "Failed to create plane"; - break; - } - } - - if (ret) { - requestBuffers(0); - pool->destroyBuffers(); - return ret; - } - - bufferPool_ = pool; - - return 0; -} - -int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, - unsigned int planeIndex, unsigned int length) -{ - int fd; - - LOG(V4L2, Debug) - << "Buffer " << index - << " plane " << planeIndex - << ": length=" << length; - - fd = exportDmaBuffer(index, planeIndex); - if (fd < 0) - return fd; - - buffer->planes().emplace_back(fd, length); - ::close(fd); - - return 0; -} - -/** - * \brief Import the externally allocated \a pool of buffers - * \param[in] pool BufferPool of buffers to import - * \return 0 on success or a negative error code otherwise - */ -int V4L2VideoDevice::importBuffers(BufferPool *pool) -{ - int ret; - - memoryType_ = V4L2_MEMORY_DMABUF; - - ret = requestBuffers(pool->count()); - if (ret) - return ret; - - LOG(V4L2, Debug) << "provided pool of " << pool->count() << " buffers"; - bufferPool_ = pool; - - return 0; -} - /** * \brief Operate using buffers allocated from local video device * \param[in] count Number of buffers to allocate @@ -1193,7 +1086,6 @@ int V4L2VideoDevice::releaseBuffers() { LOG(V4L2, Debug) << "Releasing buffers"; - bufferPool_ = nullptr; delete cache_; cache_ = nullptr; From patchwork Tue Nov 26 23:36:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2377 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C96260C3D for ; Wed, 27 Nov 2019 00:39:52 +0100 (CET) X-Halon-ID: 099e7011-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 099e7011-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:50 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:19 +0100 Message-Id: <20191126233620.1695316-30-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 29/30] libcamera: pipeline: Remove explicit buffer handling 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:52 -0000 With FrameBuffer interface in place there is no need for the Camera to call into the specific pipelines allocation and freeing of buffers as it no longer needs to be synced with buffer allocation by the application. Remove the function prototypes in the pipeline handler base class and fold the functionality in the pipelines start() and stop() functions where needed. A follow up patch will remove the now no-op Camera::allocateBuffers() and Camera::freeBuffers(). Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 8 +------ src/libcamera/include/pipeline_handler.h | 5 ---- src/libcamera/pipeline/ipu3/ipu3.cpp | 21 +++++++++-------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 21 ++++++++++------- src/libcamera/pipeline/uvcvideo.cpp | 17 -------------- src/libcamera/pipeline/vimc.cpp | 17 -------------- src/libcamera/pipeline_handler.cpp | 29 ------------------------ 7 files changed, 26 insertions(+), 92 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 05fdcaab8f918afa..12878a1c2e6076d3 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -709,12 +709,6 @@ int Camera::allocateBuffers() return -EINVAL; } - int ret = pipe_->allocateBuffers(this, activeStreams_); - if (ret) { - LOG(Camera, Error) << "Failed to allocate buffers"; - return ret; - } - state_ = CameraPrepared; return 0; @@ -735,7 +729,7 @@ int Camera::freeBuffers() state_ = CameraConfigured; - return pipe_->freeBuffers(this, activeStreams_); + return 0; } /** diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index e4588f10bac0228c..9e5dca894aff45ae 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -69,11 +69,6 @@ public: const StreamRoles &roles) = 0; virtual int configure(Camera *camera, CameraConfiguration *config) = 0; - virtual int allocateBuffers(Camera *camera, - const std::set &streams) = 0; - virtual int freeBuffers(Camera *camera, - const std::set &streams) = 0; - virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8d9420aea3eb0b21..6d6ecb511777ec49 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -215,10 +215,8 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; + int allocateBuffers(Camera *camera); + int freeBuffers(Camera *camera); int start(Camera *camera) override; void stop(Camera *camera) override; @@ -629,8 +627,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) * In order to be able to start the 'viewfinder' and 'stat' nodes, we need * memory to be reserved. */ -int PipelineHandlerIPU3::allocateBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); unsigned int bufferCount; @@ -670,13 +667,12 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, return 0; error: - freeBuffers(camera, streams); + freeBuffers(camera); return ret; } -int PipelineHandlerIPU3::freeBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerIPU3::freeBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); @@ -693,6 +689,10 @@ int PipelineHandlerIPU3::start(Camera *camera) ImgUDevice *imgu = data->imgu_; int ret; + ret = allocateBuffers(camera); + if (ret) + return ret; + /* * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. @@ -711,6 +711,7 @@ int PipelineHandlerIPU3::start(Camera *camera) return 0; error: + freeBuffers(camera); LOG(IPU3, Error) << "Failed to start camera " << camera->name(); return ret; @@ -726,6 +727,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); + + freeBuffers(camera); } int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 96e863f0208fa748..a56c7022f57ce771 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -174,10 +174,8 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; + int allocateBuffers(Camera *camera); + int freeBuffers(Camera *camera); int start(Camera *camera) override; void stop(Camera *camera) override; @@ -651,8 +649,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) return 0; } -int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); std::vector paramBuffers, statBuffers; @@ -695,8 +692,7 @@ err: return ret; } -int PipelineHandlerRkISP1::freeBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerRkISP1::freeBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); @@ -731,10 +727,15 @@ int PipelineHandlerRkISP1::start(Camera *camera) RkISP1CameraData *data = cameraData(camera); int ret; + ret = allocateBuffers(camera); + if (ret) + return ret; + data->frame_ = 0; ret = param_->streamOn(); if (ret) { + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start parameters " << camera->name(); return ret; @@ -743,6 +744,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) ret = stat_->streamOn(); if (ret) { param_->streamOff(); + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start statistics " << camera->name(); return ret; @@ -752,6 +754,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) { param_->streamOff(); stat_->streamOff(); + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start camera " << camera->name(); @@ -796,6 +799,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) data->timeline_.reset(); + freeBuffers(camera); + activeCamera_ = nullptr; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index c275b9f0f75429bd..2be7864ef59e932b 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -65,11 +65,6 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -193,18 +188,6 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) return 0; } -int PipelineHandlerUVC::allocateBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - -int PipelineHandlerUVC::freeBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - int PipelineHandlerUVC::start(Camera *camera) { UVCCameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 64793788c752c791..cacf8bfe3ff9cc45 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -84,11 +84,6 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -261,18 +256,6 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) return 0; } -int PipelineHandlerVimc::allocateBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - -int PipelineHandlerVimc::freeBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - int PipelineHandlerVimc::start(Camera *camera) { VimcCameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index b107e23258dee692..67758f3f1ebc39e5 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -287,35 +287,6 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * \return 0 on success or a negative error code otherwise */ -/** - * \fn PipelineHandler::allocateBuffers() - * \brief Allocate buffers for a stream - * \param[in] camera The camera the \a stream belongs to - * \param[in] streams The set of streams to allocate buffers for - * - * This method allocates buffers internally in the pipeline handler for each - * stream in the \a streams buffer set, and associates them with the stream's - * buffer pool. - * - * The intended caller of this method is the Camera class. - * - * \return 0 on success or a negative error code otherwise - */ - -/** - * \fn PipelineHandler::freeBuffers() - * \brief Free all buffers associated with a stream - * \param[in] camera The camera the \a stream belongs to - * \param[in] streams The set of streams to free buffers from - * - * After a capture session has been stopped all buffers associated with each - * stream shall be freed. - * - * The intended caller of this method is the Camera class. - * - * \return 0 on success or a negative error code otherwise - */ - /** * \fn PipelineHandler::start() * \brief Start capturing from a group of streams From patchwork Tue Nov 26 23:36:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2378 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 419DC6136B for ; Wed, 27 Nov 2019 00:39:53 +0100 (CET) X-Halon-ID: 0a2f0fc4-10a6-11ea-a0b9-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 0a2f0fc4-10a6-11ea-a0b9-005056917f90; Wed, 27 Nov 2019 00:39:51 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Nov 2019 00:36:20 +0100 Message-Id: <20191126233620.1695316-31-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> References: <20191126233620.1695316-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 30/30] libcamera: camera: Remove the prepared state 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: , X-List-Received-Date: Tue, 26 Nov 2019 23:39:53 -0000 With the FrameBuffer rework completed there is no reason to keep the camera prepared state around as buffer allocations are now decoupled from the camera state. Remove the camera state simplifying the API. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/camera.h | 4 -- src/android/camera_device.cpp | 10 +--- src/cam/capture.cpp | 8 --- src/libcamera/camera.cpp | 104 ++++++++-------------------------- src/qcam/main_window.cpp | 9 --- test/camera/buffer_import.cpp | 10 ---- test/camera/capture.cpp | 10 ---- test/camera/statemachine.cpp | 83 --------------------------- 8 files changed, 26 insertions(+), 212 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 02d047b9649f53d0..9f542aab213c5d47 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -90,9 +90,6 @@ public: std::unique_ptr generateConfiguration(const StreamRoles &roles); int configure(CameraConfiguration *config); - int allocateBuffers(); - int freeBuffers(); - Request *createRequest(uint64_t cookie = 0); int queueRequest(Request *request); @@ -104,7 +101,6 @@ private: CameraAvailable, CameraAcquired, CameraConfigured, - CameraPrepared, CameraRunning, }; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index a6da658bec277933..34c6119228a9ac0e 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -78,7 +78,6 @@ void CameraDevice::close() { camera_->stop(); - camera_->freeBuffers(); camera_->release(); running_ = false; @@ -690,16 +689,9 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque /* Start the camera if that's the first request we handle. */ if (!running_) { - int ret = camera_->allocateBuffers(); - if (ret) { - LOG(HAL, Error) << "Failed to allocate buffers"; - return; - } - - ret = camera_->start(); + int ret = camera_->start(); if (ret) { LOG(HAL, Error) << "Failed to start camera"; - camera_->freeBuffers(); return; } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 154e6e461ee2b96b..835943a7d2b82e79 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -42,12 +42,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) return ret; } - ret = camera_->allocateBuffers(); - if (ret) { - std::cerr << "Failed to allocate buffers" << std::endl; - return ret; - } - camera_->requestCompleted.connect(this, &Capture::requestComplete); if (options.isSet(OptFile)) { @@ -67,8 +61,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) writer_ = nullptr; } - camera_->freeBuffers(); - return ret; } diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 12878a1c2e6076d3..fca75040ad8b16e7 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -274,15 +274,13 @@ std::size_t CameraConfiguration::size() const * \section camera_operation Operating the Camera * * An application needs to perform a sequence of operations on a camera before - * it is ready to process requests. The camera needs to be acquired, configured - * and resources allocated or imported to prepare the camera for capture. Once - * started the camera can process requests until it is stopped. When an - * application is done with a camera all resources allocated need to be freed - * and the camera released. + * it is ready to process requests. The camera needs to be acquired and + * configured to prepare the camera for capture. Once started the camera can + * process requests until it is stopped. When an application is done with a + * camera it needs to be released. * * An application may start and stop a camera multiple times as long as it is - * not released. The camera may also be reconfigured provided that all - * resources allocated are freed prior to the reconfiguration. + * not released. The camera may also be reconfigured. * * \subsection Camera States * @@ -296,7 +294,6 @@ std::size_t CameraConfiguration::size() const * node [shape = doublecircle ]; Available; * node [shape = circle ]; Acquired; * node [shape = circle ]; Configured; - * node [shape = circle ]; Prepared; * node [shape = circle ]; Running; * * Available -> Available [label = "release()"]; @@ -306,14 +303,10 @@ std::size_t CameraConfiguration::size() const * Acquired -> Configured [label = "configure()"]; * * Configured -> Available [label = "release()"]; - * Configured -> Configured [label = "configure()"]; - * Configured -> Prepared [label = "allocateBuffers()"]; + * Configured -> Configured [label = "configure(), createRequest()"]; + * Configured -> Running [label = "start()"]; * - * Prepared -> Configured [label = "freeBuffers()"]; - * Prepared -> Prepared [label = "createRequest()"]; - * Prepared -> Running [label = "start()"]; - * - * Running -> Prepared [label = "stop()"]; + * Running -> Configured [label = "stop()"]; * Running -> Running [label = "createRequest(), queueRequest()"]; * } * \enddot @@ -329,19 +322,14 @@ std::size_t CameraConfiguration::size() const * Configured state. * * \subsubsection Configured - * The camera is configured and ready for the application to prepare it with - * resources. The camera may be reconfigured multiple times until resources - * are provided and the state progresses to Prepared. - * - * \subsubsection Prepared - * The camera has been configured and provided with resources and is ready to be - * started. The application may free the camera's resources to get back to the - * Configured state or start() it to progress to the Running state. + * The camera has been configured and is ready to be started. The application + * may release() the camera and to get back to the Available state or start() + * it to progress to the Running state. * * \subsubsection Running * The camera is running and ready to process requests queued by the * application. The camera remains in this state until it is stopped and moved - * to the Prepared state. + * to the Configured state. */ /** @@ -419,7 +407,6 @@ static const char *const camera_state_names[] = { "Available", "Acquired", "Configured", - "Prepared", "Running", }; @@ -473,11 +460,11 @@ void Camera::disconnect() /* * If the camera was running when the hardware was removed force the - * state to Prepared to allow applications to call freeBuffers() and - * release() before deleting the camera. + * state to Configured state to allow applications to free resources + * and release() before deleting the camera. */ if (state_ == CameraRunning) - state_ = CameraPrepared; + state_ = CameraConfigured; disconnected_ = true; disconnected.emit(this); @@ -685,53 +672,6 @@ int Camera::configure(CameraConfiguration *config) return 0; } -/** - * \brief Allocate buffers for all configured streams - * - * This function affects the state of the camera, see \ref camera_operation. - * - * \return 0 on success or a negative error code otherwise - * \retval -ENODEV The camera has been disconnected from the system - * \retval -EACCES The camera is not in a state where buffers can be allocated - * \retval -EINVAL The configuration is not valid - */ -int Camera::allocateBuffers() -{ - if (disconnected_) - return -ENODEV; - - if (!stateIs(CameraConfigured)) - return -EACCES; - - if (activeStreams_.empty()) { - LOG(Camera, Error) - << "Can't allocate buffers without streams"; - return -EINVAL; - } - - state_ = CameraPrepared; - - return 0; -} - -/** - * \brief Release all buffers from allocated pools in each stream - * - * This function affects the state of the camera, see \ref camera_operation. - * - * \return 0 on success or a negative error code otherwise - * \retval -EACCES The camera is not in a state where buffers can be freed - */ -int Camera::freeBuffers() -{ - if (!stateIs(CameraPrepared)) - return -EACCES; - - state_ = CameraConfigured; - - return 0; -} - /** * \brief Create a request object for the camera * \param[in] cookie Opaque cookie for application use @@ -747,14 +687,14 @@ int Camera::freeBuffers() * The ownership of the returned request is passed to the caller, which is * responsible for either queueing the request or deleting it. * - * This function shall only be called when the camera is in the Prepared + * This function shall only be called when the camera is in the Configured * or Running state, see \ref camera_operation. * * \return A pointer to the newly created request, or nullptr on error */ Request *Camera::createRequest(uint64_t cookie) { - if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning)) + if (disconnected_ || !stateBetween(CameraConfigured, CameraRunning)) return nullptr; return new Request(this, cookie); @@ -828,9 +768,15 @@ int Camera::start() if (disconnected_) return -ENODEV; - if (!stateIs(CameraPrepared)) + if (!stateIs(CameraConfigured)) return -EACCES; + if (activeStreams_.empty()) { + LOG(Camera, Error) + << "Can't start camera without streams"; + return -EINVAL; + } + for (Stream *stream : activeStreams_) { ret = stream->start(); if (ret < 0) @@ -870,7 +816,7 @@ int Camera::stop() LOG(Camera, Debug) << "Stopping capture"; - state_ = CameraPrepared; + state_ = CameraConfigured; pipe_->stop(this); diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 9650bd970f89aa41..38cf79deed37a089 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -172,13 +172,6 @@ int MainWindow::startCapture() adjustSize(); - ret = camera_->allocateBuffers(); - if (ret) { - std::cerr << "Failed to allocate buffers" - << std::endl; - return ret; - } - ret = allocator_->allocate(stream, cfg); if (ret < 0) { std::cerr << "Failed to allocate internal buffers" << std::endl; @@ -230,7 +223,6 @@ error: for (Request *request : requests) delete request; - camera_->freeBuffers(); return ret; } @@ -243,7 +235,6 @@ void MainWindow::stopCapture() if (ret) std::cout << "Failed to stop capture" << std::endl; - camera_->freeBuffers(); isCapturing_ = false; config_.reset(); diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index c98afeb9597e21c2..9ae364d44d63b4d5 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -174,11 +174,6 @@ protected: return TestFail; } - if (camera_->allocateBuffers()) { - cout << "Failed to allocate buffers" << endl; - return TestFail; - } - Stream *stream = cfg.stream(); BufferSource source; @@ -246,11 +241,6 @@ protected: return TestFail; } - if (camera_->freeBuffers()) { - cout << "Failed to free buffers" << endl; - return TestFail; - } - return TestPass; } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index ee6d0a1bc1fa839d..545496fe2f7655ea 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -80,11 +80,6 @@ protected: return TestFail; } - if (camera_->allocateBuffers()) { - cout << "Failed to allocate buffers" << endl; - return TestFail; - } - Stream *stream = cfg.stream(); BufferAllocator allocator(camera_); @@ -152,11 +147,6 @@ protected: return TestFail; } - if (camera_->freeBuffers()) { - cout << "Failed to free buffers" << endl; - return TestFail; - } - return TestPass; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index e9468d806f977a63..7d409ca20689568b 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -29,12 +29,6 @@ protected: if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->createRequest()) return TestFail; @@ -65,12 +59,6 @@ protected: if (camera_->acquire() != -EBUSY) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->createRequest()) return TestFail; @@ -103,57 +91,6 @@ protected: if (camera_->acquire() != -EBUSY) return TestFail; - if (camera_->freeBuffers() != -EACCES) - return TestFail; - - if (camera_->createRequest()) - return TestFail; - - Request request(camera_.get()); - if (camera_->queueRequest(&request) != -EACCES) - return TestFail; - - if (camera_->start() != -EACCES) - return TestFail; - - if (camera_->stop() != -EACCES) - return TestFail; - - /* Test operations which should pass. */ - if (camera_->configure(defconf_.get())) - return TestFail; - - /* Test valid state transitions, end in Prepared state. */ - if (camera_->release()) - return TestFail; - - if (camera_->acquire()) - return TestFail; - - if (camera_->configure(defconf_.get())) - return TestFail; - - if (camera_->allocateBuffers()) - return TestFail; - - return TestPass; - } - - int testPrepared() - { - /* Test operations which should fail. */ - if (camera_->acquire() != -EBUSY) - return TestFail; - - if (camera_->release() != -EBUSY) - return TestFail; - - if (camera_->configure(defconf_.get()) != -EACCES) - return TestFail; - - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - Request request1(camera_.get()); if (camera_->queueRequest(&request1) != -EACCES) return TestFail; @@ -170,9 +107,6 @@ protected: delete request2; /* Test valid state transitions, end in Running state. */ - if (camera_->freeBuffers()) - return TestFail; - if (camera_->release()) return TestFail; @@ -182,9 +116,6 @@ protected: if (camera_->configure(defconf_.get())) return TestFail; - if (camera_->allocateBuffers()) - return TestFail; - /* Use internally allocated buffers. */ allocator_ = new BufferAllocator(camera_); Stream *stream = *camera_->streams().begin(); @@ -209,12 +140,6 @@ protected: if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->start() != -EACCES) return TestFail; @@ -236,9 +161,6 @@ protected: delete allocator_; - if (camera_->freeBuffers()) - return TestFail; - if (camera_->release()) return TestFail; @@ -276,11 +198,6 @@ protected: return TestFail; } - if (testPrepared() != TestPass) { - cout << "State machine in Prepared state failed" << endl; - return TestFail; - } - if (testRuning() != TestPass) { cout << "State machine in Running state failed" << endl; return TestFail;