From patchwork Sun Jul 4 23:28:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 12805 X-Patchwork-Delegate: laurent.pinchart@ideasonboard.com 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 5C454C3223 for ; Sun, 4 Jul 2021 23:29:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BB2F868509; Mon, 5 Jul 2021 01:29:05 +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="rXxyKSMu"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 092EE60285 for ; Mon, 5 Jul 2021 01:29:03 +0200 (CEST) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9AB052E4; Mon, 5 Jul 2021 01:29:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1625441342; bh=f4mBgqo2UbWxfmCfWGseTbkEI5ADgdXsN4wFD71pwMQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rXxyKSMujniIruuqqo34MRS0RuTuWnrBGW/fwOu1Q9OlYWWYdKkicd9fTOs63y7Q7 PMQyPv3vTQ7xwuAF7suFqLzlE9ofAynJZYUnJbz28iFdXFco5yJJvSs7mvzV7VyjX+ yiAH+I0DhbkcjnEimnye0kZKUxzasrfWQGpjb8Tw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 5 Jul 2021 02:28:17 +0300 Message-Id: <20210704232817.8205-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210704232817.8205-1-laurent.pinchart@ideasonboard.com> References: <20210704232817.8205-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/3] libcamera: framebuffer: Make FrameBuffer 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" Implement the D-Pointer design pattern in the FrameBuffer class to allow changing internal data without affecting the public ABI. Move the request_ field and the setRequest() function to the FrameBuffer::Private class. This allows hiding the setRequest() function from the public API, removing one todo item. More fields may be moved later. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi for 1/3 too Reviewed-by: Umang Jain --- include/libcamera/framebuffer.h | 8 +++--- include/libcamera/internal/framebuffer.h | 13 +++++++++ src/libcamera/framebuffer.cpp | 34 +++++++++++++++--------- src/libcamera/pipeline/ipu3/cio2.cpp | 3 ++- src/libcamera/pipeline/ipu3/frames.cpp | 5 ++-- src/libcamera/request.cpp | 7 ++--- 6 files changed, 47 insertions(+), 23 deletions(-) diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h index baf22a466907..7265e816b036 100644 --- a/include/libcamera/framebuffer.h +++ b/include/libcamera/framebuffer.h @@ -35,8 +35,10 @@ struct FrameMetadata { std::vector planes; }; -class FrameBuffer final +class FrameBuffer final : public Extensible { + LIBCAMERA_DECLARE_PRIVATE(); + public: struct Plane { FileDescriptor fd; @@ -47,8 +49,7 @@ public: const std::vector &planes() const { return planes_; } - Request *request() const { return request_; } - void setRequest(Request *request) { request_ = request; } + Request *request() const; const FrameMetadata &metadata() const { return metadata_; } unsigned int cookie() const { return cookie_; } @@ -63,7 +64,6 @@ private: std::vector planes_; - Request *request_; FrameMetadata metadata_; unsigned int cookie_; diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h index 0c76fc62af1d..a11e895d9b88 100644 --- a/include/libcamera/internal/framebuffer.h +++ b/include/libcamera/internal/framebuffer.h @@ -47,6 +47,19 @@ public: MappedFrameBuffer(const FrameBuffer *buffer, int flags); }; +class FrameBuffer::Private : public Extensible::Private +{ + LIBCAMERA_DECLARE_PUBLIC(FrameBuffer) + +public: + Private(FrameBuffer *buffer); + + void setRequest(Request *request) { request_ = request; } + +private: + Request *request_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__ */ diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index 4bde556c4013..5e13e281fb8c 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -100,6 +100,21 @@ LOG_DEFINE_CATEGORY(Buffer) * \brief Array of per-plane metadata */ +FrameBuffer::Private::Private(FrameBuffer *buffer) + : Extensible::Private(buffer), request_(nullptr) +{ +} + +/** + * \fn FrameBuffer::Private::setRequest() + * \brief Set the request this buffer belongs to + * \param[in] request Request to set + * + * For buffers added to requests by applications, this method is called by + * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline + * handlers, it is called by the pipeline handlers themselves. + */ + /** * \class FrameBuffer * \brief Frame buffer data and its associated dynamic metadata @@ -161,7 +176,7 @@ LOG_DEFINE_CATEGORY(Buffer) * \param[in] cookie Cookie */ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie) - : planes_(planes), request_(nullptr), cookie_(cookie) + : Extensible(new Private(this)), planes_(planes), cookie_(cookie) { } @@ -172,7 +187,6 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie) */ /** - * \fn FrameBuffer::request() * \brief Retrieve the request this buffer belongs to * * The intended callers of this method are buffer completion handlers that @@ -185,18 +199,12 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie) * \return The Request the FrameBuffer belongs to, or nullptr if the buffer is * not associated with a request */ +Request *FrameBuffer::request() const +{ + const Private *const d = LIBCAMERA_D_PTR(); -/** - * \fn FrameBuffer::setRequest() - * \brief Set the request this buffer belongs to - * \param[in] request Request to set - * - * For buffers added to requests by applications, this method is called by - * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline - * handlers, it is called by the pipeline handlers themselves. - * - * \todo Shall be hidden from applications with a d-pointer design. - */ + return d->request_; +} /** * \fn FrameBuffer::metadata() diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 1be2cbcd5cac..1bcd580e251c 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -14,6 +14,7 @@ #include #include "libcamera/internal/camera_sensor.h" +#include "libcamera/internal/framebuffer.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/v4l2_subdevice.h" @@ -278,7 +279,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer) buffer = availableBuffers_.front(); availableBuffers_.pop(); - buffer->setRequest(request); + buffer->_d()->setRequest(request); } int ret = output_->queueBuffer(buffer); diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp index ce5ccbf18e41..a4c3477cd9ef 100644 --- a/src/libcamera/pipeline/ipu3/frames.cpp +++ b/src/libcamera/pipeline/ipu3/frames.cpp @@ -10,6 +10,7 @@ #include #include +#include "libcamera/internal/framebuffer.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -56,8 +57,8 @@ IPU3Frames::Info *IPU3Frames::create(Request *request) FrameBuffer *paramBuffer = availableParamBuffers_.front(); FrameBuffer *statBuffer = availableStatBuffers_.front(); - paramBuffer->setRequest(request); - statBuffer->setRequest(request); + paramBuffer->_d()->setRequest(request); + statBuffer->_d()->setRequest(request); availableParamBuffers_.pop(); availableStatBuffers_.pop(); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 5faf3c71ff02..c095c9f45b09 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -18,6 +18,7 @@ #include #include "libcamera/internal/camera_controls.h" +#include "libcamera/internal/framebuffer.h" #include "libcamera/internal/tracepoints.h" /** @@ -121,7 +122,7 @@ void Request::reuse(ReuseFlag flags) if (flags & ReuseBuffers) { for (auto pair : bufferMap_) { FrameBuffer *buffer = pair.second; - buffer->setRequest(this); + buffer->_d()->setRequest(this); pending_.insert(buffer); } } else { @@ -191,7 +192,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) return -EEXIST; } - buffer->setRequest(this); + buffer->_d()->setRequest(this); pending_.insert(buffer); bufferMap_[stream] = buffer; @@ -336,7 +337,7 @@ bool Request::completeBuffer(FrameBuffer *buffer) int ret = pending_.erase(buffer); ASSERT(ret == 1); - buffer->setRequest(nullptr); + buffer->_d()->setRequest(nullptr); if (buffer->metadata().status == FrameMetadata::FrameCancelled) cancelled_ = true;