[{"id":37598,"web_url":"https://patchwork.libcamera.org/comment/37598/","msgid":"<9f878c95-c077-4ee8-9c7a-5d1173a0a777@ideasonboard.com>","date":"2026-01-13T12:31:06","subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> The Request class has a set of private member variables, with some of\n> them stored in the Request class itself, and some in the\n> Request::Private class. Storing the variables in the Request class\n> itself has the advantage that accessors can be inline, at the cost of\n> ABI breakage if variables need to be added, removed or otherwise\n> modified.\n> \n> The controls_ and metadata_ variables have recently been turned from\n> pointers to instances. This broke the ABI. To avoid further breakages,\n> move all remaining private member variables to Request::Private. The\n> performance impact of not inlining accessors will be negligible.\n\nI feel like this Request class is moving into a suboptimal direction.\nAfter this change a `Request` is essentially just a single pointer to\nits `Private` class. But `std::unique_ptr` is what is used\nin e.g. `Camera::createRequest()`, so essentially it is just introducing\nunnecessary indirection, and increases the number of allocations.\n\nBut short of turning `Request` into a `RequestHandle` kind of thing,\nI am not sure what would be a good approach.\n\n> \n> While at it, drop an unneeded class forward declaration.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/request.h | 12 ++-\n> include/libcamera/request.h | 15 +---\n> src/libcamera/request.cpp | 106 +++++++++++++++------------\n> 3 files changed, 71 insertions(+), 62 deletions(-)\n> \n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 693097ee9a26..7715077b3f7c 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> @@ -41,7 +41,7 @@ public:\n> \tbool completeBuffer(FrameBuffer *buffer);\n> \tvoid complete();\n> \tvoid cancel();\n> -\tvoid reset();\n> +\tvoid reset(Request::ReuseFlag flags);\n> \n> \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n> \tSignal<> prepared;\n> @@ -56,14 +56,20 @@ private:\n> \tvoid timeout();\n> \n> \tCamera *camera_;\n> +\tconst uint64_t cookie_;\n> +\n> +\tStatus status_;\n> \tbool cancelled_;\n> \tuint32_t sequence_ = 0;\n> \tbool prepared_ = false;\n> \n> +\tControlList controls_;\n> +\tControlList metadata_;\n> +\tBufferMap bufferMap_;\n> +\n> \tstd::unordered_set pending_;\n> \tstd::map notifiers_;\n> \tstd::unique_ptr timer_;\n> -\tControlList metadata_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 290983f61352..08ac6e8daba7 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -22,7 +22,6 @@\n> namespace libcamera {\n> \n> class Camera;\n> -class CameraControlValidator;\n> class FrameBuffer;\n> class Stream;\n> \n> @@ -49,16 +48,16 @@ public:\n> \n> \tvoid reuse(ReuseFlag flags = Default);\n> \n> -\tControlList &controls() { return controls_; }\n> +\tControlList &controls();\n> \tconst ControlList &metadata() const;\n> -\tconst BufferMap &buffers() const { return bufferMap_; }\n> +\tconst BufferMap &buffers() const;\n> \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\t std::unique_ptr &&fence = {});\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,12 +65,6 @@ public:\n> \n> private:\n> \tLIBCAMERA_DISABLE_COPY(Request)\n> -\n> -\tControlList controls_;\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 57f1f060d5b4..9d30091a9af7 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -52,12 +52,16 @@ LOG_DEFINE_CATEGORY(Request)\n> \n> /**\n> * \\brief Create a Request::Private\n> - * \\param camera The Camera that creates the request\n> + * \\param[in] camera The Camera that creates the request\n> + * \\param[in] cookie Opaque cookie for application use\n> *\n> * \\todo Add a validator for metadata controls.\n> */\n> -Request::Private::Private(Camera *camera)\n> -\t: camera_(camera), cancelled_(false), metadata_(controls::controls)\n> +Request::Private::Private(Camera *camera, uint64_t cookie)\n> +\t: camera_(camera), cookie_(cookie), status_(RequestPending),\n> +\t cancelled_(false),\n> +\t controls_(camera->controls(), camera->_d()->validator()),\n> +\t metadata_(controls::controls)\n> {\n> }\n> \n> @@ -132,7 +136,7 @@ void Request::Private::complete()\n> \tASSERT(request->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> @@ -174,18 +178,38 @@ void Request::Private::cancel()\n> \n> /**\n> * \\brief Reset the request internal data to default values\n> + * \\param[in] flags Indicate whether or not to reuse the buffers\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> + * values as if the Request::Private instance had just been constructed, with\n> + * the exception of bufferMap_ if the Request::ReuseFlag::ReuseBuffers flag is\n> + * set in \\a flags.\n> */\n> -void Request::Private::reset()\n> +void Request::Private::reset(Request::ReuseFlag flags)\n> {\n> -\tsequence_ = 0;\n> +\tstatus_ = RequestPending;\n> \tcancelled_ = false;\n> +\tsequence_ = 0;\n> \tprepared_ = false;\n> +\n> +\tcontrols_.clear();\n> +\tmetadata_.clear();\n> +\n> \tpending_.clear();\n> \tnotifiers_.clear();\n> \ttimer_.reset();\n> +\n> +\tif (flags & ReuseBuffers) {\n> +\t\tRequest *request = _o();\n> +\n> +\t\tfor (auto pair : bufferMap_) {\n> +\t\t\tFrameBuffer *buffer = pair.second;\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> @@ -286,9 +310,8 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n> \tASSERT(it != notifiers_.end());\n> \tnotifiers_.erase(it);\n> \n> -\tRequest *request = _o();\n> \tLOG(Request, Debug)\n> -\t\t<< \"Request \" << request->cookie() << \" buffer \" << buffer\n> +\t\t<< \"Request \" << cookie_ << \" buffer \" << buffer\n> \t\t<< \" fence signalled\";\n> \n> \tif (!notifiers_.empty())\n> @@ -305,8 +328,7 @@ void Request::Private::timeout()\n> \tASSERT(!notifiers_.empty());\n> \tnotifiers_.clear();\n> \n> -\tRequest *request = _o();\n> -\tLOG(Request, Debug) << \"Request prepare timeout: \" << request->cookie();\n> +\tLOG(Request, Debug) << \"Request prepare timeout: \" << cookie_;\n> \n> \tcancel();\n> \n> @@ -361,13 +383,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(camera)),\n> -\t controls_(camera->controls(), camera->_d()->validator()),\n> -\t cookie_(cookie), status_(RequestPending)\n> +\t: Extensible(std::make_unique(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> @@ -389,26 +409,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> -\t_d()->metadata_.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> @@ -422,6 +426,10 @@ 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> * \\brief Retrieve the request's metadata\n> @@ -433,14 +441,19 @@ const ControlList &Request::metadata() const\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> - * request to the FrameBuffer the Stream output should be directed to.\n> + * request to the FrameBuffer the Stream output should be directed to. If a\n> + * stream is not utilised in this request there will be no buffer for that\n> + * stream in the map.\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> @@ -493,7 +506,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> @@ -508,15 +521,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> -\n> /**\n> * \\brief Return the buffer associated with a stream\n> * \\param[in] stream The stream the buffer is associated to\n> @@ -525,8 +529,8 @@ 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 it = _d()->bufferMap_.find(stream);\n> +\tif (it == _d()->bufferMap_.end())\n> \t\treturn nullptr;\n> \n> \treturn it->second;\n> @@ -553,13 +557,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> @@ -570,6 +576,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":"","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 305A0BE08B\n\tfor ;\n\tTue, 13 Jan 2026 12:31:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38E6F61FC5;\n\tTue, 13 Jan 2026 13:31:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A0D8061FA3\n\tfor ;\n\tTue, 13 Jan 2026 13:31:09 +0100 (CET)","from [192.168.33.30] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8FA3A50A;\n\tTue, 13 Jan 2026 13:30:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PQGCilVe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768307443;\n\tbh=X5UYogJi/6qUZLZqANLEAz8igA/fnNPsgQx3MEN6aHQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=PQGCilVe7/CaY82Z/0alQMaWosRD6ZdrXarUowrwkfKcJy+Q97D4K9tmSoh/El54g\n\tdWnuGpuWioMdR3ZbBkvAUyofhCaYP4jqRVTRdyiG2gw9KRNYJC6HI+JTw2t7o765GW\n\tcKxJRrcqnt3kCzf6NTJZBULNY8vIcTS8rBkVb0N8=","Message-ID":"<9f878c95-c077-4ee8-9c7a-5d1173a0a777@ideasonboard.com>","Date":"Tue, 13 Jan 2026 13:31:06 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-3-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260113000808.15395-3-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37602,"web_url":"https://patchwork.libcamera.org/comment/37602/","msgid":"<20260113134615.GF6198@pendragon.ideasonboard.com>","date":"2026-01-13T13:46:15","subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 13, 2026 at 01:31:06PM +0100, Barnabás Pőcze wrote:\n> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> > The Request class has a set of private member variables, with some of\n> > them stored in the Request class itself, and some in the\n> > Request::Private class. Storing the variables in the Request class\n> > itself has the advantage that accessors can be inline, at the cost of\n> > ABI breakage if variables need to be added, removed or otherwise\n> > modified.\n> > \n> > The controls_ and metadata_ variables have recently been turned from\n> > pointers to instances. This broke the ABI. To avoid further breakages,\n> > move all remaining private member variables to Request::Private. The\n> > performance impact of not inlining accessors will be negligible.\n> \n> I feel like this Request class is moving into a suboptimal direction.\n> After this change a `Request` is essentially just a single pointer to\n> its `Private` class. But `std::unique_ptr` is what is used\n> in e.g. `Camera::createRequest()`, so essentially it is just introducing\n> unnecessary indirection, and increases the number of allocations.\n> \n> But short of turning `Request` into a `RequestHandle` kind of thing,\n> I am not sure what would be a good approach.\n\nI agree with you, the class effectively becomes a handle.\n\nAs applications are not supposed to allocate Request instances manually,\nwe could replace the Request::Private mechanism with inheritance.\nRequest::Private could derive from Request, and Camera::createRequest()\nwould allocate an instance of Request::Private and return a pointer to\nRequest. That would however require a virtual destructor. This would\nbreak the ABI but not the API.\n\nThis being said, I don't think the current implementation causes a real\nperformance issue, so I would prefer experimenting with it later and\nseeing if we should apply the same concept to more classes instead of\nrushing it out for Request right now.\n\n> > While at it, drop an unneeded class forward declaration.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/request.h | 12 ++-\n> > include/libcamera/request.h | 15 +---\n> > src/libcamera/request.cpp | 106 +++++++++++++++------------\n> > 3 files changed, 71 insertions(+), 62 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > index 693097ee9a26..7715077b3f7c 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> > @@ -41,7 +41,7 @@ public:\n> > \tbool completeBuffer(FrameBuffer *buffer);\n> > \tvoid complete();\n> > \tvoid cancel();\n> > -\tvoid reset();\n> > +\tvoid reset(Request::ReuseFlag flags);\n> > \n> > \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n> > \tSignal<> prepared;\n> > @@ -56,14 +56,20 @@ private:\n> > \tvoid timeout();\n> > \n> > \tCamera *camera_;\n> > +\tconst uint64_t cookie_;\n> > +\n> > +\tStatus status_;\n> > \tbool cancelled_;\n> > \tuint32_t sequence_ = 0;\n> > \tbool prepared_ = false;\n> > \n> > +\tControlList controls_;\n> > +\tControlList metadata_;\n> > +\tBufferMap bufferMap_;\n> > +\n> > \tstd::unordered_set pending_;\n> > \tstd::map notifiers_;\n> > \tstd::unique_ptr timer_;\n> > -\tControlList metadata_;\n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index 290983f61352..08ac6e8daba7 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -22,7 +22,6 @@\n> > namespace libcamera {\n> > \n> > class Camera;\n> > -class CameraControlValidator;\n> > class FrameBuffer;\n> > class Stream;\n> > \n> > @@ -49,16 +48,16 @@ public:\n> > \n> > \tvoid reuse(ReuseFlag flags = Default);\n> > \n> > -\tControlList &controls() { return controls_; }\n> > +\tControlList &controls();\n> > \tconst ControlList &metadata() const;\n> > -\tconst BufferMap &buffers() const { return bufferMap_; }\n> > +\tconst BufferMap &buffers() const;\n> > \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > \t\t std::unique_ptr &&fence = {});\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,12 +65,6 @@ public:\n> > \n> > private:\n> > \tLIBCAMERA_DISABLE_COPY(Request)\n> > -\n> > -\tControlList controls_;\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 57f1f060d5b4..9d30091a9af7 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -52,12 +52,16 @@ LOG_DEFINE_CATEGORY(Request)\n> > \n> > /**\n> > * \\brief Create a Request::Private\n> > - * \\param camera The Camera that creates the request\n> > + * \\param[in] camera The Camera that creates the request\n> > + * \\param[in] cookie Opaque cookie for application use\n> > *\n> > * \\todo Add a validator for metadata controls.\n> > */\n> > -Request::Private::Private(Camera *camera)\n> > -\t: camera_(camera), cancelled_(false), metadata_(controls::controls)\n> > +Request::Private::Private(Camera *camera, uint64_t cookie)\n> > +\t: camera_(camera), cookie_(cookie), status_(RequestPending),\n> > +\t cancelled_(false),\n> > +\t controls_(camera->controls(), camera->_d()->validator()),\n> > +\t metadata_(controls::controls)\n> > {\n> > }\n> > \n> > @@ -132,7 +136,7 @@ void Request::Private::complete()\n> > \tASSERT(request->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> > @@ -174,18 +178,38 @@ void Request::Private::cancel()\n> > \n> > /**\n> > * \\brief Reset the request internal data to default values\n> > + * \\param[in] flags Indicate whether or not to reuse the buffers\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> > + * values as if the Request::Private instance had just been constructed, with\n> > + * the exception of bufferMap_ if the Request::ReuseFlag::ReuseBuffers flag is\n> > + * set in \\a flags.\n> > */\n> > -void Request::Private::reset()\n> > +void Request::Private::reset(Request::ReuseFlag flags)\n> > {\n> > -\tsequence_ = 0;\n> > +\tstatus_ = RequestPending;\n> > \tcancelled_ = false;\n> > +\tsequence_ = 0;\n> > \tprepared_ = false;\n> > +\n> > +\tcontrols_.clear();\n> > +\tmetadata_.clear();\n> > +\n> > \tpending_.clear();\n> > \tnotifiers_.clear();\n> > \ttimer_.reset();\n> > +\n> > +\tif (flags & ReuseBuffers) {\n> > +\t\tRequest *request = _o();\n> > +\n> > +\t\tfor (auto pair : bufferMap_) {\n> > +\t\t\tFrameBuffer *buffer = pair.second;\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> > @@ -286,9 +310,8 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n> > \tASSERT(it != notifiers_.end());\n> > \tnotifiers_.erase(it);\n> > \n> > -\tRequest *request = _o();\n> > \tLOG(Request, Debug)\n> > -\t\t<< \"Request \" << request->cookie() << \" buffer \" << buffer\n> > +\t\t<< \"Request \" << cookie_ << \" buffer \" << buffer\n> > \t\t<< \" fence signalled\";\n> > \n> > \tif (!notifiers_.empty())\n> > @@ -305,8 +328,7 @@ void Request::Private::timeout()\n> > \tASSERT(!notifiers_.empty());\n> > \tnotifiers_.clear();\n> > \n> > -\tRequest *request = _o();\n> > -\tLOG(Request, Debug) << \"Request prepare timeout: \" << request->cookie();\n> > +\tLOG(Request, Debug) << \"Request prepare timeout: \" << cookie_;\n> > \n> > \tcancel();\n> > \n> > @@ -361,13 +383,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(camera)),\n> > -\t controls_(camera->controls(), camera->_d()->validator()),\n> > -\t cookie_(cookie), status_(RequestPending)\n> > +\t: Extensible(std::make_unique(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> > @@ -389,26 +409,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> > -\t_d()->metadata_.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> > @@ -422,6 +426,10 @@ 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> > * \\brief Retrieve the request's metadata\n> > @@ -433,14 +441,19 @@ const ControlList &Request::metadata() const\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> > - * request to the FrameBuffer the Stream output should be directed to.\n> > + * request to the FrameBuffer the Stream output should be directed to. If a\n> > + * stream is not utilised in this request there will be no buffer for that\n> > + * stream in the map.\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> > @@ -493,7 +506,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> > @@ -508,15 +521,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> > -\n> > /**\n> > * \\brief Return the buffer associated with a stream\n> > * \\param[in] stream The stream the buffer is associated to\n> > @@ -525,8 +529,8 @@ 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 it = _d()->bufferMap_.find(stream);\n> > +\tif (it == _d()->bufferMap_.end())\n> > \t\treturn nullptr;\n> > \n> > \treturn it->second;\n> > @@ -553,13 +557,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> > @@ -570,6 +576,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":"","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 07B33BE08B\n\tfor ;\n\tTue, 13 Jan 2026 13:46:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4962061FBC;\n\tTue, 13 Jan 2026 14:46:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18BEB61FA0\n\tfor ;\n\tTue, 13 Jan 2026 14:46:36 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B90F250A;\n\tTue, 13 Jan 2026 14:46:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"T0//fgcl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768311969;\n\tbh=CUenybphyJiapTEuDiHCJqT7c2jjP6UdW1n155357S0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=T0//fgclRk3aMkliZDd4yA3Qi40mSaqbXPje9qKMcRnvUJsI89J400MFzpe5Y7DzG\n\tgxkYsNL3CGfyXNleh+M/MPTZ1jNLiDVbRTLZTh8tH+ag5a4+61HK94R1TMxu6Ely+Y\n\txAPpq1bkqDR3sVGf57b94fa1evSGbcf4aR1dv5ns=","Date":"Tue, 13 Jan 2026 15:46:15 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","Message-ID":"<20260113134615.GF6198@pendragon.ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-3-laurent.pinchart@ideasonboard.com>\n\t<9f878c95-c077-4ee8-9c7a-5d1173a0a777@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9f878c95-c077-4ee8-9c7a-5d1173a0a777@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37640,"web_url":"https://patchwork.libcamera.org/comment/37640/","msgid":"<176838417560.20276.5540328246440840412@localhost>","date":"2026-01-14T09:49:35","subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nQuoting Laurent Pinchart (2026-01-13 14:46:15)\n> On Tue, Jan 13, 2026 at 01:31:06PM +0100, Barnabás Pőcze wrote:\n> > 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> > > The Request class has a set of private member variables, with some of\n> > > them stored in the Request class itself, and some in the\n> > > Request::Private class. Storing the variables in the Request class\n> > > itself has the advantage that accessors can be inline, at the cost of\n> > > ABI breakage if variables need to be added, removed or otherwise\n> > > modified.\n> > > \n> > > The controls_ and metadata_ variables have recently been turned from\n> > > pointers to instances. This broke the ABI. To avoid further breakages,\n> > > move all remaining private member variables to Request::Private. The\n> > > performance impact of not inlining accessors will be negligible.\n> > \n> > I feel like this Request class is moving into a suboptimal direction.\n> > After this change a `Request` is essentially just a single pointer to\n> > its `Private` class. But `std::unique_ptr` is what is used\n> > in e.g. `Camera::createRequest()`, so essentially it is just introducing\n> > unnecessary indirection, and increases the number of allocations.\n> > \n> > But short of turning `Request` into a `RequestHandle` kind of thing,\n> > I am not sure what would be a good approach.\n\nI share similar concerns of Barnabás. Isn't this kind of mixing two\nconcepts? Moving everything into an internal implementation is the pimpl\npattern and helps in keeping the ABI stable. My understanding of\nRequest::Private class was that it is thought for private data that\nshould not be visible to the external application. I see that it could\nalso be used to pimpl. But I am not sure if that was the intention. \n\nThere are other classes like StreamConfiguration that we might want to\nchange in the future in an ABI breaking way, so I don't see (without\npimpling everything) how we could completely prevent ABI breaks.\n\nSo I miss the incentive of breaking the request ABI again (without\nimmediate need) if we don't decide for a strategy libcamera wide.\n\nRegards,\nStefan\n\n> \n> I agree with you, the class effectively becomes a handle.\n> \n> As applications are not supposed to allocate Request instances manually,\n> we could replace the Request::Private mechanism with inheritance.\n> Request::Private could derive from Request, and Camera::createRequest()\n> would allocate an instance of Request::Private and return a pointer to\n> Request. That would however require a virtual destructor. This would\n> break the ABI but not the API.\n> \n> This being said, I don't think the current implementation causes a real\n> performance issue, so I would prefer experimenting with it later and\n> seeing if we should apply the same concept to more classes instead of\n> rushing it out for Request right now.\n> \n> > > While at it, drop an unneeded class forward declaration.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/internal/request.h | 12 ++-\n> > > include/libcamera/request.h | 15 +---\n> > > src/libcamera/request.cpp | 106 +++++++++++++++------------\n> > > 3 files changed, 71 insertions(+), 62 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > > index 693097ee9a26..7715077b3f7c 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> > > @@ -41,7 +41,7 @@ public:\n> > > bool completeBuffer(FrameBuffer *buffer);\n> > > void complete();\n> > > void cancel();\n> > > - void reset();\n> > > + void reset(Request::ReuseFlag flags);\n> > > \n> > > void prepare(std::chrono::milliseconds timeout = 0ms);\n> > > Signal<> prepared;\n> > > @@ -56,14 +56,20 @@ private:\n> > > void timeout();\n> > > \n> > > Camera *camera_;\n> > > + const uint64_t cookie_;\n> > > +\n> > > + Status status_;\n> > > bool cancelled_;\n> > > uint32_t sequence_ = 0;\n> > > bool prepared_ = false;\n> > > \n> > > + ControlList controls_;\n> > > + ControlList metadata_;\n> > > + BufferMap bufferMap_;\n> > > +\n> > > std::unordered_set pending_;\n> > > std::map notifiers_;\n> > > std::unique_ptr timer_;\n> > > - ControlList metadata_;\n> > > };\n> > > \n> > > } /* namespace libcamera */\n> > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > index 290983f61352..08ac6e8daba7 100644\n> > > --- a/include/libcamera/request.h\n> > > +++ b/include/libcamera/request.h\n> > > @@ -22,7 +22,6 @@\n> > > namespace libcamera {\n> > > \n> > > class Camera;\n> > > -class CameraControlValidator;\n> > > class FrameBuffer;\n> > > class Stream;\n> > > \n> > > @@ -49,16 +48,16 @@ public:\n> > > \n> > > void reuse(ReuseFlag flags = Default);\n> > > \n> > > - ControlList &controls() { return controls_; }\n> > > + ControlList &controls();\n> > > const ControlList &metadata() const;\n> > > - const BufferMap &buffers() const { return bufferMap_; }\n> > > + const BufferMap &buffers() const;\n> > > int addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > > std::unique_ptr &&fence = {});\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,12 +65,6 @@ public:\n> > > \n> > > private:\n> > > LIBCAMERA_DISABLE_COPY(Request)\n> > > -\n> > > - ControlList controls_;\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 57f1f060d5b4..9d30091a9af7 100644\n> > > --- a/src/libcamera/request.cpp\n> > > +++ b/src/libcamera/request.cpp\n> > > @@ -52,12 +52,16 @@ LOG_DEFINE_CATEGORY(Request)\n> > > \n> > > /**\n> > > * \\brief Create a Request::Private\n> > > - * \\param camera The Camera that creates the request\n> > > + * \\param[in] camera The Camera that creates the request\n> > > + * \\param[in] cookie Opaque cookie for application use\n> > > *\n> > > * \\todo Add a validator for metadata controls.\n> > > */\n> > > -Request::Private::Private(Camera *camera)\n> > > - : camera_(camera), cancelled_(false), metadata_(controls::controls)\n> > > +Request::Private::Private(Camera *camera, uint64_t cookie)\n> > > + : camera_(camera), cookie_(cookie), status_(RequestPending),\n> > > + cancelled_(false),\n> > > + controls_(camera->controls(), camera->_d()->validator()),\n> > > + metadata_(controls::controls)\n> > > {\n> > > }\n> > > \n> > > @@ -132,7 +136,7 @@ void Request::Private::complete()\n> > > ASSERT(request->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> > > @@ -174,18 +178,38 @@ void Request::Private::cancel()\n> > > \n> > > /**\n> > > * \\brief Reset the request internal data to default values\n> > > + * \\param[in] flags Indicate whether or not to reuse the buffers\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> > > + * values as if the Request::Private instance had just been constructed, with\n> > > + * the exception of bufferMap_ if the Request::ReuseFlag::ReuseBuffers flag is\n> > > + * set in \\a flags.\n> > > */\n> > > -void Request::Private::reset()\n> > > +void Request::Private::reset(Request::ReuseFlag flags)\n> > > {\n> > > - sequence_ = 0;\n> > > + status_ = RequestPending;\n> > > cancelled_ = false;\n> > > + sequence_ = 0;\n> > > prepared_ = false;\n> > > +\n> > > + controls_.clear();\n> > > + metadata_.clear();\n> > > +\n> > > pending_.clear();\n> > > notifiers_.clear();\n> > > timer_.reset();\n> > > +\n> > > + if (flags & ReuseBuffers) {\n> > > + Request *request = _o();\n> > > +\n> > > + for (auto pair : bufferMap_) {\n> > > + FrameBuffer *buffer = pair.second;\n> > > + buffer->_d()->setRequest(request);\n> > > + pending_.insert(buffer);\n> > > + }\n> > > + } else {\n> > > + bufferMap_.clear();\n> > > + }\n> > > }\n> > > \n> > > /*\n> > > @@ -286,9 +310,8 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n> > > ASSERT(it != notifiers_.end());\n> > > notifiers_.erase(it);\n> > > \n> > > - Request *request = _o();\n> > > LOG(Request, Debug)\n> > > - << \"Request \" << request->cookie() << \" buffer \" << buffer\n> > > + << \"Request \" << cookie_ << \" buffer \" << buffer\n> > > << \" fence signalled\";\n> > > \n> > > if (!notifiers_.empty())\n> > > @@ -305,8 +328,7 @@ void Request::Private::timeout()\n> > > ASSERT(!notifiers_.empty());\n> > > notifiers_.clear();\n> > > \n> > > - Request *request = _o();\n> > > - LOG(Request, Debug) << \"Request prepare timeout: \" << request->cookie();\n> > > + LOG(Request, Debug) << \"Request prepare timeout: \" << cookie_;\n> > > \n> > > cancel();\n> > > \n> > > @@ -361,13 +383,11 @@ void Request::Private::timeout()\n> > > * completely opaque to libcamera.\n> > > */\n> > > Request::Request(Camera *camera, uint64_t cookie)\n> > > - : Extensible(std::make_unique(camera)),\n> > > - controls_(camera->controls(), camera->_d()->validator()),\n> > > - cookie_(cookie), status_(RequestPending)\n> > > + : Extensible(std::make_unique(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> > > @@ -389,26 +409,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> > > - _d()->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> > > @@ -422,6 +426,10 @@ 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> > > * \\brief Retrieve the request's metadata\n> > > @@ -433,14 +441,19 @@ const ControlList &Request::metadata() const\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> > > - * request to the FrameBuffer the Stream output should be directed to.\n> > > + * request to the FrameBuffer the Stream output should be directed to. If a\n> > > + * stream is not utilised in this request there will be no buffer for that\n> > > + * stream in the map.\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> > > @@ -493,7 +506,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> > > @@ -508,15 +521,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> > > @@ -525,8 +529,8 @@ 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 it = _d()->bufferMap_.find(stream);\n> > > + if (it == _d()->bufferMap_.end())\n> > > return nullptr;\n> > > \n> > > return it->second;\n> > > @@ -553,13 +557,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> > > @@ -570,6 +576,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> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","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 ED0CFBDCBF\n\tfor ;\n\tWed, 14 Jan 2026 09:49:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14F5061FC4;\n\tWed, 14 Jan 2026 10:49:40 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3DC4F615B2\n\tfor ;\n\tWed, 14 Jan 2026 10:49:38 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:b781:dff2:957:7831])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 67766316;\n\tWed, 14 Jan 2026 10:49:11 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gLS2IhZD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768384151;\n\tbh=Vmbv/0LRdgryJ1fK7gw+3zpLiQCR+kP9iMvSu3LGqxg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gLS2IhZDMBg45JXm6HE1qKe5seJDZXu0N2dvWkNARNssmKwHDIWQuGGOnYZ+akd4J\n\tLPVT2BFqeAFd/gkLV4Te8DEYN1R5gNDCKWuT94JH4C3DIpNQxxadsyV8z5f0ywyRJV\n\t9lJBpbQSpFpyGQBSEtVdM1iVmhmclir15eqHq9Ng=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260113134615.GF6198@pendragon.ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-3-laurent.pinchart@ideasonboard.com>\n\t<9f878c95-c077-4ee8-9c7a-5d1173a0a777@ideasonboard.com>\n\t<20260113134615.GF6198@pendragon.ideasonboard.com>","Subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart ","Date":"Wed, 14 Jan 2026 10:49:35 +0100","Message-ID":"<176838417560.20276.5540328246440840412@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37710,"web_url":"https://patchwork.libcamera.org/comment/37710/","msgid":"<20260118224730.GA7759@pendragon.ideasonboard.com>","date":"2026-01-18T22:47:30","subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jan 14, 2026 at 10:49:35AM +0100, Stefan Klug wrote:\n> Quoting Laurent Pinchart (2026-01-13 14:46:15)\n> > On Tue, Jan 13, 2026 at 01:31:06PM +0100, Barnabás Pőcze wrote:\n> > > 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> > > > The Request class has a set of private member variables, with some of\n> > > > them stored in the Request class itself, and some in the\n> > > > Request::Private class. Storing the variables in the Request class\n> > > > itself has the advantage that accessors can be inline, at the cost of\n> > > > ABI breakage if variables need to be added, removed or otherwise\n> > > > modified.\n> > > > \n> > > > The controls_ and metadata_ variables have recently been turned from\n> > > > pointers to instances. This broke the ABI. To avoid further breakages,\n> > > > move all remaining private member variables to Request::Private. The\n> > > > performance impact of not inlining accessors will be negligible.\n> > > \n> > > I feel like this Request class is moving into a suboptimal direction.\n> > > After this change a `Request` is essentially just a single pointer to\n> > > its `Private` class. But `std::unique_ptr` is what is used\n> > > in e.g. `Camera::createRequest()`, so essentially it is just introducing\n> > > unnecessary indirection, and increases the number of allocations.\n> > > \n> > > But short of turning `Request` into a `RequestHandle` kind of thing,\n> > > I am not sure what would be a good approach.\n> \n> I share similar concerns of Barnabás. Isn't this kind of mixing two\n> concepts? Moving everything into an internal implementation is the pimpl\n> pattern and helps in keeping the ABI stable. My understanding of\n> Request::Private class was that it is thought for private data that\n> should not be visible to the external application. I see that it could\n> also be used to pimpl. But I am not sure if that was the intention. \n\nThe Private classes are meant to implement the pimpl pattern. They also\nhelp making internal data available internally, but that's not the main\ngoal. We used friend statements for that purpose before. Using Private\nfor this purpose is nicer in my opinion, but it's still not the original\ngoal.\n\n> There are other classes like StreamConfiguration that we might want to\n> change in the future in an ABI breaking way, so I don't see (without\n> pimpling everything) how we could completely prevent ABI breaks.\n\nOnce we'll have a C API everything will be private and opaque, except\nfor arguments passed to functions.\n\n> So I miss the incentive of breaking the request ABI again (without\n> immediate need) if we don't decide for a strategy libcamera wide.\n\nThis patch was written before the last breakage that changed the\nhandling of controls_ and metadata_. I had another patch in the series\nthat turned the dynamically allocated members into unique_ptr, and as\nthat broke the ABI, I decided to make everything private to avoid future\nbreakages. That patch got dropped as the two member variables are now\nembedded in the Request or Request::Private classes, but I still think\nmoving the last members to Request::Private will minimize future ABI\nbreakages.\n\nNote that this particular ABI breakage doesn't change the API, it only\nrequires recompilation. Given that we have other ABI breakages queued in\nthe master branch (if I recall correctly), merging this one will not\nintroduce any further inconvienence.\n\n> > I agree with you, the class effectively becomes a handle.\n> > \n> > As applications are not supposed to allocate Request instances manually,\n> > we could replace the Request::Private mechanism with inheritance.\n> > Request::Private could derive from Request, and Camera::createRequest()\n> > would allocate an instance of Request::Private and return a pointer to\n> > Request. That would however require a virtual destructor. This would\n> > break the ABI but not the API.\n> > \n> > This being said, I don't think the current implementation causes a real\n> > performance issue, so I would prefer experimenting with it later and\n> > seeing if we should apply the same concept to more classes instead of\n> > rushing it out for Request right now.\n> > \n> > > > While at it, drop an unneeded class forward declaration.\n> > > > \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > include/libcamera/internal/request.h | 12 ++-\n> > > > include/libcamera/request.h | 15 +---\n> > > > src/libcamera/request.cpp | 106 +++++++++++++++------------\n> > > > 3 files changed, 71 insertions(+), 62 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > > > index 693097ee9a26..7715077b3f7c 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> > > > @@ -41,7 +41,7 @@ public:\n> > > > bool completeBuffer(FrameBuffer *buffer);\n> > > > void complete();\n> > > > void cancel();\n> > > > - void reset();\n> > > > + void reset(Request::ReuseFlag flags);\n> > > > \n> > > > void prepare(std::chrono::milliseconds timeout = 0ms);\n> > > > Signal<> prepared;\n> > > > @@ -56,14 +56,20 @@ private:\n> > > > void timeout();\n> > > > \n> > > > Camera *camera_;\n> > > > + const uint64_t cookie_;\n> > > > +\n> > > > + Status status_;\n> > > > bool cancelled_;\n> > > > uint32_t sequence_ = 0;\n> > > > bool prepared_ = false;\n> > > > \n> > > > + ControlList controls_;\n> > > > + ControlList metadata_;\n> > > > + BufferMap bufferMap_;\n> > > > +\n> > > > std::unordered_set pending_;\n> > > > std::map notifiers_;\n> > > > std::unique_ptr timer_;\n> > > > - ControlList metadata_;\n> > > > };\n> > > > \n> > > > } /* namespace libcamera */\n> > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > > index 290983f61352..08ac6e8daba7 100644\n> > > > --- a/include/libcamera/request.h\n> > > > +++ b/include/libcamera/request.h\n> > > > @@ -22,7 +22,6 @@\n> > > > namespace libcamera {\n> > > > \n> > > > class Camera;\n> > > > -class CameraControlValidator;\n> > > > class FrameBuffer;\n> > > > class Stream;\n> > > > \n> > > > @@ -49,16 +48,16 @@ public:\n> > > > \n> > > > void reuse(ReuseFlag flags = Default);\n> > > > \n> > > > - ControlList &controls() { return controls_; }\n> > > > + ControlList &controls();\n> > > > const ControlList &metadata() const;\n> > > > - const BufferMap &buffers() const { return bufferMap_; }\n> > > > + const BufferMap &buffers() const;\n> > > > int addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > > > std::unique_ptr &&fence = {});\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,12 +65,6 @@ public:\n> > > > \n> > > > private:\n> > > > LIBCAMERA_DISABLE_COPY(Request)\n> > > > -\n> > > > - ControlList controls_;\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 57f1f060d5b4..9d30091a9af7 100644\n> > > > --- a/src/libcamera/request.cpp\n> > > > +++ b/src/libcamera/request.cpp\n> > > > @@ -52,12 +52,16 @@ LOG_DEFINE_CATEGORY(Request)\n> > > > \n> > > > /**\n> > > > * \\brief Create a Request::Private\n> > > > - * \\param camera The Camera that creates the request\n> > > > + * \\param[in] camera The Camera that creates the request\n> > > > + * \\param[in] cookie Opaque cookie for application use\n> > > > *\n> > > > * \\todo Add a validator for metadata controls.\n> > > > */\n> > > > -Request::Private::Private(Camera *camera)\n> > > > - : camera_(camera), cancelled_(false), metadata_(controls::controls)\n> > > > +Request::Private::Private(Camera *camera, uint64_t cookie)\n> > > > + : camera_(camera), cookie_(cookie), status_(RequestPending),\n> > > > + cancelled_(false),\n> > > > + controls_(camera->controls(), camera->_d()->validator()),\n> > > > + metadata_(controls::controls)\n> > > > {\n> > > > }\n> > > > \n> > > > @@ -132,7 +136,7 @@ void Request::Private::complete()\n> > > > ASSERT(request->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> > > > @@ -174,18 +178,38 @@ void Request::Private::cancel()\n> > > > \n> > > > /**\n> > > > * \\brief Reset the request internal data to default values\n> > > > + * \\param[in] flags Indicate whether or not to reuse the buffers\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> > > > + * values as if the Request::Private instance had just been constructed, with\n> > > > + * the exception of bufferMap_ if the Request::ReuseFlag::ReuseBuffers flag is\n> > > > + * set in \\a flags.\n> > > > */\n> > > > -void Request::Private::reset()\n> > > > +void Request::Private::reset(Request::ReuseFlag flags)\n> > > > {\n> > > > - sequence_ = 0;\n> > > > + status_ = RequestPending;\n> > > > cancelled_ = false;\n> > > > + sequence_ = 0;\n> > > > prepared_ = false;\n> > > > +\n> > > > + controls_.clear();\n> > > > + metadata_.clear();\n> > > > +\n> > > > pending_.clear();\n> > > > notifiers_.clear();\n> > > > timer_.reset();\n> > > > +\n> > > > + if (flags & ReuseBuffers) {\n> > > > + Request *request = _o();\n> > > > +\n> > > > + for (auto pair : bufferMap_) {\n> > > > + FrameBuffer *buffer = pair.second;\n> > > > + buffer->_d()->setRequest(request);\n> > > > + pending_.insert(buffer);\n> > > > + }\n> > > > + } else {\n> > > > + bufferMap_.clear();\n> > > > + }\n> > > > }\n> > > > \n> > > > /*\n> > > > @@ -286,9 +310,8 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n> > > > ASSERT(it != notifiers_.end());\n> > > > notifiers_.erase(it);\n> > > > \n> > > > - Request *request = _o();\n> > > > LOG(Request, Debug)\n> > > > - << \"Request \" << request->cookie() << \" buffer \" << buffer\n> > > > + << \"Request \" << cookie_ << \" buffer \" << buffer\n> > > > << \" fence signalled\";\n> > > > \n> > > > if (!notifiers_.empty())\n> > > > @@ -305,8 +328,7 @@ void Request::Private::timeout()\n> > > > ASSERT(!notifiers_.empty());\n> > > > notifiers_.clear();\n> > > > \n> > > > - Request *request = _o();\n> > > > - LOG(Request, Debug) << \"Request prepare timeout: \" << request->cookie();\n> > > > + LOG(Request, Debug) << \"Request prepare timeout: \" << cookie_;\n> > > > \n> > > > cancel();\n> > > > \n> > > > @@ -361,13 +383,11 @@ void Request::Private::timeout()\n> > > > * completely opaque to libcamera.\n> > > > */\n> > > > Request::Request(Camera *camera, uint64_t cookie)\n> > > > - : Extensible(std::make_unique(camera)),\n> > > > - controls_(camera->controls(), camera->_d()->validator()),\n> > > > - cookie_(cookie), status_(RequestPending)\n> > > > + : Extensible(std::make_unique(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> > > > @@ -389,26 +409,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> > > > - _d()->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> > > > @@ -422,6 +426,10 @@ 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> > > > * \\brief Retrieve the request's metadata\n> > > > @@ -433,14 +441,19 @@ const ControlList &Request::metadata() const\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> > > > - * request to the FrameBuffer the Stream output should be directed to.\n> > > > + * request to the FrameBuffer the Stream output should be directed to. If a\n> > > > + * stream is not utilised in this request there will be no buffer for that\n> > > > + * stream in the map.\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> > > > @@ -493,7 +506,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> > > > @@ -508,15 +521,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> > > > @@ -525,8 +529,8 @@ 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 it = _d()->bufferMap_.find(stream);\n> > > > + if (it == _d()->bufferMap_.end())\n> > > > return nullptr;\n> > > > \n> > > > return it->second;\n> > > > @@ -553,13 +557,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> > > > @@ -570,6 +576,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","headers":{"Return-Path":"","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 CF50CBDCBF\n\tfor ;\n\tSun, 18 Jan 2026 22:47:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB7E961FBC;\n\tSun, 18 Jan 2026 23:47:53 +0100 (CET)","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 0931E61FB7\n\tfor ;\n\tSun, 18 Jan 2026 23:47:53 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id AEA8522A;\n\tSun, 18 Jan 2026 23:47:22 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dk36owz+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768776442;\n\tbh=XXvskCksjCzvxV/SwcpKmDKioS5ND9KmSgzqifeAl3E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dk36owz+KmIN23YSS+xFBmnvEctvf6eEcFMCziZuyMw3SDEzuLh7R+h3KGwFj2d3D\n\tMJk0WEL+eyyBL7FGPEFE8ZDbWsscbKLZ98tkiTDwbL5pzFHFkRSPQhArUCR0DSis8s\n\tURtzcVl9z34UKAOiIvG37M/p5nza0VQ068niZ7U4=","Date":"Mon, 19 Jan 2026 00:47:30 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 02/36] libcamera: request: Move all private member\n\tvariables to Private class","Message-ID":"<20260118224730.GA7759@pendragon.ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-3-laurent.pinchart@ideasonboard.com>\n\t<9f878c95-c077-4ee8-9c7a-5d1173a0a777@ideasonboard.com>\n\t<20260113134615.GF6198@pendragon.ideasonboard.com>\n\t<176838417560.20276.5540328246440840412@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<176838417560.20276.5540328246440840412@localhost>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]