Summary: | [GTK] Build fix for X11 and GStreamerGL after r183731 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gwang Yoon Hwang <yoon> | ||||||
Component: | New Bugs | Assignee: | Gwang Yoon Hwang <yoon> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cgarcia, commit-queue, gustavo, mario, pnormand | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Gwang Yoon Hwang
2016-10-06 00:13:17 PDT
Created attachment 290788 [details]
Patch
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. (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 (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. 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 :) Created attachment 290807 [details]
Patch
(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. Comment on attachment 290807 [details] Patch Clearing flags on attachment: 290807 Committed r206861: <http://trac.webkit.org/changeset/206861> All reviewed patches have been landed. Closing bug. |