[Bug 797358] New: Util: Build error when GL is enabled while EGL is disabled

["GStreamer" (GNOME Bugzilla)]

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

Bug ID: 797358
Summary: Util: Build error when GL is enabled while EGL is
disabled
Classification: Platform
Product: GStreamer
Version: unspecified
OS: Linux
Status: NEW
Severity: normal
Priority: Normal
Component: gstreamer-vaapi
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@hotmail.com
QA Contact: gstreamer-***@lists.freedesktop.org
CC: ***@gmail.com, ***@igalia.com
GNOME version: ---

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

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #1 from He Junyan <***@hotmail.com> ---
Created attachment 374113
--> https://bugzilla.gnome.org/attachment.cgi?id=374113&action=edit
The patch

["GStreamer" (GNOME Bugzilla)]

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

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Version|unspecified |git master
Summary|Util: Build error when GL |vaapi: build error when GL
|is enabled while EGL is |is enabled and EGL disabled
|disabled |

["GStreamer" (GNOME Bugzilla)]

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

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #374113|none |needs-work
status| |

--- Comment #2 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 374113:
--> (https://bugzilla.gnome.org/review?bug=797358&attachment=374113)

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()

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #3 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
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)

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #4 from He Junyan <***@hotmail.com> ---
Created attachment 374116
--> https://bugzilla.gnome.org/attachment.cgi?id=374116&action=edit
V2 patch

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #5 from He Junyan <***@hotmail.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #3)

Post by "GStreamer" (GNOME Bugzilla)
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:)

["GStreamer" (GNOME Bugzilla)]

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

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #374113|0 |1
is obsolete| |

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #6 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Created attachment 374126
--> https://bugzilla.gnome.org/attachment.cgi?id=374126&action=edit
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 <***@intel.com>

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #7 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
@Junyan,

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

Thanks.

["GStreamer" (GNOME Bugzilla)]

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

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #374116|0 |1
is obsolete| |

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #8 from He Junyan <***@hotmail.com> ---
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

["GStreamer" (GNOME Bugzilla)]

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

GStreamer system administrator <***@gstreamer.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE

--- Comment #9 from GStreamer system administrator <***@gstreamer.net> ---
-- 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.