Bug 163000 – [GTK] Build fix for X11 and GStreamerGL after r183731

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gwang Yoon Hwang]

[GTK] Build fix for X11 and GStreamerGL after r183731

[Gwang Yoon Hwang]

Created attachment 290788 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 290788 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290788&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:267
> -#endif
> -
> -#if PLATFORM(WAYLAND)
> +#elif PLATFORM(WAYLAND)

What's exactly the build failure? I don't think this change is correct, X11 and Wayland are not mutually exclusive, so the ifdefs are correct here. Why is the display created twice? if both X11 and wayland are enabled, shared display will be either X11 or wayland.

[Gwang Yoon Hwang]

(In reply to comment #2)
> Comment on attachment 290788 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290788&action=review
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:267
> > -#endif
> > -
> > -#if PLATFORM(WAYLAND)
> > +#elif PLATFORM(WAYLAND)
> 
> What's exactly the build failure? I don't think this change is correct, X11
> and Wayland are not mutually exclusive, so the ifdefs are correct here. Why
> is the display created twice? if both X11 and wayland are enabled, shared
> display will be either X11 or wayland.

As you mentioned, PlatformDisplay is only created just for once.
And I misunderstood that gst_gl_display_x11_new_with_display and gst_gl_display_egl_new_with_egl_display would be executed at same time.

The build problem is, MediaPlayerPrivateGStreamerBase does not include gstgldisplay_egl.h if we are using GLX. So gst_gl_display_egl_new_with_egl_display is not visible from GSteamerBase.

I'll take a look if there is no problem to include both of gstgldisplay_x11 and gstgldisplay_egl

[Gwang Yoon Hwang]

(In reply to comment #3)
> (In reply to comment #2)
> As you mentioned, PlatformDisplay is only created just for once.
> And I misunderstood that gst_gl_display_x11_new_with_display and
> gst_gl_display_egl_new_with_egl_display would be executed at same time.
> 
> The build problem is, MediaPlayerPrivateGStreamerBase does not include
> gstgldisplay_egl.h if we are using GLX. So
> gst_gl_display_egl_new_with_egl_display is not visible from GSteamerBase.
> 
> I'll take a look if there is no problem to include both of gstgldisplay_x11
> and gstgldisplay_egl

There could be a build problem if the platform has only X11 or EGL.
Since gstgldisplay_x11.h includes headers of X11 and gstgldisplay_egl.h includes headers of EGL, we cannot include both of them at same time if the target does not have both.

[Philippe Normand]

Comment on attachment 290788 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290788&action=review

>>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:267
>>> +#elif PLATFORM(WAYLAND)
>> 
>> What's exactly the build failure? I don't think this change is correct, X11 and Wayland are not mutually exclusive, so the ifdefs are correct here. Why is the display created twice? if both X11 and wayland are enabled, shared display will be either X11 or wayland.
> 
> As you mentioned, PlatformDisplay is only created just for once.
> And I misunderstood that gst_gl_display_x11_new_with_display and gst_gl_display_egl_new_with_egl_display would be executed at same time.
> 
> The build problem is, MediaPlayerPrivateGStreamerBase does not include gstgldisplay_egl.h if we are using GLX. So gst_gl_display_egl_new_with_egl_display is not visible from GSteamerBase.
> 
> I'll take a look if there is no problem to include both of gstgldisplay_x11 and gstgldisplay_egl

This is wrong because we can have both X11 and WAYLAND enabled. IIUC anyway :)

[Gwang Yoon Hwang]

Created attachment 290807 [details]
Patch

[Gwang Yoon Hwang]

(In reply to comment #6)
> Created attachment 290807 [details]
> Patch

Found much simpler solution. Just include gstdisplay_egl.h if we have EGL.
Ready for review.

[WebKit Commit Bot]

Comment on attachment 290807 [details]
Patch

Clearing flags on attachment: 290807

Committed r206861: <http://trac.webkit.org/changeset/206861>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.