[Bug 693826] dmabuf-based GstMemory and GstAllocator incl. v4l2src support

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=693826
  GStreamer | gst-plugins-good | git

Sebastian Dröge <slomo> changed:

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

--- Comment #15 from Sebastian Dröge <slomo at circular-chaos.org> 2013-02-18 11:35:34 UTC ---
Review of attachment 236577:
 --> (https://bugzilla.gnome.org/review?bug=693826&attachment=236577)

Just mostly nitpicking now, looks good :)

::: configure.ac
@@ +845,3 @@
 gst-libs/Makefile
 gst-libs/gst/Makefile
+gst-libs/gst/allocators/Makefile

There should be a configure check for checking if dmabuf is supported or not...
which then enables compilation of the dmabuf code or not. The library should be
built all the time though, and there should be a gstallocatorsconfig.h or
something like that that specifies which allocators are available.

::: gst-libs/gst/allocators/Makefile.am
@@ +16,3 @@
+
+if HAVE_INTROSPECTION
+BUILT_GIRSOURCES = Gstallocators- at GST_API_VERSION@.gir

Uppercase 'a', GstAllocators-  (also everywhere else of course)

::: gst-libs/gst/allocators/gstdmabuf.c
@@ +154,3 @@
+{
+  gint newfd = dup (mem->fd);
+  g_return_val_if_fail (newfd != -1, NULL);

I don't think this should be an assertion... or is it guaranteed that all
dmabufs are always dup'able?

@@ +206,3 @@
+  GstAllocator *allocator =
+      g_object_new (dmabuf_mem_allocator_get_type (), NULL);
+  gst_allocator_register (ALLOCATOR_NAME, gst_object_ref (allocator));

Why the additional ref here?

@@ +245,3 @@
+  if (!allocator) {
+    GST_WARNING ("missing allocator");
+    return NULL;

You could let it use gst_dmabuf_allocator_obtain() if the allocator is NULL

::: pkgconfig/gstreamer-allocators-uninstalled.pc.in
@@ +12,3 @@
+Version: @VERSION@
+Requires: gstreamer- at GST_API_VERSION@
+Libs:
@abs_top_builddir@/gst-libs/gst/allocators/libgstallocators- at GST_API_VERSION@.la
@LIBM@

You don't need libm

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.