From patchwork Mon Dec 16 12:44: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: 2429 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 6E50E601E4 for ; Mon, 16 Dec 2019 13:45:21 +0100 (CET) X-Halon-ID: e410c1d2-2001-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id e410c1d2-2001-11ea-a00b-005056917a89; Mon, 16 Dec 2019 13:45:20 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 16 Dec 2019 13:44:55 +0100 Message-Id: <20191216124456.1049508-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191216124456.1049508-1-niklas.soderlund@ragnatech.se> References: <20191216124456.1049508-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] 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: Mon, 16 Dec 2019 12:45:21 -0000 Expecting pipeline handler implementations of queueRequest() to call the base class queueRequest() at the correct point have led to different behaviors between the pipelines. Fix this by splitting queueRequest() into a base class implementation which handles the bookkeeping and a new queueRequestDevice() that is to be implemented by pipeline handlers and only deals with committing the request to the device. 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..f3622631022d87b9 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 queueRequestDevice(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..6d8c3fada127310e 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 queueRequestDevice(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::queueRequestDevice(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..7e41222e3d01a475 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 queueRequestDevice(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::queueRequestDevice(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..3043366bee38bcfc 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 queueRequestDevice(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::queueRequestDevice(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..292900bcf8d1b359 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 queueRequestDevice(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::queueRequestDevice(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..5badf31ce793edf8 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -345,16 +345,12 @@ 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. + * The request is first added to the internal list of queued requests, and + * then passed to the pipeline handler with a call to queueRequestDevice(). * - * 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 - * completeRequest() method. + * Keeping track of queued requests ensures automatic completion of all requests + * when the pipeline handler is stopped with stop(). Request completion shall be + * signalled by the pipeline handler using the completeRequest() method. * * \return 0 on success or a negative error code otherwise */ @@ -363,9 +359,28 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request) CameraData *data = cameraData(camera); data->queuedRequests_.push_back(request); - return 0; + int ret = queueRequestDevice(camera, request); + if (ret) + data->queuedRequests_.remove(request); + + return ret; } +/** + * \fn PipelineHandler::queueRequestDevice() + * \brief Queue a request to the device + * \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 device 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 Mon Dec 16 12:44: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: 2430 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 251C9601E4 for ; Mon, 16 Dec 2019 13:45:37 +0100 (CET) X-Halon-ID: ebc2056e-2001-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5865.dip0.t-ipconnect.de [84.172.88.101]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id ebc2056e-2001-11ea-a00b-005056917a89; Mon, 16 Dec 2019 13:45:33 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 16 Dec 2019 13:44:56 +0100 Message-Id: <20191216124456.1049508-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191216124456.1049508-1-niklas.soderlund@ragnatech.se> References: <20191216124456.1049508-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/2] 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: Mon, 16 Dec 2019 12:45:37 -0000 The buffer index is a V4L2 concept that will be hidden from users with the introduction of a new FrameBuffer class. In preparation for this, remove the index from log messages. 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 | 6 ++---- test/v4l2_videodevice/capture_async.cpp | 2 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 4 ++-- 6 files changed, 8 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..13e023237dab0daf 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1102,6 +1102,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer() return nullptr; } + LOG(V4L2, Debug) << "Dequeuing buffer " << buf.index; ASSERT(buf.index < bufferPool_->count()); auto it = queuedBuffers_.find(buf.index); @@ -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..3a56862cb2b77d38 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -92,8 +92,7 @@ protected: void captureBufferReady(Buffer *buffer) { - std::cout << "Received capture buffer: " << buffer->index() - << " sequence " << buffer->sequence() << std::endl; + std::cout << "Received capture buffer" << std::endl; if (buffer->status() != Buffer::BufferSuccess) return; @@ -104,8 +103,7 @@ protected: void outputBufferReady(Buffer *buffer) { - std::cout << "Received output buffer: " << buffer->index() - << " sequence " << buffer->sequence() << std::endl; + std::cout << "Received output buffer" << 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..f62bbd837b213a0a 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 << "Buffer received" << 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_++;