From patchwork Tue Oct 19 15:49:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 14526 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5718CBDB1C for ; Wed, 10 Nov 2021 14:01:52 +0000 (UTC) Received: from pendragon.ideasonboard.com (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E6FA1501 for ; Wed, 10 Nov 2021 15:01:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1636552912; bh=w/QHewBj4dTtJgxt8wXCLKHt/9AuF4VyhXdqb7avJ2U=; h=From:To:Cc:Subject:Date:Resent-From:Resent-To:From; b=syurQuAi01mw3+v2eRxpPUS2g3nHVtYBpWw9oHrod6DfoDuNfxNSTZo+afz/H+Gvg xhiMQwMgol5Y5XGK6odo8dMxCxeD7+EkHcC/V6NJmJNT1cZJurxNPe/OSZSH4TXrV1 bSaBB3TuPmeIXPjJiYIHypUMM3wDF10qm/A9498I= Delivered-To: kbingham@ideasonboard.com Received: from perceval.ideasonboard.com by perceval.ideasonboard.com with LMTP id SKrrNf7obmEGNAAA4E0KoQ (envelope-from ) for ; Tue, 19 Oct 2021 17:49:18 +0200 Received: from Monstersaurus.ksquared.org.uk.beta.tailscale.net (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AC71612A; Tue, 19 Oct 2021 17:49:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634658558; bh=w/QHewBj4dTtJgxt8wXCLKHt/9AuF4VyhXdqb7avJ2U=; h=From:To:Cc:Subject:Date:From; b=vMDIznmV/3pQJbUu0HjoVmC+IOAp5WI34BHykqaXqnL46In4BlP2wVgAF/9KKFVVz sQSb7pK5S79ZiECf4dpH4JyGLzsFDC5kTlbiGi70TEfGu6kH8yJ6RJBsfZL/Bd5M13 RMmomFmaSpalU1KEAH8Lsu6P32CkxKIYbztQ3hEA= From: Kieran Bingham To: libcamera devel Cc: Kieran Bingham Subject: [PATCH] libcamera: request: Make it extensible Date: Tue, 19 Oct 2021 16:49:15 +0100 Message-Id: <20211019154915.2123366-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-TUID: VvZsZrHmO+DA Resent-From: Kieran Bingham Resent-To: parsemail@patchwork.libcamera.org Provide an extensible private object for the Request class. This allows us to make modifications to the private API and storage of requests without affecting the public API or ABI. Initially the pending_ structure and hasPendingBuffers are adapted to support this move. Signed-off-by: Kieran Bingham --- v2: - Drop all the previous assertions that were added here. - Focus on making the Request use the d pointer design. - Move Request::Private to internal/request.h to make it available to other internal components such as PipelineHandlers This directly competes with Laurent's implementation. Posted mostly as reference, I don't mind which one goes forward. If Laurent's goes in, I'll just have to pull out the remaining parts of this and apply on top. --- include/libcamera/internal/meson.build | 1 + include/libcamera/internal/request.h | 32 +++++++++++++++++++++ include/libcamera/request.h | 7 +++-- src/libcamera/request.cpp | 39 ++++++++++++++++++-------- 4 files changed, 64 insertions(+), 15 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..630ae60e8b6c --- /dev/null +++ b/include/libcamera/internal/request.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * + * request.h - Internal capture request handling + */ +#ifndef __LIBCAMERA_INTERNAL_REQUEST_H__ +#define __LIBCAMERA_INTERNAL_REQUEST_H__ + +#include + +#include + +#include + +namespace libcamera { + +class Camera; +class CameraControlValidator; +class FrameBuffer; + +class Request::Private : public Extensible::Private +{ + LIBCAMERA_DECLARE_PUBLIC(Request) + +private: + std::unordered_set pending_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_INTERNAL_REQUEST_H__ */ diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 2d361c9d97dc..71d088551d94 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, @@ -56,7 +58,7 @@ public: uint64_t cookie() const { return cookie_; } Status status() const { return status_; } - bool hasPendingBuffers() const { return !pending_.empty(); } + bool hasPendingBuffers() const; std::string toString() const; @@ -75,7 +77,6 @@ private: ControlList *controls_; ControlList *metadata_; BufferMap bufferMap_; - std::unordered_set pending_; uint32_t sequence_; const uint64_t cookie_; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index f95ce4db5eaa..d363d7a7b005 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 @@ -22,14 +22,26 @@ #include "libcamera/internal/tracepoints.h" /** - * \file request.h + * \file libcamera/request.h * \brief Describes a frame capture request to be processed by a camera + * + * \file internal/request.h + * \brief Provides the internal implementation details for a Request */ namespace libcamera { LOG_DEFINE_CATEGORY(Request) +/** + * \class Request::Private + * \brief Base class for FrameBuffer 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 pipeline handlers. + */ + /** * \enum Request::Status * Request completion status @@ -74,8 +86,9 @@ 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_(camera), + sequence_(0), cookie_(cookie), status_(RequestPending), + cancelled_(false) { /** * \todo Should the Camera expose a validator instance, to avoid @@ -117,12 +130,11 @@ void Request::reuse(ReuseFlag flags) { LIBCAMERA_TRACEPOINT(request_reuse, this); - pending_.clear(); + _d()->pending_.clear(); if (flags & ReuseBuffers) { for (auto pair : bufferMap_) { FrameBuffer *buffer = pair.second; buffer->_d()->setRequest(this); - pending_.insert(buffer); } } else { bufferMap_.clear(); @@ -192,7 +204,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; @@ -267,12 +279,15 @@ 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 */ +bool Request::hasPendingBuffers() const +{ + return !_d()->pending_.empty(); +} /** * \brief Complete a queued request @@ -307,12 +322,12 @@ void Request::cancel() ASSERT(status_ == RequestPending); - for (FrameBuffer *buffer : pending_) { + for (FrameBuffer *buffer : _d()->pending_) { buffer->cancel(); camera_->bufferCompleted.emit(this, buffer); } - pending_.clear(); + _d()->pending_.clear(); cancelled_ = true; } @@ -333,7 +348,7 @@ bool Request::completeBuffer(FrameBuffer *buffer) { LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer); - int ret = pending_.erase(buffer); + int ret = _d()->pending_.erase(buffer); ASSERT(ret == 1); buffer->_d()->setRequest(nullptr); @@ -361,7 +376,7 @@ std::string Request::toString() const /* Example Output: Request(55:P:1/2:6523524) */ ss << "Request(" << sequence_ << ":" << statuses[status_] << ":" - << pending_.size() << "/" << bufferMap_.size() << ":" + << _d()->pending_.size() << "/" << bufferMap_.size() << ":" << cookie_ << ")"; return ss.str();