[Bug 783521] gl: Add "direct" dmabuf uploader

[GStreamer (GNOME Bugzilla)]

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

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

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

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

Apart from the type checks, this looks ok to me.

::: gst-libs/gst/gl/egl/gsteglimage.c
@@ +592,3 @@
+{
+  EGLDisplay egl_display = EGL_DEFAULT_DISPLAY;
+  EGLuint64KHR *modifiers;

This will need configure/meson checks as old android versions don't have 64-bit
integers in EGL.

@@ +647,3 @@
+ *
+ * Another notable difference to gst_egl_image_from_dmabuf()
+ * is that this function creates one EGL image for all planes, not just one.

I assume this is meant to read as "one EGLImage for all planes, not one for a
single plane."

@@ +653,3 @@
+GstEGLImage *
+gst_egl_image_from_dmabuf_direct (GstGLContext * context,
+    gint fd[GST_VIDEO_MAX_PLANES], gsize offset[GST_VIDEO_MAX_PLANES],

Some documentation should be added to say that these 'arrays' have as many
valid values as there are planes in @in_info.

I'm also not a fan of arrays as function arguments.  'type *name' is exactly
the same and the size (in []) is not useful to the compiler.

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