After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 792378 - qmlglsink: make work with eglfs_kms
qmlglsink: make work with eglfs_kms
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.12.x
Other Linux
: Normal enhancement
: 1.14.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 782923
Blocks:
 
 
Reported: 2018-01-09 18:24 UTC by Vavooon
Modified: 2018-03-28 11:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Log file (16.02 KB, text/plain)
2018-01-09 18:24 UTC, Vavooon
  Details
qmlglsink gbm patch (1.81 KB, patch)
2018-02-23 19:30 UTC, Carlos Rafael Giani
none Details | Review
qmlglsink gbm patch v2 (1.84 KB, patch)
2018-02-23 19:53 UTC, Carlos Rafael Giani
none Details | Review
qmlglsink qpa egldisplay patch (2.18 KB, patch)
2018-03-04 14:38 UTC, Carlos Rafael Giani
committed Details | Review

Description Vavooon 2018-01-09 18:24:45 UTC
Created attachment 366567 [details]
Log file

I'm trying to run use qmlglsink.
I'm running test qml app (gst-plugins-bad/tests/examples/qt/qmlsink/) on x86_64 linux PC using eglfs_kms mode.
While everything works well on my desktop using X11, it always fails with 
"Failed to create a OpenGL context: EGL_BAD_CONTEXT" error without X.

I believe it is the same bug described here: http://gstreamer-devel.966125.n4.nabble.com/Running-gmlglsink-without-x11-td4682127.html
Comment 1 Matthew Waters (ystreet00) 2018-01-11 23:47:03 UTC
Unless you're running with the patches from bug 782923, then I believe kms+qmlglsink will not work.
Comment 2 Vavooon 2018-01-12 11:09:13 UTC
Thank you for the info. Is it only required patch to apply?
So I need to close the bug and follow mentioned one, right?
Comment 3 Vavooon 2018-02-08 16:47:45 UTC
Today I've tried to apply the patch to latest master (it was applied without errors) and there is an error "qtglwidget qtitem.cc:350:initWinSys: Failed to initialize egl: EGL_NOT_INITIALIZED".
Comment 4 Carlos Rafael Giani 2018-02-23 19:30:55 UTC
Created attachment 368849 [details] [review]
qmlglsink gbm patch

The patch from bug 782923 is a requirement, but it is not sufficent. qmlglsink itself also needs to be patched. Here is a patch.
Comment 5 Carlos Rafael Giani 2018-02-23 19:37:08 UTC
In the comments for bug 783521, there was this response by Matthew:


::: ext/qt/gstqtglutility.cc
@@ +122,3 @@
+    EGLDisplay egl_display = (EGLDisplay)
+        native->nativeResourceForWindow("egldisplay", NULL);
+    display = (GstGLDisplay *) gst_gl_display_egl_new_with_egl_display (egl_display);

What if egl_display is invalid?

Also, this code will always be run when libgstgl is GST_GL_HAVE_WINDOW_GBM enabled and realistically, could probably replace the next #else cause so you don't need to #elif GST_GL_HAVE_WINDOW_GBM.


I added a check to see if egl_display is EGL_NO_DISPLAY, but beyond that, it is actually unclear to me what to do if there's no valid egl_display. The code does not indicate error handling for that. I also do not understand the comment about #else - removing that part seems erroneous, since it looks like a "generic EGL fallback"...?
Comment 6 Carlos Rafael Giani 2018-02-23 19:53:29 UTC
Created attachment 368851 [details] [review]
qmlglsink gbm patch v2

Accidentally uploaded an older version of the patch, which had an error. Here is the new one.
Comment 7 Vavooon 2018-02-24 13:44:10 UTC
With latest patches from related issue qmlglsink shows video. I'd be glad to help you in any further testing.
Comment 8 Sebastian Dröge (slomo) 2018-02-26 12:31:23 UTC
Review of attachment 368851 [details] [review]:

::: ext/qt/gstqtglutility.cc
@@ +43,3 @@
+#include <gst/gl/egl/gstegl.h>
+#include <gst/gl/egl/gstgldisplay_egl.h>
+#include <qpa/qplatformnativeinterface.h>

This probably needs a new configure check, meson check?

@@ +124,3 @@
+        QGuiApplication::platformNativeInterface();
+    EGLDisplay egl_display = (EGLDisplay)
+        native->nativeResourceForWindow("egldisplay", NULL);

I'm wondering if this code shouldn't also be used for the #else (generic EGL) case below instead of randomly selecting 0 as EGL display.
Comment 9 Matthew Waters (ystreet00) 2018-02-27 02:38:50 UTC
Review of attachment 368851 [details] [review]:

::: ext/qt/gstqtglutility.cc
@@ +43,3 @@
+#include <gst/gl/egl/gstegl.h>
+#include <gst/gl/egl/gstgldisplay_egl.h>
+#include <qpa/qplatformnativeinterface.h>

There's no meson definition for qml plugins yet.  configure check for qpa/qplatformnativeinterface.h already exists however need to be explicitly checked in the GBM case in order to enable GBM in qmlglsink.  Same for the viv-fb case actually.

@@ +120,3 @@
+     * display. (It must be a non-NULL struct gbm_device * pointer.) So we have
+     * no choice but to use the native interface to get the EGLDisplay.
+     */

Remove "We actually use the code mentioned above in the comments," It's confusing :)

Just state what we need to do for GBM.

@@ +124,3 @@
+        QGuiApplication::platformNativeInterface();
+    EGLDisplay egl_display = (EGLDisplay)
+        native->nativeResourceForWindow("egldisplay", NULL);

0 is the 'any display' value defined in the egl headers so is not 'random'

As for whether this should be used for generic EGL, probably. The main other use case for that is android where there is only ever one EGLDisplay.
Comment 10 Carlos Rafael Giani 2018-03-04 14:38:10 UTC
Created attachment 369271 [details] [review]
qmlglsink qpa egldisplay patch

Looking at the GBM patch, it is apparent that there is nothing in there that is strictly GBM specific. Instead, it is QPA specific. So I changed the patch to add a preprocessor define to indicate the presence of qplatformnativeinterface.h . If present, QPlatformNativeInterface is used for getting the EGLDisplay. The existing GST_GL_DISPLAY_TYPE_ANY path is retained as a fallback if the QPA header isn't present.
Comment 11 Sebastian Dröge (slomo) 2018-03-22 07:59:26 UTC
commit 56a1eb65e2f82c36d5f63e1182ebf78680a0adba (HEAD -> master)
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Sun Mar 4 15:14:08 2018 +0100

    qt: Get EGL native display from QPA if platform header is available
    
    https://bugzilla.gnome.org/show_bug.cgi?id=792378