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 797358 - vaapi: build error when GL is enabled and EGL disabled
vaapi: build error when GL is enabled and EGL disabled
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-10-31 04:26 UTC by He Junyan
Modified: 2018-11-03 15:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The patch (1.98 KB, patch)
2018-10-31 05:31 UTC, He Junyan
needs-work Details | Review
V2 patch (3.03 KB, patch)
2018-11-01 06:58 UTC, He Junyan
none Details | Review
plugins: Fix build error when GL is enabled while EGL is disabled. (2.67 KB, patch)
2018-11-01 13:21 UTC, Víctor Manuel Jáquez Leal
none Details | Review

Description He Junyan 2018-10-31 04:26:40 UTC
When we configure gstreamer-vaapi use glx and no egl

gstreamer-vaapi configuration summary:

Installation Prefix .............. : /home/robinhe/media/build
GStreamer API version ............ : 1.15
VA-API version ................... : 1.3.0
Video encoding ................... : yes
Video outputs .................... : x11 glx


We get build error:

gstvaapipluginutil.c: In function ‘gst_vaapi_get_display_type_from_gl_env’:
gstvaapipluginutil.c:174:22: error: unused variable ‘gl_platform_type’ [-Werror=unused-variable]
   const gchar *const gl_platform_type = g_getenv ("GST_GL_PLATFORM");
                      ^~~~~~~~~~~~~~~~
gstvaapipluginutil.c: In function ‘gst_vaapi_create_display_from_egl’:
gstvaapipluginutil.c:252:15: error: implicit declaration of function ‘gst_vaapi_display_egl_new_with_native_display’; did you mean ‘gst_vaapi_display_x11_new_with_va_display’? [-Werror=implicit-function-declaration]
     display = gst_vaapi_display_egl_new_with_native_display (native_display_egl,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               gst_vaapi_display_x11_new_with_va_display
gstvaapipluginutil.c:252:15: error: nested extern declaration of ‘gst_vaapi_display_egl_new_with_native_display’ [-Werror=nested-externs]
gstvaapipluginutil.c:252:13: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
     display = gst_vaapi_display_egl_new_with_native_display (native_display_egl,
             ^
gstvaapipluginutil.c:262:17: error: implicit declaration of function ‘gst_vaapi_display_egl_new’; did you mean ‘gst_vaapi_display_glx_new’? [-Werror=implicit-function-declaration]
       display = gst_vaapi_display_egl_new (wrapped_display, gles_version);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~
                 gst_vaapi_display_glx_new
gstvaapipluginutil.c:262:17: error: nested extern declaration of ‘gst_vaapi_display_egl_new’ [-Werror=nested-externs]
gstvaapipluginutil.c:262:15: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
       display = gst_vaapi_display_egl_new (wrapped_display, gles_version);
               ^
gstvaapipluginutil.c:268:5: error: implicit declaration of function ‘gst_vaapi_display_egl_set_gl_context’; did you mean ‘gst_vaapi_plugin_base_set_gl_context’? [-Werror=implicit-function-declaration]
     gst_vaapi_display_egl_set_gl_context (GST_VAAPI_DISPLAY_EGL (display),
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     gst_vaapi_plugin_base_set_gl_context
gstvaapipluginutil.c:268:5: error: nested extern declaration of ‘gst_vaapi_display_egl_set_gl_context’ [-Werror=nested-externs]
gstvaapipluginutil.c:268:43: error: implicit declaration of function ‘GST_VAAPI_DISPLAY_EGL’; did you mean ‘GST_VAAPI_DISPLAY_GLX’? [-Werror=implicit-function-declaration]
     gst_vaapi_display_egl_set_gl_context (GST_VAAPI_DISPLAY_EGL (display),
                                           ^~~~~~~~~~~~~~~~~~~~~
                                           GST_VAAPI_DISPLAY_GLX
gstvaapipluginutil.c:268:43: error: nested extern declaration of ‘GST_VAAPI_DISPLAY_EGL’ [-Werror=nested-externs]
cc1: all warnings being treated as errors
Comment 1 He Junyan 2018-10-31 05:31:15 UTC
Created attachment 374113 [details] [review]
The patch
Comment 2 Víctor Manuel Jáquez Leal 2018-10-31 18:36:49 UTC
Review of attachment 374113 [details] [review]:

Thanks for the patch. It's a good catch.


A nitpick: the subject line, should start with the namespace of "plugins: ...". You can look the formats of the commits in that file.

::: gst/vaapi/gstvaapipluginutil.c
@@ +195,3 @@
     return GST_VAAPI_DISPLAY_TYPE_EGL;
+#else
+  (void) gl_platform_type;

Tricking the compiler perhaps here is not the best option. I would take a different approach: move the declaration of gl_platform_type into the guarded block. To say:

#if USE_EGL
  {
       const gchar *const gl_platform_type = g_getenv ("GST_GL_PLATFORM");
       if (g_strcmp0 (gl_platform_type, "egl") == 0
         ...
  }
#endif

and remove the assignation at the beginning of the function

@@ +283,3 @@
+{
+  return NULL;
+}

This is the same approach of above and it only adds spurious code.

Add a

#if USE_EGL && GST_GL_HAVE_PLATFORM_EGL

inside of the function gst_vaapi_create_display_from_egl() as in gst_vaapi_get_egl_handle_from_gl_display()
Comment 3 Víctor Manuel Jáquez Leal 2018-10-31 18:38:18 UTC
Also, there's no need to send the patch to the mailing list. The workflow in GStreamer is not that way, but through bugzilla (and in a few days or weeks through fdo's gitlab)
Comment 4 He Junyan 2018-11-01 06:58:20 UTC
Created attachment 374116 [details] [review]
V2 patch
Comment 5 He Junyan 2018-11-01 06:59:37 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #3)
> Also, there's no need to send the patch to the mailing list. The workflow in
> GStreamer is not that way, but through bugzilla (and in a few days or weeks
> through fdo's gitlab)

OK, get it.
Really a little different from my previous opensource project experience:)
Comment 6 Víctor Manuel Jáquez Leal 2018-11-01 13:21:28 UTC
Created attachment 374126 [details] [review]
plugins: Fix build error when GL is enabled while EGL is disabled.

gl_platform_type in gst_vaapi_get_display_type_from_gl_env generate
unused-variable warning and may block build when Werror enabled.
Several functions like gst_vaapi_display_egl_new_with_native_display
have no prototype warning and link error when GL is enabled but EGL
is disabled. Fix all these warning and link error.

https://bugzilla.gnome.org/show_bug.cgi?id=797358

Signed-off-by: Junyan He <junyan.he@intel.com>
Comment 7 Víctor Manuel Jáquez Leal 2018-11-01 13:22:21 UTC
@Junyan,

Please verify if the uploaded version (I changed some bits only) works for you.

Thanks.
Comment 8 He Junyan 2018-11-02 04:53:52 UTC
That can not work,

gstvaapipluginutil.c:219:1: error: ‘gst_vaapi_get_egl_handle_from_gl_display’ defined but not used [-Werror=unused-function]
 gst_vaapi_get_egl_handle_from_gl_display (GstGLDisplay * gl_display)
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
gstvaapipluginutil.c:202:1: error: ‘gst_vaapi_get_gles_version_from_gl_api’ defined but not used [-Werror=unused-function]
 gst_vaapi_get_gles_version_from_gl_api (GstGLAPI gl_api)
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


gst_vaapi_get_egl_handle_from_gl_display
will be called inside your #ifdef

It is really easy to make mistake when so many #ifdef
Comment 9 GStreamer system administrator 2018-11-03 15:56:26 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/issues/111.