[{"id":33804,"web_url":"https://patchwork.libcamera.org/comment/33804/","msgid":"<174344395206.3394313.990791375306469119@ping.linuxembedded.co.uk>","date":"2025-03-31T17:59:12","subject":"Re: [PATCH v2 3/3] libcamera: request: Move all members to internal\n\tprivate class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-03-31 15:17:14)\n> Move all members of `Request` into `Request::Private`.\n> \n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> ---\n>  include/libcamera/internal/request.h |  10 ++-\n>  include/libcamera/request.h          |  18 ++---\n>  src/libcamera/request.cpp            | 104 ++++++++++++++-------------\n>  3 files changed, 69 insertions(+), 63 deletions(-)\n> \n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 73e9bb5cc..009ebbf14 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -30,7 +30,7 @@ class Request::Private : public Extensible::Private\n>         LIBCAMERA_DECLARE_PUBLIC(Request)\n>  \n>  public:\n> -       Private(Camera *camera);\n> +       Private(Camera *camera, uint64_t cookie);\n>         ~Private();\n>  \n>         Camera *camera() const { return camera_; }\n> @@ -39,7 +39,7 @@ public:\n>         bool completeBuffer(FrameBuffer *buffer);\n>         void complete();\n>         void cancel();\n> -       void reset();\n> +       void reset(ReuseFlag flags);\n>  \n>         void prepare(std::chrono::milliseconds timeout = 0ms);\n>         Signal<> prepared;\n> @@ -57,10 +57,16 @@ private:\n>         bool cancelled_;\n>         uint32_t sequence_ = 0;\n>         bool prepared_ = false;\n> +       Status status_ = RequestPending;\n> +       const uint64_t cookie_;\n>  \n>         std::unordered_set<FrameBuffer *> pending_;\n>         std::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n>         std::unique_ptr<Timer> timer_;\n> +\n> +       ControlList controls_;\n> +       ControlList metadata_;\n> +       BufferMap bufferMap_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 3061e2fb0..609b55885 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -49,16 +49,17 @@ public:\n>  \n>         void reuse(ReuseFlag flags = Default);\n>  \n> -       ControlList &controls() { return controls_; }\n> -       ControlList &metadata() { return metadata_; }\n> -       const BufferMap &buffers() const { return bufferMap_; }\n> +       ControlList &controls();\n> +       ControlList &metadata();\n> +\n> +       const BufferMap &buffers() const;\n>         int addBuffer(const Stream *stream, FrameBuffer *buffer,\n>                       std::unique_ptr<Fence> fence = nullptr);\n>         FrameBuffer *findBuffer(const Stream *stream) const;\n>  \n>         uint32_t sequence() const;\n> -       uint64_t cookie() const { return cookie_; }\n> -       Status status() const { return status_; }\n> +       uint64_t cookie() const;\n> +       Status status() const;\n>  \n>         bool hasPendingBuffers() const;\n>  \n> @@ -66,13 +67,6 @@ public:\n>  \n>  private:\n>         LIBCAMERA_DISABLE_COPY(Request)\n> -\n> -       ControlList controls_;\n> -       ControlList metadata_;\n> -       BufferMap bufferMap_;\n> -\n> -       const uint64_t cookie_;\n> -       Status status_;\n>  };\n>  \n>  std::ostream &operator<<(std::ostream &out, const Request &r);\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index fc364441a..1f1be1c7e 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -53,9 +53,12 @@ LOG_DEFINE_CATEGORY(Request)\n>  /**\n>   * \\brief Create a Request::Private\n>   * \\param camera The Camera that creates the request\n> + * \\param cookie Opaque cookie for application use\n>   */\n> -Request::Private::Private(Camera *camera)\n> -       : camera_(camera), cancelled_(false)\n> +Request::Private::Private(Camera *camera, uint64_t cookie)\n> +       : camera_(camera), cancelled_(false), cookie_(cookie),\n> +         controls_(controls::controls, camera->_d()->validator()),\n> +         metadata_(controls::controls) /* \\todo Add a validator for metadata controls. */\n>  {\n>  }\n>  \n> @@ -121,10 +124,10 @@ void Request::Private::complete()\n>  {\n>         Request *request = _o<Request>();\n>  \n> -       ASSERT(request->status() == RequestPending);\n> +       ASSERT(status_ == RequestPending);\n>         ASSERT(!hasPendingBuffers());\n>  \n> -       request->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> +       status_ = cancelled_ ? RequestCancelled : RequestComplete;\n>  \n>         LOG(Request, Debug) << request->toString();\n>  \n> @@ -158,26 +161,39 @@ void Request::Private::cancel()\n>  {\n>         LIBCAMERA_TRACEPOINT(request_cancel, this);\n>  \n> -       Request *request = _o<Request>();\n> -       ASSERT(request->status() == RequestPending);\n> +       ASSERT(status_ == RequestPending);\n>  \n>         doCancelRequest();\n>  }\n>  \n>  /**\n> - * \\brief Reset the request internal data to default values\n> - *\n> - * After calling this function, all request internal data will have default\n> - * values as if the Request::Private instance had just been constructed.\n> + * \\copydoc Request::reuse()\n> + * \\sa Request::reuse()\n>   */\n> -void Request::Private::reset()\n> +void Request::Private::reset(Request::ReuseFlag flags)\n>  {\n>         sequence_ = 0;\n>         cancelled_ = false;\n>         prepared_ = false;\n> +       status_ = RequestPending;\n> +\n>         pending_.clear();\n>         notifiers_.clear();\n>         timer_.reset();\n> +\n> +       controls_.clear();\n> +       metadata_.clear();\n> +\n> +       if (flags & ReuseBuffers) {\n> +               auto *request = _o<Request>();\n> +\n> +               for (const auto &[stream, buffer] : bufferMap_) {\n> +                       buffer->_d()->setRequest(request);\n> +                       pending_.insert(buffer);\n> +               }\n> +       } else {\n> +               bufferMap_.clear();\n> +       }\n>  }\n>  \n>  /*\n> @@ -353,14 +369,11 @@ void Request::Private::timeout()\n>   * completely opaque to libcamera.\n>   */\n>  Request::Request(Camera *camera, uint64_t cookie)\n> -       : Extensible(std::make_unique<Private>(camera)),\n> -         controls_(controls::controls, camera->_d()->validator()),\n> -         metadata_(controls::controls), /* \\todo Add a validator for metadata controls. */\n> -         cookie_(cookie), status_(RequestPending)\n> +       : Extensible(std::make_unique<Private>(camera, cookie))\n>  {\n>         LIBCAMERA_TRACEPOINT(request_construct, this);\n>  \n> -       LOG(Request, Debug) << \"Created request - cookie: \" << cookie_;\n> +       LOG(Request, Debug) << \"Created request - cookie: \" << cookie;\n>  }\n>  \n>  Request::~Request()\n> @@ -382,26 +395,10 @@ void Request::reuse(ReuseFlag flags)\n>  {\n>         LIBCAMERA_TRACEPOINT(request_reuse, this);\n>  \n> -       _d()->reset();\n> -\n> -       if (flags & ReuseBuffers) {\n> -               for (auto pair : bufferMap_) {\n> -                       FrameBuffer *buffer = pair.second;\n> -                       buffer->_d()->setRequest(this);\n> -                       _d()->pending_.insert(buffer);\n> -               }\n> -       } else {\n> -               bufferMap_.clear();\n> -       }\n> -\n> -       status_ = RequestPending;\n> -\n> -       controls_.clear();\n> -       metadata_.clear();\n> +       _d()->reset(flags);\n>  }\n>  \n>  /**\n> - * \\fn Request::controls()\n>   * \\brief Retrieve the request's ControlList\n>   *\n>   * Requests store a list of controls to be applied to all frames captured for\n> @@ -415,9 +412,12 @@ void Request::reuse(ReuseFlag flags)\n>   *\n>   * \\return A reference to the ControlList in this request\n>   */\n> +ControlList &Request::controls()\n> +{\n> +       return _d()->controls_;\n> +}\n>  \n>  /**\n> - * \\fn Request::buffers()\n>   * \\brief Retrieve the request's streams to buffers map\n>   *\n>   * Return a reference to the map that associates each Stream part of the\n> @@ -425,6 +425,10 @@ void Request::reuse(ReuseFlag flags)\n>   *\n>   * \\return The map of Stream to FrameBuffer\n>   */\n> +const Request::BufferMap &Request::buffers() const\n> +{\n> +       return _d()->bufferMap_;\n> +}\n>  \n>  /**\n>   * \\brief Add a FrameBuffer with its associated Stream to the Request\n> @@ -475,7 +479,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n>                 return -EEXIST;\n>         }\n>  \n> -       auto [it, inserted] = bufferMap_.try_emplace(stream, buffer);\n> +       auto [it, inserted] = _d()->bufferMap_.try_emplace(stream, buffer);\n>         if (!inserted) {\n>                 LOG(Request, Error) << \"FrameBuffer already set for stream\";\n>                 return -EEXIST;\n> @@ -490,15 +494,6 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n>         return 0;\n>  }\n>  \n> -/**\n> - * \\var Request::bufferMap_\n> - * \\brief Mapping of streams to buffers for this request\n> - *\n> - * The bufferMap_ tracks the buffers associated with each stream. If a stream is\n> - * not utilised in this request there will be no buffer for that stream in the\n> - * map.\n> - */\n> -\n>  /**\n>   * \\brief Return the buffer associated with a stream\n>   * \\param[in] stream The stream the buffer is associated to\n> @@ -507,20 +502,25 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n>   */\n>  FrameBuffer *Request::findBuffer(const Stream *stream) const\n>  {\n> -       const auto it = bufferMap_.find(stream);\n> -       if (it == bufferMap_.end())\n> +       const auto &bufferMap = _d()->bufferMap_;\n> +\n> +       const auto it = bufferMap.find(stream);\n> +       if (it == bufferMap.end())\n>                 return nullptr;\n>  \n>         return it->second;\n>  }\n>  \n>  /**\n> - * \\fn Request::metadata()\n>   * \\brief Retrieve the request's metadata\n>   * \\todo Offer a read-only API towards applications while keeping a read/write\n>   * API internally.\n>   * \\return The metadata associated with the request\n>   */\n> +ControlList &Request::metadata()\n> +{\n> +       return _d()->metadata_;\n> +}\n>  \n>  /**\n>   * \\brief Retrieve the sequence number for the request\n> @@ -543,13 +543,15 @@ uint32_t Request::sequence() const\n>  }\n>  \n>  /**\n> - * \\fn Request::cookie()\n>   * \\brief Retrieve the cookie set when the request was created\n>   * \\return The request cookie\n>   */\n> +uint64_t Request::cookie() const\n> +{\n> +       return _d()->cookie_;\n> +}\n>  \n>  /**\n> - * \\fn Request::status()\n>   * \\brief Retrieve the request completion status\n>   *\n>   * The request status indicates whether the request has completed successfully\n> @@ -560,6 +562,10 @@ uint32_t Request::sequence() const\n>   *\n>   * \\return The request completion status\n>   */\n> +Request::Status Request::status() const\n> +{\n> +       return _d()->status_;\n> +}\n>  \n>  /**\n>   * \\brief Check if a request has buffers yet to be completed\n> -- \n> 2.49.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7186DC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 31 Mar 2025 17:59:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 61E2768981;\n\tMon, 31 Mar 2025 19:59:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6994968967\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 31 Mar 2025 19:59:14 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C0FF5703;\n\tMon, 31 Mar 2025 19:57:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"n4/6zr2m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743443842;\n\tbh=HC0K694pBApat/nEoTcAk2/LZ+ArKfhtzLjJIyd0/iI=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=n4/6zr2m5azJ87XOyFbwC+If0ypCG+PpnpLvB2BOMq0chJYiLaqGccUPRdNRqOnjV\n\tNtAIrVjgpjVY150d6Z+dnIlr5bujWR0cgai3PTNLhpKDO2UWo+FXZJ/58V1Rdafvpq\n\tOBUA4ldEeXCBeE/HXvxGlQk232hyqKwNHjbg5K4Y=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250331141714.512538-3-barnabas.pocze@ideasonboard.com>","References":"<20250331141714.512538-1-barnabas.pocze@ideasonboard.com>\n\t<20250331141714.512538-3-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v2 3/3] libcamera: request: Move all members to internal\n\tprivate class","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 31 Mar 2025 18:59:12 +0100","Message-ID":"<174344395206.3394313.990791375306469119@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33874,"web_url":"https://patchwork.libcamera.org/comment/33874/","msgid":"<20250401225724.GN3494@pendragon.ideasonboard.com>","date":"2025-04-01T22:57:24","subject":"Re: [PATCH v2 3/3] libcamera: request: Move all members to internal\n\tprivate class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Mon, Mar 31, 2025 at 04:17:14PM +0200, Barnabás Pőcze wrote:\n> Move all members of `Request` into `Request::Private`.\n\nThe commit message should explain why.\n\n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n>  include/libcamera/internal/request.h |  10 ++-\n>  include/libcamera/request.h          |  18 ++---\n>  src/libcamera/request.cpp            | 104 ++++++++++++++-------------\n>  3 files changed, 69 insertions(+), 63 deletions(-)\n> \n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 73e9bb5cc..009ebbf14 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -30,7 +30,7 @@ class Request::Private : public Extensible::Private\n>  \tLIBCAMERA_DECLARE_PUBLIC(Request)\n>  \n>  public:\n> -\tPrivate(Camera *camera);\n> +\tPrivate(Camera *camera, uint64_t cookie);\n>  \t~Private();\n>  \n>  \tCamera *camera() const { return camera_; }\n> @@ -39,7 +39,7 @@ public:\n>  \tbool completeBuffer(FrameBuffer *buffer);\n>  \tvoid complete();\n>  \tvoid cancel();\n> -\tvoid reset();\n> +\tvoid reset(ReuseFlag flags);\n>  \n>  \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n>  \tSignal<> prepared;\n> @@ -57,10 +57,16 @@ private:\n>  \tbool cancelled_;\n>  \tuint32_t sequence_ = 0;\n>  \tbool prepared_ = false;\n> +\tStatus status_ = RequestPending;\n> +\tconst uint64_t cookie_;\n>  \n>  \tstd::unordered_set<FrameBuffer *> pending_;\n>  \tstd::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n>  \tstd::unique_ptr<Timer> timer_;\n> +\n> +\tControlList controls_;\n> +\tControlList metadata_;\n> +\tBufferMap bufferMap_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 3061e2fb0..609b55885 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -49,16 +49,17 @@ public:\n>  \n>  \tvoid reuse(ReuseFlag flags = Default);\n>  \n> -\tControlList &controls() { return controls_; }\n> -\tControlList &metadata() { return metadata_; }\n> -\tconst BufferMap &buffers() const { return bufferMap_; }\n> +\tControlList &controls();\n> +\tControlList &metadata();\n> +\n> +\tconst BufferMap &buffers() const;\n>  \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n>  \t\t      std::unique_ptr<Fence> fence = nullptr);\n>  \tFrameBuffer *findBuffer(const Stream *stream) const;\n>  \n>  \tuint32_t sequence() const;\n> -\tuint64_t cookie() const { return cookie_; }\n> -\tStatus status() const { return status_; }\n> +\tuint64_t cookie() const;\n> +\tStatus status() const;\n>  \n>  \tbool hasPendingBuffers() const;\n>  \n> @@ -66,13 +67,6 @@ public:\n>  \n>  private:\n>  \tLIBCAMERA_DISABLE_COPY(Request)\n> -\n> -\tControlList controls_;\n> -\tControlList metadata_;\n> -\tBufferMap bufferMap_;\n> -\n> -\tconst uint64_t cookie_;\n> -\tStatus status_;\n>  };\n>  \n>  std::ostream &operator<<(std::ostream &out, const Request &r);\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index fc364441a..1f1be1c7e 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -53,9 +53,12 @@ LOG_DEFINE_CATEGORY(Request)\n>  /**\n>   * \\brief Create a Request::Private\n>   * \\param camera The Camera that creates the request\n> + * \\param cookie Opaque cookie for application use\n>   */\n> -Request::Private::Private(Camera *camera)\n> -\t: camera_(camera), cancelled_(false)\n> +Request::Private::Private(Camera *camera, uint64_t cookie)\n> +\t: camera_(camera), cancelled_(false), cookie_(cookie),\n> +\t  controls_(controls::controls, camera->_d()->validator()),\n> +\t  metadata_(controls::controls) /* \\todo Add a validator for metadata controls. */\n>  {\n>  }\n>  \n> @@ -121,10 +124,10 @@ void Request::Private::complete()\n>  {\n>  \tRequest *request = _o<Request>();\n>  \n> -\tASSERT(request->status() == RequestPending);\n> +\tASSERT(status_ == RequestPending);\n>  \tASSERT(!hasPendingBuffers());\n>  \n> -\trequest->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> +\tstatus_ = cancelled_ ? RequestCancelled : RequestComplete;\n>  \n>  \tLOG(Request, Debug) << request->toString();\n>  \n> @@ -158,26 +161,39 @@ void Request::Private::cancel()\n>  {\n>  \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n>  \n> -\tRequest *request = _o<Request>();\n> -\tASSERT(request->status() == RequestPending);\n> +\tASSERT(status_ == RequestPending);\n>  \n>  \tdoCancelRequest();\n>  }\n>  \n>  /**\n> - * \\brief Reset the request internal data to default values\n> - *\n> - * After calling this function, all request internal data will have default\n> - * values as if the Request::Private instance had just been constructed.\n> + * \\copydoc Request::reuse()\n> + * \\sa Request::reuse()\n>   */\n> -void Request::Private::reset()\n> +void Request::Private::reset(Request::ReuseFlag flags)\n>  {\n>  \tsequence_ = 0;\n>  \tcancelled_ = false;\n>  \tprepared_ = false;\n> +\tstatus_ = RequestPending;\n> +\n>  \tpending_.clear();\n>  \tnotifiers_.clear();\n>  \ttimer_.reset();\n> +\n> +\tcontrols_.clear();\n> +\tmetadata_.clear();\n> +\n> +\tif (flags & ReuseBuffers) {\n> +\t\tauto *request = _o<Request>();\n\n\t\tRequest *request = _o<Request>();\n\n> +\n> +\t\tfor (const auto &[stream, buffer] : bufferMap_) {\n> +\t\t\tbuffer->_d()->setRequest(request);\n> +\t\t\tpending_.insert(buffer);\n> +\t\t}\n> +\t} else {\n> +\t\tbufferMap_.clear();\n> +\t}\n>  }\n>  \n>  /*\n> @@ -353,14 +369,11 @@ void Request::Private::timeout()\n>   * completely opaque to libcamera.\n>   */\n>  Request::Request(Camera *camera, uint64_t cookie)\n> -\t: Extensible(std::make_unique<Private>(camera)),\n> -\t  controls_(controls::controls, camera->_d()->validator()),\n> -\t  metadata_(controls::controls), /* \\todo Add a validator for metadata controls. */\n> -\t  cookie_(cookie), status_(RequestPending)\n> +\t: Extensible(std::make_unique<Private>(camera, cookie))\n>  {\n>  \tLIBCAMERA_TRACEPOINT(request_construct, this);\n>  \n> -\tLOG(Request, Debug) << \"Created request - cookie: \" << cookie_;\n> +\tLOG(Request, Debug) << \"Created request - cookie: \" << cookie;\n>  }\n>  \n>  Request::~Request()\n> @@ -382,26 +395,10 @@ void Request::reuse(ReuseFlag flags)\n>  {\n>  \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n>  \n> -\t_d()->reset();\n> -\n> -\tif (flags & ReuseBuffers) {\n> -\t\tfor (auto pair : bufferMap_) {\n> -\t\t\tFrameBuffer *buffer = pair.second;\n> -\t\t\tbuffer->_d()->setRequest(this);\n> -\t\t\t_d()->pending_.insert(buffer);\n> -\t\t}\n> -\t} else {\n> -\t\tbufferMap_.clear();\n> -\t}\n> -\n> -\tstatus_ = RequestPending;\n> -\n> -\tcontrols_.clear();\n> -\tmetadata_.clear();\n> +\t_d()->reset(flags);\n>  }\n>  \n>  /**\n> - * \\fn Request::controls()\n>   * \\brief Retrieve the request's ControlList\n>   *\n>   * Requests store a list of controls to be applied to all frames captured for\n> @@ -415,9 +412,12 @@ void Request::reuse(ReuseFlag flags)\n>   *\n>   * \\return A reference to the ControlList in this request\n>   */\n> +ControlList &Request::controls()\n> +{\n> +\treturn _d()->controls_;\n> +}\n>  \n>  /**\n> - * \\fn Request::buffers()\n>   * \\brief Retrieve the request's streams to buffers map\n>   *\n>   * Return a reference to the map that associates each Stream part of the\n> @@ -425,6 +425,10 @@ void Request::reuse(ReuseFlag flags)\n>   *\n>   * \\return The map of Stream to FrameBuffer\n>   */\n> +const Request::BufferMap &Request::buffers() const\n> +{\n> +\treturn _d()->bufferMap_;\n> +}\n>  \n>  /**\n>   * \\brief Add a FrameBuffer with its associated Stream to the Request\n> @@ -475,7 +479,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n>  \t\treturn -EEXIST;\n>  \t}\n>  \n> -\tauto [it, inserted] = bufferMap_.try_emplace(stream, buffer);\n> +\tauto [it, inserted] = _d()->bufferMap_.try_emplace(stream, buffer);\n>  \tif (!inserted) {\n>  \t\tLOG(Request, Error) << \"FrameBuffer already set for stream\";\n>  \t\treturn -EEXIST;\n> @@ -490,15 +494,6 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n>  \treturn 0;\n>  }\n>  \n> -/**\n> - * \\var Request::bufferMap_\n> - * \\brief Mapping of streams to buffers for this request\n> - *\n> - * The bufferMap_ tracks the buffers associated with each stream. If a stream is\n> - * not utilised in this request there will be no buffer for that stream in the\n> - * map.\n> - */\n\nMaybe move this to Request::Private instead of deleting it ?\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> -\n>  /**\n>   * \\brief Return the buffer associated with a stream\n>   * \\param[in] stream The stream the buffer is associated to\n> @@ -507,20 +502,25 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n>   */\n>  FrameBuffer *Request::findBuffer(const Stream *stream) const\n>  {\n> -\tconst auto it = bufferMap_.find(stream);\n> -\tif (it == bufferMap_.end())\n> +\tconst auto &bufferMap = _d()->bufferMap_;\n> +\n> +\tconst auto it = bufferMap.find(stream);\n> +\tif (it == bufferMap.end())\n>  \t\treturn nullptr;\n>  \n>  \treturn it->second;\n>  }\n>  \n>  /**\n> - * \\fn Request::metadata()\n>   * \\brief Retrieve the request's metadata\n>   * \\todo Offer a read-only API towards applications while keeping a read/write\n>   * API internally.\n>   * \\return The metadata associated with the request\n>   */\n> +ControlList &Request::metadata()\n> +{\n> +\treturn _d()->metadata_;\n> +}\n>  \n>  /**\n>   * \\brief Retrieve the sequence number for the request\n> @@ -543,13 +543,15 @@ uint32_t Request::sequence() const\n>  }\n>  \n>  /**\n> - * \\fn Request::cookie()\n>   * \\brief Retrieve the cookie set when the request was created\n>   * \\return The request cookie\n>   */\n> +uint64_t Request::cookie() const\n> +{\n> +\treturn _d()->cookie_;\n> +}\n>  \n>  /**\n> - * \\fn Request::status()\n>   * \\brief Retrieve the request completion status\n>   *\n>   * The request status indicates whether the request has completed successfully\n> @@ -560,6 +562,10 @@ uint32_t Request::sequence() const\n>   *\n>   * \\return The request completion status\n>   */\n> +Request::Status Request::status() const\n> +{\n> +\treturn _d()->status_;\n> +}\n>  \n>  /**\n>   * \\brief Check if a request has buffers yet to be completed","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5180EC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Apr 2025 22:57:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E4C868967;\n\tWed,  2 Apr 2025 00:57:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5189A62C65\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  2 Apr 2025 00:57:50 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 969156A2;\n\tWed,  2 Apr 2025 00:55:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"T9mBq5N3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743548157;\n\tbh=fOG1YYq29wgp3yHx39nheAlqPIHRKdqxP9SyCK7ztG0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=T9mBq5N3gq5Cdhu3u0h94pP+AWvZFJ4xUWTD7EXrGlMzE4TrZZOM3cNON36VzsEVD\n\te7GXmBQeziUEQhtbRfh6FkyRwFfPUiU/iG6hdT6/g8Z6iKX74dq4DcU7cXK8KXrVyX\n\tsaM22W0GfRNAfa7angsVJ7uxp2eQau6wNyXKOYew=","Date":"Wed, 2 Apr 2025 01:57:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: request: Move all members to internal\n\tprivate class","Message-ID":"<20250401225724.GN3494@pendragon.ideasonboard.com>","References":"<20250331141714.512538-1-barnabas.pocze@ideasonboard.com>\n\t<20250331141714.512538-3-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250331141714.512538-3-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]