From patchwork Tue Nov 30 23:36:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 14916 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 990D7BDB13 for ; Tue, 30 Nov 2021 23:35:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5DA8E6072E; Wed, 1 Dec 2021 00:35:52 +0100 (CET) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DE5AD60592 for ; Wed, 1 Dec 2021 00:35:49 +0100 (CET) Received: (Authenticated sender: jacopo@jmondi.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 5F1C41BF208; Tue, 30 Nov 2021 23:35:48 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Wed, 1 Dec 2021 00:36:25 +0100 Message-Id: <20211130233634.34173-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.33.1 In-Reply-To: <20211130233634.34173-1-jacopo@jmondi.org> References: <20211130233634.34173-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 02/11] libcamera: request: Make Request class Extensible 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" From: Laurent Pinchart Implement the D-Pointer design pattern in the Request class to allow changing internal data without affecting the public ABI. Move the internal fields that are not needed to implement the public API to the Request::Private class already. This allow to remove the friend class declaration for the PipelineHandler class, which can now use the Request::Private API. Signed-off-by: Laurent Pinchart [Move all internal fields to Request::Private and remove friend declaration] Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/internal/meson.build | 1 + include/libcamera/internal/request.h | 49 ++++ .../libcamera/internal/tracepoints/request.tp | 13 +- include/libcamera/request.h | 19 +- src/libcamera/pipeline_handler.cpp | 15 +- src/libcamera/request.cpp | 230 ++++++++++++------ 6 files changed, 219 insertions(+), 108 deletions(-) create mode 100644 include/libcamera/internal/request.h diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 665fd6de4ed3..cae65b0604ff 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -34,6 +34,7 @@ libcamera_internal_headers = files([ 'pipeline_handler.h', 'process.h', 'pub_key.h', + 'request.h', 'source_paths.h', 'sysfs.h', 'v4l2_device.h', diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h new file mode 100644 index 000000000000..1340ffa2a683 --- /dev/null +++ b/include/libcamera/internal/request.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * request.h - Request class private data + */ +#ifndef __LIBCAMERA_INTERNAL_REQUEST_H__ +#define __LIBCAMERA_INTERNAL_REQUEST_H__ + +#include + +#include + +namespace libcamera { + +class Camera; +class FrameBuffer; + +class Request::Private : public Extensible::Private +{ + LIBCAMERA_DECLARE_PUBLIC(Request) + +public: + Private(Camera *camera); + ~Private(); + + Camera *camera() const { return camera_; } + bool hasPendingBuffers() const; + + bool completeBuffer(FrameBuffer *buffer); + void complete(); + void cancel(); + void reuse(); + +private: + friend class PipelineHandler; + + void doCancelRequest(); + + Camera *camera_; + bool cancelled_; + uint32_t sequence_ = 0; + + std::unordered_set pending_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_INTERNAL_REQUEST_H__ */ diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp index 37d8c46f4d96..f039ffd4f88d 100644 --- a/include/libcamera/internal/tracepoints/request.tp +++ b/include/libcamera/internal/tracepoints/request.tp @@ -5,8 +5,9 @@ * request.tp - Tracepoints for the request object */ +#include + #include -#include TRACEPOINT_EVENT_CLASS( libcamera, @@ -62,7 +63,7 @@ TRACEPOINT_EVENT_INSTANCE( request, request_complete, TP_ARGS( - libcamera::Request *, req + libcamera::Request::Private *, req ) ) @@ -71,7 +72,7 @@ TRACEPOINT_EVENT_INSTANCE( request, request_cancel, TP_ARGS( - libcamera::Request *, req + libcamera::Request::Private *, req ) ) @@ -79,13 +80,13 @@ TRACEPOINT_EVENT( libcamera, request_complete_buffer, TP_ARGS( - libcamera::Request *, req, + libcamera::Request::Private *, req, libcamera::FrameBuffer *, buf ), TP_FIELDS( ctf_integer_hex(uintptr_t, request, reinterpret_cast(req)) - ctf_integer(uint64_t, cookie, req->cookie()) - ctf_integer(int, status, req->status()) + ctf_integer(uint64_t, cookie, req->_o()->cookie()) + ctf_integer(int, status, req->_o()->status()) ctf_integer_hex(uintptr_t, buffer, reinterpret_cast(buf)) ctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status) ) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index f434335b243a..8c78970d88ab 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -25,8 +25,10 @@ class CameraControlValidator; class FrameBuffer; class Stream; -class Request +class Request : public Extensible { + LIBCAMERA_DECLARE_PRIVATE() + public: enum Status { RequestPending, @@ -52,34 +54,23 @@ public: int addBuffer(const Stream *stream, FrameBuffer *buffer); FrameBuffer *findBuffer(const Stream *stream) const; - uint32_t sequence() const { return sequence_; } + uint32_t sequence() const; uint64_t cookie() const { return cookie_; } Status status() const { return status_; } - bool hasPendingBuffers() const { return !pending_.empty(); } + bool hasPendingBuffers() const; std::string toString() const; private: LIBCAMERA_DISABLE_COPY(Request) - friend class PipelineHandler; - - void complete(); - void cancel(); - - bool completeBuffer(FrameBuffer *buffer); - - Camera *camera_; ControlList *controls_; ControlList *metadata_; BufferMap bufferMap_; - std::unordered_set pending_; - uint32_t sequence_; const uint64_t cookie_; Status status_; - bool cancelled_; }; } /* namespace libcamera */ diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index f69c4f03b80f..67fdf1d8db01 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -19,6 +19,7 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/media_device.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/tracepoints.h" /** @@ -311,15 +312,15 @@ void PipelineHandler::queueRequest(Request *request) { LIBCAMERA_TRACEPOINT(request_queue, request); - Camera *camera = request->camera_; + Camera *camera = request->_d()->camera(); Camera::Private *data = camera->_d(); data->queuedRequests_.push_back(request); - request->sequence_ = data->requestSequence_++; + request->_d()->sequence_ = data->requestSequence_++; int ret = queueRequestDevice(camera, request); if (ret) { - request->cancel(); + request->_d()->cancel(); completeRequest(request); } } @@ -360,9 +361,9 @@ void PipelineHandler::queueRequest(Request *request) */ bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer) { - Camera *camera = request->camera_; + Camera *camera = request->_d()->camera(); camera->bufferCompleted.emit(request, buffer); - return request->completeBuffer(buffer); + return request->_d()->completeBuffer(buffer); } /** @@ -381,9 +382,9 @@ bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer) */ void PipelineHandler::completeRequest(Request *request) { - Camera *camera = request->camera_; + Camera *camera = request->_d()->camera(); - request->complete(); + request->_d()->complete(); Camera::Private *data = camera->_d(); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 17fefab7ad0e..692202473360 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -5,7 +5,7 @@ * request.cpp - Capture request handling */ -#include +#include "libcamera/internal/request.h" #include #include @@ -23,7 +23,7 @@ #include "libcamera/internal/tracepoints.h" /** - * \file request.h + * \file libcamera/request.h * \brief Describes a frame capture request to be processed by a camera */ @@ -31,6 +31,139 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Request) +/** + * \class Request::Private + * \brief Request private data + * + * The Request::Private class stores all private data associated with a + * request. It implements the d-pointer design pattern to hide core + * Request data from the public API, and exposes utility functions to + * internal users of the request (namely the PipelineHandler class and its + * subclasses). + */ + +/** + * \brief Create a Request::Private + * \param camera The Camera that creates the request + */ +Request::Private::Private(Camera *camera) + : camera_(camera), cancelled_(false) +{ +} + +Request::Private::~Private() +{ + doCancelRequest(); +} + +/** + * \fn Request::Private::camera() + * \brief Retrieve the camera this request has been queued to + * \return The Camera this request has been queued to, or nullptr if the + * request hasn't been queued + */ + +/** + * \brief Check if a request has buffers yet to be completed + * + * \return True if the request has buffers pending for completion, false + * otherwise + */ +bool Request::Private::hasPendingBuffers() const +{ + return !pending_.empty(); +} + +/** + * \brief Complete a buffer for the request + * \param[in] buffer The buffer that has completed + * + * A request tracks the status of all buffers it contains through a set of + * pending buffers. This function removes the \a buffer from the set to mark it + * as complete. All buffers associate with the request shall be marked as + * complete by calling this function once and once only before reporting the + * request as complete with the complete() function. + * + * \return True if all buffers contained in the request have completed, false + * otherwise + */ +bool Request::Private::completeBuffer(FrameBuffer *buffer) +{ + LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer); + + int ret = pending_.erase(buffer); + ASSERT(ret == 1); + + buffer->_d()->setRequest(nullptr); + + if (buffer->metadata().status == FrameMetadata::FrameCancelled) + cancelled_ = true; + + return !hasPendingBuffers(); +} + +/** + * \brief Complete a queued request + * + * Mark the request as complete by updating its status to RequestComplete, + * unless buffers have been cancelled in which case the status is set to + * RequestCancelled. + */ +void Request::Private::complete() +{ + Request *request = _o(); + + ASSERT(request->status() == RequestPending); + ASSERT(!hasPendingBuffers()); + + request->status_ = cancelled_ ? RequestCancelled : RequestComplete; + + LOG(Request, Debug) << request->toString(); + + LIBCAMERA_TRACEPOINT(request_complete, this); +} + +void Request::Private::doCancelRequest() +{ + Request *request = _o(); + + for (FrameBuffer *buffer : pending_) { + buffer->cancel(); + camera_->bufferCompleted.emit(request, buffer); + } + + cancelled_ = true; + pending_.clear(); +} + +/** + * \brief Cancel a queued request + * + * Mark the request and its associated buffers as cancelled and complete it. + * + * Set each pending buffer in error state and emit the buffer completion signal + * before completing the Request. + */ +void Request::Private::cancel() +{ + LIBCAMERA_TRACEPOINT(request_cancel, this); + + Request *request = _o(); + ASSERT(request->status() == RequestPending); + + doCancelRequest(); +} + +/** + * \copydoc Request::reuse() + */ +void Request::Private::reuse() +{ + sequence_ = 0; + cancelled_ = false; + pending_.clear(); +} + /** * \enum Request::Status * Request completion status @@ -75,8 +208,8 @@ LOG_DEFINE_CATEGORY(Request) * completely opaque to libcamera. */ Request::Request(Camera *camera, uint64_t cookie) - : camera_(camera), sequence_(0), cookie_(cookie), - status_(RequestPending), cancelled_(false) + : Extensible(std::make_unique(camera)), + cookie_(cookie), status_(RequestPending) { controls_ = new ControlList(controls::controls, camera->_d()->validator()); @@ -113,20 +246,19 @@ void Request::reuse(ReuseFlag flags) { LIBCAMERA_TRACEPOINT(request_reuse, this); - pending_.clear(); + _d()->reuse(); + if (flags & ReuseBuffers) { for (auto pair : bufferMap_) { FrameBuffer *buffer = pair.second; buffer->_d()->setRequest(this); - pending_.insert(buffer); + _d()->pending_.insert(buffer); } } else { bufferMap_.clear(); } - sequence_ = 0; status_ = RequestPending; - cancelled_ = false; controls_->clear(); metadata_->clear(); @@ -188,7 +320,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) } buffer->_d()->setRequest(this); - pending_.insert(buffer); + _d()->pending_.insert(buffer); bufferMap_[stream] = buffer; return 0; @@ -227,7 +359,6 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const */ /** - * \fn Request::sequence() * \brief Retrieve the sequence number for the request * * When requests are queued, they are given a sequential number to track the @@ -242,6 +373,10 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const * * \return The request sequence number */ +uint32_t Request::sequence() const +{ + return _d()->sequence_; +} /** * \fn Request::cookie() @@ -263,81 +398,14 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const */ /** - * \fn Request::hasPendingBuffers() * \brief Check if a request has buffers yet to be completed * * \return True if the request has buffers pending for completion, false * otherwise */ - -/** - * \brief Complete a queued request - * - * Mark the request as complete by updating its status to RequestComplete, - * unless buffers have been cancelled in which case the status is set to - * RequestCancelled. - */ -void Request::complete() -{ - ASSERT(status_ == RequestPending); - ASSERT(!hasPendingBuffers()); - - status_ = cancelled_ ? RequestCancelled : RequestComplete; - - LOG(Request, Debug) << toString(); - - LIBCAMERA_TRACEPOINT(request_complete, this); -} - -/** - * \brief Cancel a queued request - * - * Mark the request and its associated buffers as cancelled and complete it. - * - * Set each pending buffer in error state and emit the buffer completion signal - * before completing the Request. - */ -void Request::cancel() -{ - LIBCAMERA_TRACEPOINT(request_cancel, this); - - ASSERT(status_ == RequestPending); - - for (FrameBuffer *buffer : pending_) { - buffer->cancel(); - camera_->bufferCompleted.emit(this, buffer); - } - - pending_.clear(); - cancelled_ = true; -} - -/** - * \brief Complete a buffer for the request - * \param[in] buffer The buffer that has completed - * - * A request tracks the status of all buffers it contains through a set of - * pending buffers. This function removes the \a buffer from the set to mark it - * as complete. All buffers associate with the request shall be marked as - * complete by calling this function once and once only before reporting the - * request as complete with the complete() function. - * - * \return True if all buffers contained in the request have completed, false - * otherwise - */ -bool Request::completeBuffer(FrameBuffer *buffer) +bool Request::hasPendingBuffers() const { - LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer); - - int ret = pending_.erase(buffer); - ASSERT(ret == 1); - - buffer->_d()->setRequest(nullptr); - - if (buffer->metadata().status == FrameMetadata::FrameCancelled) - cancelled_ = true; - - return !hasPendingBuffers(); + return !_d()->pending_.empty(); } /** @@ -356,8 +424,8 @@ std::string Request::toString() const static const char *statuses = "PCX"; /* Example Output: Request(55:P:1/2:6523524) */ - ss << "Request(" << sequence_ << ":" << statuses[status_] << ":" - << pending_.size() << "/" << bufferMap_.size() << ":" + ss << "Request(" << sequence() << ":" << statuses[status_] << ":" + << _d()->pending_.size() << "/" << bufferMap_.size() << ":" << cookie_ << ")"; return ss.str();