[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 #332082|none                        |needs-work
             status|                            |

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

(In reply to Haihua Hu from comment #6)
> My design of qmlglsrc has no performance concern. 
> 
> 1. In my code, qmlglsrc will warp qt OpenGL context and I make glcontext
> shared with qt warp context so we can copy texture from scene graph to
> glmemory which is created in glcontext. This will be very fast because it is
> operated by GPU. 
> 
> 2.When scene graph updated, if downstream has not consumed the old buffer
> and no new buffer set to qt, just skip this  as we cannot block scene graph.

This effectively makes you a live source which you'll have to advertise in the
latency query.

The moving of the example should be in a separate patch.

::: ext/qt/gstqtsrc.cc
@@ +95,3 @@
+  g_object_class_install_property (gobject_class, PROP_WIDGET,
+      g_param_spec_pointer ("widget", "QQuickWindow",
+          "The QQuickWindow to place in the object heirachy",

Isn't this the other way, i.e. the application needs to set the QQuickWindow to
use?

@@ +219,3 @@
+  }
+
+  caps = GST_BASE_SRC_CLASS (parent_class)->get_caps (bsrc, filter);

This strikes me as odd.  Can you explain why you need to call the base class
before setting the width/height/...?

@@ +228,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);

par? framerate?

@@ +239,3 @@
+
+static GstCaps *
+gst_qt_src_src_fixate (GstBaseSrc * bsrc, GstCaps * caps)

empty implementation isn't necessary.

@@ +513,3 @@
+        g_print ("qmlglsrc Total refresh frames (%lld), playing for
(%"GST_TIME_FORMAT"), fps (%.3f).\n",
+                frame_showed, GST_TIME_ARGS (qt_src->run_time),
+                (gfloat)GST_SECOND * frame_showed / qt_src->run_time);

no prints.

This should be gst debugging if necessary.

::: ext/qt/qtitem.cc
@@ +35,3 @@
 #include <qpa/qplatformnativeinterface.h>

+#if GST_GL_HAVE_WINDOW_X11 && GST_GL_HAVE_PLATFORM_EGL && defined
(HAVE_QT_X11)

This isn't right and will cause errors on GLX.

If necessary, this should be a separate patch.

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

and GLX ?

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

non-existent header/defines.

@@ +219,3 @@
+
+  if (!gst_video_frame_map (&gl_frame, info, this->priv->buffer,
+          (GstMapFlags) (GST_MAP_READ | GST_MAP_GL))) {

Shouldn't this be WRITE | GL?

@@ +236,3 @@
+      this->source->renderTargetId(), dst_tex, width,height);
+
+  gl->GenFramebuffers (1, &fbo);

You can keep the fbo around here instead of creating/destroying it every frame.

@@ +245,3 @@
+
+  gl->ReadBuffer (GL_COLOR_ATTACHMENT0);
+  gl->BlitFramebuffer ( 0, 0, width, height,

glBlitFramebuffer isn't available natively in GLES2 so you'll need an
alternative, like glCopyTexImage2D

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

This looks like duplicated code with the sink.  Maybe make a utility function
to make this easier?

::: tests/examples/qt/qmlsrc/main.cpp
@@ +62,3 @@
+    sigemptyset(&act.sa_mask);
+    act.sa_flags = 0;
+    sigaction(sig, &act, NULL);

Unix specific code which won't work on Win32 at least.

Is it even necessary?

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