[Bug 768160] qtplugins: How to implement qmlglsrc.

[GStreamer (GNOME Bugzilla)]

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

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

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

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

Still some more work necessary

::: ext/qt/gstqtsrc.cc
@@ +91,3 @@
+
+  g_object_class_install_property (gobject_class, PROP_WIDGET,
+      g_param_spec_pointer ("widget", "QQuickWindow",

would window be better instead of widget?

@@ +134,3 @@
+    case PROP_WIDGET:
+      qwindow = static_cast<QQuickWindow *> (g_value_get_pointer (value));
+      qt_src->widget = new QtGLWindow (NULL, qwindow);

What if we set widget more than once?  This will leak.

@@ +153,3 @@
+  switch (prop_id) {
+    case PROP_WIDGET:
+      g_value_set_pointer (value, qt_src->widget);

This doesn't retrieve what was set in set_property()

@@ +230,3 @@
+      GstStructure *s = gst_caps_get_structure (temp, i);
+      gst_structure_set (s, "width", G_TYPE_INT, width, NULL);
+      gst_structure_set (s, "height", G_TYPE_INT, height, NULL);

framerate? should be 0/1 if unknown.  pixel-aspect-ratio should also be set to
1/1

::: ext/qt/qtwindow.cc
@@ +33,3 @@
+#include <QOpenGLFramebufferObject>
+
+#if GST_GL_HAVE_WINDOW_X11 && GST_GL_HAVE_PLATFORM_EGL && defined
(HAVE_QT_X11)

EGL here, should be GLX

@@ +56,3 @@
+#define LINUX
+#define EGL_API_FB
+#include <gst/gl/fb/gstgldisplay_fb.h>

This is still here

@@ +79,3 @@
+  GstGLDisplay *display;
+  GstGLContext *other_context;
+  guint64 frame_showed;

frames_shown or more correctly, frames_grabbed?

@@ +102,3 @@
+  this->priv->quit = FALSE;
+  this->priv->useDefaultFbo = FALSE;
+  this->priv->frame_showed = 0;

0-initializing all these private variables isn't necessary, g_new0 already does
that.

@@ +167,3 @@
+
+  g_mutex_lock (&this->priv->lock);
+  gst_gl_context_activate (this->priv->other_context, TRUE);

Are these really necessary?

You don't actually use anything that would need/have access to
this->priv->other_context

@@ +182,3 @@
+  }
+
+  gst_gl_context_activate (this->priv->other_context, FALSE);

see above.

@@ +271,3 @@
+
+void
+QtGLWindow::onSceneGraphInitialized()

This winsys initialization is out of date with qmlglsink and may fail on
windows.

Please make a utility function from qmlglsink's version and use that here as
well.

@@ +335,3 @@
+  }
+#endif
+#if GST_GL_HAVE_WINDOW_FB

Still here.

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