Created attachment 372871 [details] [review]
fdallocator: add gst_fd_allocator_alloc_wrapped

Created attachment 372872 [details] [review]
dmabufallocator: add gst_dmabuf_allocator_alloc_wrapped

Review of attachment 372871 [details] [review]:

::: gst-libs/gst/allocators/gstfdmemory.c
@@ +284,3 @@
+
+/**
+ * gst_fd_allocator_alloc_wrapped:

Why _wrapped ? in other places it would have been called _full ? Just asking.

Any code using that ?

(In reply to Nicolas Dufresne (ndufresne) from comment #3)
> Review of attachment 372871 [details] [review] [review]:
> 
> ::: gst-libs/gst/allocators/gstfdmemory.c
> @@ +284,3 @@
> +
> +/**
> + * gst_fd_allocator_alloc_wrapped:
> 
> Why _wrapped ? in other places it would have been called _full ? Just asking.

Something between the existing gst_buffer_new_wrapped() and gst_buffer_new_wrapped_full(), maybe should change to
gst_fd_allocator_alloc_wrapped_full()?

(In reply to Nicolas Dufresne (ndufresne) from comment #4)
> Any code using that ?

Some custom code on an embeeded device (imx6 based), keeping a appsrc-->v4l2sink running (to avoid HDMI-Out blackouts), feeding alternating by v4l2src-->appsink HDMI-In capture pipeline and custom allocated dma buffers in case the HDMI-In source is changing...

Created attachment 373409 [details] [review]
dmabufallocator: add gst_dmabuf_allocator_alloc_wrapped

Changes v1 -> v2: add additional GstFdMemoryFlags parameter

Created attachment 373410 [details] [review]
dmabufallocator: add gst_dmabuf_allocator_alloc_wrapped

Changes v1 -> v2: add additional GstFdMemoryFlags parameter (and upload the right patch)

Why not just call the base class function ?

Because of the same reason all other functions above doing the same pattern,
checking GST_IS_DMABUF_ALLOCATOR (allocator) and calling the gst_fd version (or
do I miss something)?

They existed before the base class. Do you have any better reason why we need more API?

For consistency? Use dmabuf_allocator functions to gain dma buffers (see e.g.
this commit from 4 month ago [1])?

[1] https://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/gst-libs/gst/allocators/gstdmabuf.c?id=56ab7e0e1d0ebcbfef48c6a1ce89463a415c8e9

Looks like a mistake of mine to merge this one. It's exactly the same function, it's totally safe to use. Though for this one, it's used in one of the core repository, so it was easier to say yes.

Anyone else ? Can someone bring pro/cons ? Do we only merge the FDAlloctor patch or both ?

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/467.