[Bug 768794] ion: DMA Buf allocator based on ion

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=768794

Matthew Waters (ystreet00) <ystreet00 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #333309|none                        |needs-work
             status|                            |

--- Comment #8 from Matthew Waters (ystreet00) <ystreet00 at gmail.com> ---
Review of attachment 333309:
 --> (https://bugzilla.gnome.org/review?bug=768794&attachment=333309)

I think this should be inheriting from GstFDMemory instead of allocating
GstDmabufMemory.

You can then create a separate allocator that subclasses the Dmabuf allocator
for the Dmabuf compatibility in gstreamer (although this code doesn't provide
that so that may not be necessary).

There's also no pkg-config file for this.

::: gst-libs/gst/ion/Makefile.am
@@ +1,1 @@
+lib_LTLIBRARIES = libgstbadion- at GST_API_VERSION@.la

Why is bad in the name?  It's not conflicting with any other library.

@@ +5,3 @@
+
+libgstbadion_ at GST_API_VERSION@_la_CFLAGS = $(GST_CFLAGS) \
+    -DGST_USE_UNSTABLE_API

What unstable API ar you using that requires this flag?

::: gst-libs/gst/ion/gstionmemory.c
@@ +36,3 @@
+#define gst_ion_allocator_parent_class parent_class
+
+#define PAGE_ALIGN(x) (((x) + 4095) & ~4095)

This should probably use getpagesize () or similar

http://man7.org/linux/man-pages/man2/getpagesize.2.html

@@ +120,3 @@
+  allocation_data.len = ion_size;
+  allocation_data.align = params->align;
+  allocation_data.heap_id_mask = ION_HEAP_TYPE_DMA_MASK;

This should be overridable for other heaps.

@@ +121,3 @@
+  allocation_data.align = params->align;
+  allocation_data.heap_id_mask = ION_HEAP_TYPE_DMA_MASK;
+  allocation_data.flags = 0;

flags possibly also needs to be overridable

@@ +137,3 @@
+
+  ion_mem = g_slice_new0 (GstIONMemory);
+  gst_memory_init (GST_MEMORY_CAST (ion_mem), GST_MEMORY_FLAG_NO_SHARE,

Why do you have NO_SHARE here? any specific reason?

@@ +147,3 @@
+
+  gst_mini_object_set_qdata (GST_MINI_OBJECT (mem), GST_ION_MEMORY_QUARK,
+      ion_mem, (GDestroyNotify) gst_memory_unref);

This won't propagate the ion memory across copies/shares.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.