| Message ID | 20200109232647.2068851-1-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Delegated to: | Niklas Söderlund |
| Headers | show |
| Series |
|
| Related | show |
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@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@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
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 + * and are only guaranteed to be valid during the mapBuffers() call. + * * 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
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@ragnatech.se> --- src/libcamera/ipa_interface.cpp | 3 +++ 1 file changed, 3 insertions(+)