[libcamera-devel] [PATCH] libcamera: ipa_interface: Document the ownership of dmabufs passed to mapBuffers()

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jan 10, 2020 at 12:26:47AM +0100, Niklas Söderlund wrote:
> The ownership of the dmabuf file handles passed to mapBuffers() is not
> clear. Explicitly document that they are borrowed from the callee and
> only guaranteed to be valid for the duration of the mapBuffers() call.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/ipa_interface.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> index ee3e3622f39ae85f..25b075fef5a50db7 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -429,6 +429,9 @@ namespace libcamera {
>   * handler that the IPA needs to access. It provides dmabuf file handles for
>   * each buffer, and associates the buffers with unique numerical IDs.
>   *
> + * The dmabuf file handles provided in \a buffers are borrowed from the callee

s/handles/descriptors/

s/callee/caller/ ?

> + * and are only guaranteed to be valid during the mapBuffers() call.

But I think this isn't needed anymore, now that IPABuffer stores
FileDescriptor instances and FileDescriptor has become cheap to copy.

We should however document ownerhip of the dmabuf fds in the C API, as
we have no C++ object to make it explicit there. You can simply move
this paragraph to the documentation of ipa_context_ops::map_buffers,
with s/mapBuffers()/map_buffers()/ and maybe the following sentence
added.

 * Should the callee need to store a copy of the file descriptors, it
 * shall duplicate them first with ::dup().

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + *
>   * IPAs shall map the dmabuf file handles to their address space and keep a
>   * cache of the mappings, indexed by the buffer numerical IDs. The IDs are used
>   * in all other IPA interface methods to refer to buffers, including the

-- 
Regards,

Laurent Pinchart