From patchwork Wed Sep 23 12:14:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 9745 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 89E80C3B5B for ; Wed, 23 Sep 2020 12:15:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 197F162FE3; Wed, 23 Sep 2020 14:15:07 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Sr1kcb2L"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 098D160576 for ; Wed, 23 Sep 2020 14:15:06 +0200 (CEST) Received: from pyrite.rasen.tech (unknown [IPv6:2400:4051:61:600:2c71:1b79:d06d:5032]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6CB582FD; Wed, 23 Sep 2020 14:15:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1600863303; bh=XTjRIo3j1jveIN4lO+aSEXShPqPi4gnfRziJDeOF0oE=; h=From:To:Cc:Subject:Date:From; b=Sr1kcb2Lz6k2Nbc4iGrNIYmV8dI35zvg+zWjJeU2COzVks9O6Gc2Y8k3Ki1V3i72/ Ppw7UP65rDVq1TqfEtuHgjhPbQH68e1kk9ag13cF5zY6V/qs9WO3QXu9ivjOEP29n5 uo3Zgp9/M2im/WYf7H33kkYIKhtPzhVoK0k4MjiQ= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Wed, 23 Sep 2020 21:14:48 +0900 Message-Id: <20200923121448.281346-1-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH] libcamera, cam, qcam: Reuse Request 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Allow reuse of the Request object by implementing a reset() function. This means that the applications now have the responsibility of freeing the the Request objects, so make cam and qcam do so. Signed-off-by: Paul Elder --- In this RFC, I've only made qcam and cam reuse the Request object. Just gauging the direction, and then I'll put it in for andoird and gstreamer and v4l2-compat as well. --- include/libcamera/request.h | 2 ++ src/cam/capture.cpp | 20 ++++++++++---------- src/cam/capture.h | 3 +++ src/libcamera/camera.cpp | 4 +--- src/libcamera/request.cpp | 9 +++++++++ src/qcam/main_window.cpp | 26 ++++++++++++-------------- src/qcam/main_window.h | 14 ++++++++++---- src/qcam/viewfinder.h | 4 +++- src/qcam/viewfinder_gl.cpp | 9 +++++---- src/qcam/viewfinder_gl.h | 5 +++-- src/qcam/viewfinder_qt.cpp | 10 ++++++---- src/qcam/viewfinder_qt.h | 8 ++++++-- 12 files changed, 70 insertions(+), 44 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 5976ac50..9f615a24 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -38,6 +38,8 @@ public: Request &operator=(const Request &) = delete; ~Request(); + void reset(); + ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } const BufferMap &buffers() const { return bufferMap_; } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 5510c009..fe9d591a 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -65,6 +65,9 @@ int Capture::run(const OptionsParser::Options &options) writer_ = nullptr; } + for (Request *req : requests_) + delete req; + delete allocator; return ret; @@ -92,7 +95,6 @@ int Capture::capture(FrameBufferAllocator *allocator) * example pushing a button. For now run all streams all the time. */ - std::vector requests; for (unsigned int i = 0; i < nbuffers; i++) { Request *request = camera_->createRequest(); if (!request) { @@ -117,7 +119,7 @@ int Capture::capture(FrameBufferAllocator *allocator) writer_->mapBuffer(buffer.get()); } - requests.push_back(request); + requests_.push_back(request); } ret = camera_->start(); @@ -126,7 +128,7 @@ int Capture::capture(FrameBufferAllocator *allocator) return ret; } - for (Request *request : requests) { + for (Request *request : requests_) { ret = camera_->queueRequest(request); if (ret < 0) { std::cerr << "Can't queue request" << std::endl; @@ -153,10 +155,12 @@ int Capture::capture(FrameBufferAllocator *allocator) void Capture::requestComplete(Request *request) { - if (request->status() == Request::RequestCancelled) + if (request->status() == Request::RequestCancelled) { + request->reset(); return; + } - const Request::BufferMap &buffers = request->buffers(); + const Request::BufferMap buffers = request->buffers(); /* * Compute the frame rate. The timestamp is arbitrarily retrieved from @@ -206,11 +210,7 @@ void Capture::requestComplete(Request *request) * Create a new request and populate it with one buffer for each * stream. */ - request = camera_->createRequest(); - if (!request) { - std::cerr << "Can't create request" << std::endl; - return; - } + request->reset(); for (auto it = buffers.begin(); it != buffers.end(); ++it) { const Stream *stream = it->first; diff --git a/src/cam/capture.h b/src/cam/capture.h index 0aebdac9..62cf9bf1 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -43,6 +44,8 @@ private: EventLoop *loop_; unsigned int captureCount_; unsigned int captureLimit_; + + std::vector requests_; }; #endif /* __CAM_CAPTURE_H__ */ diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index ae16a64a..4cc68bce 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -974,13 +974,11 @@ int Camera::stop() * \param[in] request The request that has completed * * This function is called by the pipeline handler to notify the camera that - * the request has completed. It emits the requestCompleted signal and deletes - * the request. + * the request has completed. It emits the requestCompleted signal. */ void Camera::requestComplete(Request *request) { requestCompleted.emit(request); - delete request; } } /* namespace libcamera */ diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 60b30692..606866a6 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -85,6 +85,15 @@ Request::~Request() delete validator_; } +void Request::reset() +{ + bufferMap_.clear(); + pending_.clear(); + + status_ = RequestPending; + cancelled_ = false; +} + /** * \fn Request::controls() * \brief Retrieve the request's ControlList diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 985743f3..1531b44f 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -367,7 +367,6 @@ void MainWindow::toggleCapture(bool start) int MainWindow::startCapture() { StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]); - std::vector requests; int ret; /* Verify roles are supported. */ @@ -499,7 +498,7 @@ int MainWindow::startCapture() goto error; } - requests.push_back(request); + requests_.push_back(request); } /* Start the title timer and the camera. */ @@ -518,7 +517,7 @@ int MainWindow::startCapture() camera_->requestCompleted.connect(this, &MainWindow::requestComplete); /* Queue all requests. */ - for (Request *request : requests) { + for (Request *request : requests_) { ret = camera_->queueRequest(request); if (ret < 0) { qWarning() << "Can't queue request"; @@ -535,7 +534,7 @@ error_disconnect: camera_->stop(); error: - for (Request *request : requests) + for (Request *request : requests_) delete request; for (auto &iter : mappedBuffers_) { @@ -580,6 +579,9 @@ void MainWindow::stopCapture() } mappedBuffers_.clear(); + for (Request *req : requests_) + delete req; + delete allocator_; isCapturing_ = false; @@ -701,7 +703,7 @@ void MainWindow::requestComplete(Request *request) */ { QMutexLocker locker(&mutex_); - doneQueue_.enqueue({ request->buffers(), request->metadata() }); + doneQueue_.enqueue({ request->buffers(), request->metadata(), request }); } QCoreApplication::postEvent(this, new CaptureEvent); @@ -726,13 +728,13 @@ void MainWindow::processCapture() /* Process buffers. */ if (request.buffers_.count(vfStream_)) - processViewfinder(request.buffers_[vfStream_]); + processViewfinder(request.request_, request.buffers_[vfStream_]); if (request.buffers_.count(rawStream_)) processRaw(request.buffers_[rawStream_], request.metadata_); } -void MainWindow::processViewfinder(FrameBuffer *buffer) +void MainWindow::processViewfinder(Request *request, FrameBuffer *buffer) { framesCaptured_++; @@ -749,16 +751,12 @@ void MainWindow::processViewfinder(FrameBuffer *buffer) << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; /* Render the frame on the viewfinder. */ - viewfinder_->render(buffer, &mappedBuffers_[buffer]); + viewfinder_->render(request, buffer, &mappedBuffers_[buffer]); } -void MainWindow::queueRequest(FrameBuffer *buffer) +void MainWindow::queueRequest(Request *request, FrameBuffer *buffer) { - Request *request = camera_->createRequest(); - if (!request) { - qWarning() << "Can't create request"; - return; - } + request->reset(); request->addBuffer(vfStream_, buffer); diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 5c61a4df..83535373 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -8,6 +8,7 @@ #define __QCAM_MAIN_WINDOW_H__ #include +#include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include #include "../cam/stream_options.h" @@ -49,13 +51,15 @@ public: } CaptureRequest(const Request::BufferMap &buffers, - const ControlList &metadata) - : buffers_(buffers), metadata_(metadata) + const ControlList &metadata, + Request * const request) + : buffers_(buffers), metadata_(metadata), request_(request) { } Request::BufferMap buffers_; ControlList metadata_; + Request *request_; }; class MainWindow : public QMainWindow @@ -79,7 +83,7 @@ private Q_SLOTS: void captureRaw(); void processRaw(FrameBuffer *buffer, const ControlList &metadata); - void queueRequest(FrameBuffer *buffer); + void queueRequest(Request *request, FrameBuffer *buffer); private: int createToolbars(); @@ -96,7 +100,7 @@ private: void requestComplete(Request *request); void processCapture(); void processHotplug(HotplugEvent *e); - void processViewfinder(FrameBuffer *buffer); + void processViewfinder(Request *request, FrameBuffer *buffer); /* UI elements */ QToolBar *toolbar_; @@ -135,6 +139,8 @@ private: QElapsedTimer frameRateInterval_; uint32_t previousFrames_; uint32_t framesCaptured_; + + std::vector requests_; }; #endif /* __QCAM_MAIN_WINDOW__ */ diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h index 67da1df2..26c20f1b 100644 --- a/src/qcam/viewfinder.h +++ b/src/qcam/viewfinder.h @@ -27,7 +27,9 @@ public: virtual const QList &nativeFormats() const = 0; virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0; - virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0; + virtual void render(libcamera::Request *request, + libcamera::FrameBuffer *buffer, + MappedBuffer *map) = 0; virtual void stop() = 0; virtual QImage getCurrentImage() = 0; diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp index fbe21dcf..c65b41f6 100644 --- a/src/qcam/viewfinder_gl.cpp +++ b/src/qcam/viewfinder_gl.cpp @@ -23,7 +23,7 @@ static const QList supportedFormats{ }; ViewFinderGL::ViewFinderGL(QWidget *parent) - : QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr), + : QOpenGLWidget(parent), request_(nullptr), buffer_(nullptr), yuvData_(nullptr), fragmentShader_(nullptr), vertexShader_(nullptr), vertexBuffer_(QOpenGLBuffer::VertexBuffer), textureU_(QOpenGLTexture::Target2D), @@ -67,7 +67,7 @@ int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, void ViewFinderGL::stop() { if (buffer_) { - renderComplete(buffer_); + renderComplete(request_, buffer_); buffer_ = nullptr; } } @@ -79,7 +79,7 @@ QImage ViewFinderGL::getCurrentImage() return grabFramebuffer(); } -void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) +void ViewFinderGL::render(libcamera::Request *request, libcamera::FrameBuffer *buffer, MappedBuffer *map) { if (buffer->planes().size() != 1) { qWarning() << "Multi-planar buffers are not supported"; @@ -87,11 +87,12 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) } if (buffer_) - renderComplete(buffer_); + renderComplete(request_, buffer_); yuvData_ = static_cast(map->memory); update(); buffer_ = buffer; + request_ = request; } bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h index 69502b7a..7db41841 100644 --- a/src/qcam/viewfinder_gl.h +++ b/src/qcam/viewfinder_gl.h @@ -36,13 +36,13 @@ public: const QList &nativeFormats() const override; int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; + void render(libcamera::Request *request, libcamera::FrameBuffer *buffer, MappedBuffer *map) override; void stop() override; QImage getCurrentImage() override; Q_SIGNALS: - void renderComplete(libcamera::FrameBuffer *buffer); + void renderComplete(libcamera::Request *request, libcamera::FrameBuffer *buffer); protected: void initializeGL() override; @@ -60,6 +60,7 @@ private: void doRender(); /* Captured image size, format and buffer */ + libcamera::Request *request_; libcamera::FrameBuffer *buffer_; libcamera::PixelFormat format_; QSize size_; diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp index e436714c..7f9f913b 100644 --- a/src/qcam/viewfinder_qt.cpp +++ b/src/qcam/viewfinder_qt.cpp @@ -34,7 +34,7 @@ static const QMap nativeFormats }; ViewFinderQt::ViewFinderQt(QWidget *parent) - : QWidget(parent), buffer_(nullptr) + : QWidget(parent), request_(nullptr), buffer_(nullptr) { icon_ = QIcon(":camera-off.svg"); } @@ -78,7 +78,8 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, return 0; } -void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) +void ViewFinderQt::render(libcamera::Request *request, + libcamera::FrameBuffer *buffer, MappedBuffer *map) { if (buffer->planes().size() != 1) { qWarning() << "Multi-planar buffers are not supported"; @@ -106,6 +107,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) size / size_.height(), ::nativeFormats[format_]); std::swap(buffer, buffer_); + std::swap(request, request_); } else { /* * Otherwise, convert the format and release the frame @@ -118,7 +120,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map) update(); if (buffer) - renderComplete(buffer); + renderComplete(request, buffer); } void ViewFinderQt::stop() @@ -126,7 +128,7 @@ void ViewFinderQt::stop() image_ = QImage(); if (buffer_) { - renderComplete(buffer_); + renderComplete(request_, buffer_); buffer_ = nullptr; } diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h index d7554288..5f62d7f6 100644 --- a/src/qcam/viewfinder_qt.h +++ b/src/qcam/viewfinder_qt.h @@ -17,6 +17,7 @@ #include #include #include +#include #include "format_converter.h" #include "viewfinder.h" @@ -32,13 +33,15 @@ public: const QList &nativeFormats() const override; int setFormat(const libcamera::PixelFormat &format, const QSize &size) override; - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override; + void render(libcamera::Request *request, + libcamera::FrameBuffer *buffer, + MappedBuffer *map) override; void stop() override; QImage getCurrentImage() override; Q_SIGNALS: - void renderComplete(libcamera::FrameBuffer *buffer); + void renderComplete(libcamera::Request *request, libcamera::FrameBuffer *buffer); protected: void paintEvent(QPaintEvent *) override; @@ -56,6 +59,7 @@ private: QPixmap pixmap_; /* Buffer and render image */ + libcamera::Request *request_; libcamera::FrameBuffer *buffer_; QImage image_; QMutex mutex_; /* Prevent concurrent access to image_ */