[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Peter Seiderer]

Add MESA_EGL_NO_X11_HEADERS define (as other egl projects do via
QT_EGL_NO_X11).

Fixes [1]:

  In file included from .../host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include/EGL/egl.h:39:0,
                   from qsgvideonode_egl.h:48,
                   from qsgvideonode_egl.cpp:40:
  .../host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include/EGL/eglplatform.h:119:22: fatal error: X11/Xlib.h: No such file or directory
   #include <X11/Xlib.h>
                      ^
  compilation terminated.
  make[5]: Leaving directory `.../build/qt5multimedia-5.8.0/src/plugins/videonode/egl'

[1] http://autobuild.buildroot.net/results/364/364a76bbf90251f9cae2abd3ef2e382758eae538

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 ...eonode-egl-fix-x11-header-related-compile.patch | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 package/qt5/qt5multimedia/5.8.0/0001-plugins-videonode-egl-fix-x11-header-related-compile.patch

diff --git a/package/qt5/qt5multimedia/5.8.0/0001-plugins-videonode-egl-fix-x11-header-related-compile.patch b/package/qt5/qt5multimedia/5.8.0/0001-plugins-videonode-egl-fix-x11-header-related-compile.patch
new file mode 100644
index 0000000000..bbf2a94f6d
--- /dev/null
+++ b/package/qt5/qt5multimedia/5.8.0/0001-plugins-videonode-egl-fix-x11-header-related-compile.patch
@@ -0,0 +1,30 @@
+From 72e04823b49bb8ec5f126a172521c16f3281282d Mon Sep 17 00:00:00 2001
+From: Peter Seiderer <ps.report@gmx.net>
+Date: Mon, 26 Jun 2017 23:01:32 +0200
+Subject: [PATCH] plugins/videonode/egl: fix x11 header related compile failure
+
+Add MESA_EGL_NO_X11_HEADERS define (as other egl projects do via
+QT_EGL_NO_X11).
+
+Signed-off-by: Peter Seiderer <ps.report@gmx.net>
+---
+ src/plugins/videonode/egl/egl.pro | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/src/plugins/videonode/egl/egl.pro b/src/plugins/videonode/egl/egl.pro
+index 7151123..08ec149 100644
+--- a/src/plugins/videonode/egl/egl.pro
++++ b/src/plugins/videonode/egl/egl.pro
+@@ -3,6 +3,9 @@ TARGET = eglvideonode
+ QT += multimedia-private qtmultimediaquicktools-private
+ CONFIG += egl
+ 
++# Avoid X11 header collision, use generic EGL native types
++DEFINES += MESA_EGL_NO_X11_HEADERS
++
+ HEADERS += \
+     qsgvideonode_egl.h
+ 
+-- 
+2.11.0
+
-- 
2.11.0

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Thomas Petazzoni]

Hello,

On Mon, 26 Jun 2017 23:13:11 +0200, Peter Seiderer wrote:

> +diff --git a/src/plugins/videonode/egl/egl.pro b/src/plugins/videonode/egl/egl.pro
> +index 7151123..08ec149 100644
> +--- a/src/plugins/videonode/egl/egl.pro
> ++++ b/src/plugins/videonode/egl/egl.pro
> +@@ -3,6 +3,9 @@ TARGET = eglvideonode
> + QT += multimedia-private qtmultimediaquicktools-private
> + CONFIG += egl
> + 
> ++# Avoid X11 header collision, use generic EGL native types
> ++DEFINES += MESA_EGL_NO_X11_HEADERS
> ++

I am not really convinced by this patch. Indeed, I believe it should be
the OpenGL providers that define this constant when needed. If you look
at the config that generated the build failure you're trying to fix
(http://autobuild.buildroot.net/results/364/364a76bbf90251f9cae2abd3ef2e382758eae538),
you'll see that the OpenGL provider used is mali-t76x. However,
package/mali-t76x/glesv2.pc contains:

Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS

So, I would expect that Qt5 should not need to redefine it again.
Obviously, what worries me is that fixing Qt5 will fix just Qt5, and
not any other package using the same OpenGL provider.

What do you think?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Peter Seiderer]

Hello Thomas,

On Sun, 2 Jul 2017 00:33:32 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Mon, 26 Jun 2017 23:13:11 +0200, Peter Seiderer wrote:
> 
> > +diff --git a/src/plugins/videonode/egl/egl.pro b/src/plugins/videonode/egl/egl.pro
> > +index 7151123..08ec149 100644
> > +--- a/src/plugins/videonode/egl/egl.pro
> > ++++ b/src/plugins/videonode/egl/egl.pro
> > +@@ -3,6 +3,9 @@ TARGET = eglvideonode
> > + QT += multimedia-private qtmultimediaquicktools-private
> > + CONFIG += egl
> > + 
> > ++# Avoid X11 header collision, use generic EGL native types
> > ++DEFINES += MESA_EGL_NO_X11_HEADERS
> > ++
> 
> I am not really convinced by this patch. Indeed, I believe it should be
> the OpenGL providers that define this constant when needed. If you look
> at the config that generated the build failure you're trying to fix
> (http://autobuild.buildroot.net/results/364/364a76bbf90251f9cae2abd3ef2e382758eae538),
> you'll see that the OpenGL provider used is mali-t76x. However,
> package/mali-t76x/glesv2.pc contains:
> 
> Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
> 
> So, I would expect that Qt5 should not need to redefine it again.
> Obviously, what worries me is that fixing Qt5 will fix just Qt5, and
> not any other package using the same OpenGL provider.
> 
> What do you think?

Same fixing pattern as for qt5base (and for qtbase accepted upstream, see e.g. [1]),
but in general your are right, honor the given provider provided flags would be
the better solution...maybe something as suggested on the yocto meta-freescale
mailing list will work, see [2], [3]: 'egl.prf: Fix build error when egl headers
need platform definition'?

Did not yet had time to try the suggested patch (the first references says
'Upstream-Status: Pending' but without reference)...

Regards,
Peter

[1] https://codereview.qt-project.org/#/c/188158/
[2] https://lists.yoctoproject.org/pipermail/meta-freescale/2017-June/020654.html
[3] https://lists.yoctoproject.org/pipermail/meta-freescale/2017-June/020653.html

> 
> Thomas

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Thomas Petazzoni]

Hello,

On Sun, 2 Jul 2017 21:01:27 +0200, Peter Seiderer wrote:

> Same fixing pattern as for qt5base (and for qtbase accepted upstream, see e.g. [1]),

Well, [1] is about adding QT_EGL_NO_X11, which is a QT definition. It
is very different from MESA_EGL_NO_X11_HEADERS, which is an
OpenGL-level definition.

> but in general your are right, honor the given provider provided flags would be
> the better solution...maybe something as suggested on the yocto meta-freescale
> mailing list will work, see [2], [3]: 'egl.prf: Fix build error when egl headers
> need platform definition'?

I'm not sure I fully understand those patches, but apparently they use
more pkg-config flags from OpenGL, which is exactly the approach I
suggest to investigate indeed.

BTW, the same thing should be done for Qt instead of
package/qt/0012-OpenGL-EGL-Avoid-X11-header-collision.patch.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Peter Seiderer]

Hello Thomas,

On Sun, 2 Jul 2017 23:14:49 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Sun, 2 Jul 2017 21:01:27 +0200, Peter Seiderer wrote:
> 
> > Same fixing pattern as for qt5base (and for qtbase accepted upstream, see e.g. [1]),
> 
> Well, [1] is about adding QT_EGL_NO_X11, which is a QT definition. It
> is very different from MESA_EGL_NO_X11_HEADERS, which is an
> OpenGL-level definition.

The missing link is in file src/platformsupport/eglconvenience/qt_egl_p.h line 54ff:

 54 #ifdef QT_EGL_NO_X11
 55 # define MESA_EGL_NO_X11_HEADERS // MESA
 56 # define WIN_INTERFACE_CUSTOM    // NV
 57 #endif // QT_EGL_NO_X11
 58 
 59 #ifdef QT_EGL_WAYLAND
 60 # define WAYLAND // NV
 61 #endif // QT_EGL_WAYLAND
 62 
 63 #include <EGL/egl.h>
 64 #include <EGL/eglext.h>

QT_EGL_NO_X11 forces MESA_EGL_NO_X11_HEADERS before the egl headers are included...

> 
> > but in general your are right, honor the given provider provided flags would be
> > the better solution...maybe something as suggested on the yocto meta-freescale
> > mailing list will work, see [2], [3]: 'egl.prf: Fix build error when egl headers
> > need platform definition'?
> 
> I'm not sure I fully understand those patches, but apparently they use
> more pkg-config flags from OpenGL, which is exactly the approach I
> suggest to investigate indeed.
> 
> BTW, the same thing should be done for Qt instead of
> package/qt/0012-OpenGL-EGL-Avoid-X11-header-collision.patch.

Will test the patch(es) the next days...

Regards,
Peter

> 
> Best regards,
> 
> Thomas

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Thomas Petazzoni]

Hello,

On Mon, 3 Jul 2017 20:46:54 +0200, Peter Seiderer wrote:

> On Sun, 2 Jul 2017 23:14:49 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> 
> > Hello,
> > 
> > On Sun, 2 Jul 2017 21:01:27 +0200, Peter Seiderer wrote:
> >   
> > > Same fixing pattern as for qt5base (and for qtbase accepted upstream, see e.g. [1]),  
> > 
> > Well, [1] is about adding QT_EGL_NO_X11, which is a QT definition. It
> > is very different from MESA_EGL_NO_X11_HEADERS, which is an
> > OpenGL-level definition.  
> 
> The missing link is in file src/platformsupport/eglconvenience/qt_egl_p.h line 54ff:
> 
>  54 #ifdef QT_EGL_NO_X11
>  55 # define MESA_EGL_NO_X11_HEADERS // MESA
>  56 # define WIN_INTERFACE_CUSTOM    // NV
>  57 #endif // QT_EGL_NO_X11
>  58 
>  59 #ifdef QT_EGL_WAYLAND
>  60 # define WAYLAND // NV
>  61 #endif // QT_EGL_WAYLAND
>  62 
>  63 #include <EGL/egl.h>
>  64 #include <EGL/eglext.h>
> 
> QT_EGL_NO_X11 forces MESA_EGL_NO_X11_HEADERS before the egl headers are included...

Ah, okay, indeed. But still, I really think it should be the
responsibility of the OpenGL provider to define MESA_EGL_NO_X11_HEADERS
in its .pc files, rather than having this logic duplicated in all
packages "consuming" the OpenGL API.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Peter Seiderer]

Hello Thomas,

On Sun, 2 Jul 2017 23:14:49 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Sun, 2 Jul 2017 21:01:27 +0200, Peter Seiderer wrote:
> 
> > Same fixing pattern as for qt5base (and for qtbase accepted upstream, see e.g. [1]),
> 
> Well, [1] is about adding QT_EGL_NO_X11, which is a QT definition. It
> is very different from MESA_EGL_NO_X11_HEADERS, which is an
> OpenGL-level definition.
> 
> > but in general your are right, honor the given provider provided flags would be
> > the better solution...maybe something as suggested on the yocto meta-freescale
> > mailing list will work, see [2], [3]: 'egl.prf: Fix build error when egl headers
> > need platform definition'?

And two links to the upstream suggestion/discussion:

- QTBUG-61712: qtbase: A build error was occurring due to missing EGL configuration
  https://bugreports.qt.io/browse/QTBUG-61712

- Patch: qtbase: Fix build error when using EGL
  https://codereview.qt-project.org/#/c/198906/

Regards,
Peter

> 
> I'm not sure I fully understand those patches, but apparently they use
> more pkg-config flags from OpenGL, which is exactly the approach I
> suggest to investigate indeed.
> 
> BTW, the same thing should be done for Qt instead of
> package/qt/0012-OpenGL-EGL-Avoid-X11-header-collision.patch.
> 
> Best regards,
> 
> Thomas

[Buildroot] [PATCH v1] qt5multimedia: fix plugins videonode egl x11 header related compile failure

[Thomas Petazzoni]

Hello,

On Wed, 5 Jul 2017 21:53:19 +0200, Peter Seiderer wrote:

> > > but in general your are right, honor the given provider provided flags would be
> > > the better solution...maybe something as suggested on the yocto meta-freescale
> > > mailing list will work, see [2], [3]: 'egl.prf: Fix build error when egl headers
> > > need platform definition'?  
> 
> And two links to the upstream suggestion/discussion:
> 
> - QTBUG-61712: qtbase: A build error was occurring due to missing EGL configuration
>   https://bugreports.qt.io/browse/QTBUG-61712
> 
> - Patch: qtbase: Fix build error when using EGL
>   https://codereview.qt-project.org/#/c/198906/

I'm not used to the Qt contribution workflow, but the patch visible
through those links looks good (use CFLAGS provided by the EGL .pc
file), and the reception seems positive (+6 votes on the code review
tool).

So it all seems to be going in the right direction, no?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com