[Bug 784847] omxvideodec: add dmabuf support for zynqultrascaleplus

[GStreamer (GNOME Bugzilla)]

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

--- Comment #5 from Guillaume Desmottes <gdesmott at gnome.org> ---
Review of attachment 355431:
 --> (https://bugzilla.gnome.org/review?bug=784847&attachment=355431)

Thanks Julien.

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


You mean making the allocators a soft dep? It's fine with me but we already
have some many #ifdef in gst-omx...
They are part of -base so is there any reason why they wouldn't be available?

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

It is yes (read and write).

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

Actually I'm not sure why we have to set the size and offset there for
GstOMXMemory. Can't we just do it ingst_omx_memory_allocator_alloc() ?

::: omx/gstomxvideodec.c
@@ +199,3 @@
   self->dec_out_port = gst_omx_component_add_port (self->dec, out_port_index);

+#ifdef USE_OMX_TARGET_ZYNQ_USCALE_PLUS

Nope, we can rely on it being there.

@@ +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;

Yeah I wasn't sure about that either. I figured I'd rather having it failing
than passing and then having to wonder why performances are terrible. But I'm
ready to be convinced to use the fallback option.

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