[Bug 784847] omxvideodec: add dmabuf support for zynqultrascaleplus

[GStreamer (GNOME Bugzilla)]

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

--- Comment #6 from Julien Isorce <julien.isorce at gmail.com> ---
Review of attachment 355431:
 --> (https://bugzilla.gnome.org/review?bug=784847&attachment=355431)

::: configure.ac
@@ +159,1 @@


Yes making the gstdmabuf allocator a soft dep. (not the GstAllocator).
I had the same question actually, I wonder why it is done like this in gstgl,
looks like we could even include gstdmabuf.h on Windows for example without any
error :)
Maybe Nicolas knows.

::: omx/gstomxbufferpool.c
@@ +395,3 @@
+      gint fd;
+
+      fd = GPOINTER_TO_INT (omx_buf->omx_buf->pBuffer);

Oki, maybe just put a note about it, like: no need to use dmabuf caps feature
because the fd is always garantie to be mappable for rw on zynq-uscale. 
Since it is a generic code here maybe add the following to make it clear:

#if not ZYNQ
if (!gst_memory_map(RW) && dmabuf_caps_feature not present in pool caps) {
   bail out;
}
unmap;
#endif

@@ +518,2 @@
     /* If it's our own memory we have to set the sizes */
+    if (!pool->other_pool && !pool->dmabuf) {

I do not know either, maybe the reason why it is not in
gst_omx_memory_allocator_alloc is because it needs to be done after append. If
no reason then yes it should be moved. Does git blame tell more about it ?

::: omx/gstomxvideodec.c
@@ +217,3 @@
+      GST_ERROR_OBJECT (self, "Failed to set output buffer mode: %s (0x%08x)",
+          gst_omx_error_to_string (err), err);
+    buffer_mode.eMode = OMX_ALG_BUF_DMA;

Right, I usually have the exact same problem. Depending if it is dev/test or
prod I prefer or not to have the fallbacks or not, and/or errors. I wonder if
we could have some sort of infos about the pool/allocator being used in the
caps, (caps feature does not tell), but this is another story.

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