| Message ID | 20260505082310.604873-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Tue, May 05, 2026 at 10:23:10AM +0200, Barnabás Pőcze wrote: > First of all, move most of the documentation next to the `setCookie()` function > to have it in a single place. > > Secondly, make it clear that applications using a `FrameBufferAllocator` are > considered to be owners of the allocated FrameBuffer objects, and they may > freely manage the cookies of those frame buffers. > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/331 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/framebuffer.cpp | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp > index 765dab95a..acf5b5b33 100644 > --- a/src/libcamera/framebuffer.cpp > +++ b/src/libcamera/framebuffer.cpp > @@ -236,12 +236,6 @@ FrameBuffer::Private::~Private() > * during the processing of a queued capture request, and is valid from the > * completion of the buffer as signaled by Camera::bufferComplete() until the > * FrameBuffer is either reused in a new request or deleted. > - * > - * The creator of a FrameBuffer (application, IPA or pipeline handler) may > - * associate to it an integer cookie for any private purpose. The cookie may be > - * set when creating the FrameBuffer, and updated at any time with setCookie(). > - * The cookie is transparent to the libcamera core and shall only be set by the > - * creator of the FrameBuffer. This mechanism supplements the Request cookie. > */ > > /** > @@ -399,12 +393,7 @@ const FrameMetadata &FrameBuffer::metadata() const > > /** > * \brief Retrieve the cookie > - * > - * The cookie belongs to the creator of the FrameBuffer, which controls its > - * lifetime and value. > - * > * \sa setCookie() > - * > * \return The cookie > */ > uint64_t FrameBuffer::cookie() const > @@ -416,10 +405,23 @@ uint64_t FrameBuffer::cookie() const > * \brief Set the cookie > * \param[in] cookie Cookie to set > * > - * The cookie belongs to the creator of the FrameBuffer. Its value may be > - * modified at any time with this function. Applications and IPAs shall not > - * modify the cookie value of buffers they haven't created themselves. The > - * libcamera core never modifies the buffer cookie. > + * The creator (and only the creator) of the FrameBuffer may store an arbitrary > + * 64-bit integer value in the FrameBuffer, this value is called the cookie. > + * It may be retrieved or set at any time, and is guaranteed not to be modified > + * by the libcamera core. This mechanism is similar to the \ref Request cookie. > + * > + * An application using a \ref FrameBufferAllocator is considered to be the creator > + * of the allocated FrameBuffer objects, and thus it may manage the cookie as it > + * sees fit. I've always considered that to be obvious, so never thought about making it clear in the documentation. Thanks for fixing it, it's hard to see shortcomings in documentation when you are too familiar with the code. > + * > + * \internal > + * Similar rules apply to pipeline handlers and IPA modules, that is, they can only > + * manage the cookies of FrameBuffer objects that they have created, with the exception > + * that any FrameBuffer returned by \ref PipelineHandler::exportFrameBuffers() is considered > + * to have been created by the application, and thus its cookie shall not be modified. > + * \endinternal Please reflow the documentation to 80 columns. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * > + * \sa cookie() > */ > void FrameBuffer::setCookie(uint64_t cookie) > {
diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index 765dab95a..acf5b5b33 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -236,12 +236,6 @@ FrameBuffer::Private::~Private() * during the processing of a queued capture request, and is valid from the * completion of the buffer as signaled by Camera::bufferComplete() until the * FrameBuffer is either reused in a new request or deleted. - * - * The creator of a FrameBuffer (application, IPA or pipeline handler) may - * associate to it an integer cookie for any private purpose. The cookie may be - * set when creating the FrameBuffer, and updated at any time with setCookie(). - * The cookie is transparent to the libcamera core and shall only be set by the - * creator of the FrameBuffer. This mechanism supplements the Request cookie. */ /** @@ -399,12 +393,7 @@ const FrameMetadata &FrameBuffer::metadata() const /** * \brief Retrieve the cookie - * - * The cookie belongs to the creator of the FrameBuffer, which controls its - * lifetime and value. - * * \sa setCookie() - * * \return The cookie */ uint64_t FrameBuffer::cookie() const @@ -416,10 +405,23 @@ uint64_t FrameBuffer::cookie() const * \brief Set the cookie * \param[in] cookie Cookie to set * - * The cookie belongs to the creator of the FrameBuffer. Its value may be - * modified at any time with this function. Applications and IPAs shall not - * modify the cookie value of buffers they haven't created themselves. The - * libcamera core never modifies the buffer cookie. + * The creator (and only the creator) of the FrameBuffer may store an arbitrary + * 64-bit integer value in the FrameBuffer, this value is called the cookie. + * It may be retrieved or set at any time, and is guaranteed not to be modified + * by the libcamera core. This mechanism is similar to the \ref Request cookie. + * + * An application using a \ref FrameBufferAllocator is considered to be the creator + * of the allocated FrameBuffer objects, and thus it may manage the cookie as it + * sees fit. + * + * \internal + * Similar rules apply to pipeline handlers and IPA modules, that is, they can only + * manage the cookies of FrameBuffer objects that they have created, with the exception + * that any FrameBuffer returned by \ref PipelineHandler::exportFrameBuffers() is considered + * to have been created by the application, and thus its cookie shall not be modified. + * \endinternal + * + * \sa cookie() */ void FrameBuffer::setCookie(uint64_t cookie) {
First of all, move most of the documentation next to the `setCookie()` function to have it in a single place. Secondly, make it clear that applications using a `FrameBufferAllocator` are considered to be owners of the allocated FrameBuffer objects, and they may freely manage the cookies of those frame buffers. Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/331 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/framebuffer.cpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) -- 2.54.0