GNOME Bugzilla – Bug 572767
Should error out instead of doing nothing if an OpenGL feature is not present
Last modified: 2012-02-18 20:05:47 UTC
Hi, currently the OpenGL effects, etc don't error out if the framebuffer object OpenGL extension is not present but instead they do nothing. This should be changed to return GST_FLOW_ERROR from the chain function and do GST_ELEMENT_ERROR().
Hi, Those kind of checks (glewinit, framebuffer object OpenGL extension, GLSL etc..) are made in gstgldisplay.c. I could make call gst_gl_display_new(gstelement) instead of just gst_gl_display_new(). Then I would be able to call GST_ELEMENT_ERROR (gstelement, RESOURCE, NOT_FOUND, ("no framebuffer object OpenGL extension"), (NULL)) inside gstgldisplay.c. In this way we do not need to change anything else (no need to return GST_FLOW_ERROR, see glimagesink) Note that in a gl flow (for example: glupload ! glfilters ! glimagesink) the gl element that calls gst_gl_display_new is always the first one. I think it would be enough for now, at least we could handle most of the basic checks (because actually they are handle silentely, I mean no messages are posted to the bus or no signals) Any comments before I start the changes ?
GST_ELEMENT_ERROR() is not enough, you should also make sure to return an ERROR flow return from any chain function.
Should'nt the null->ready state change fail instead ?
here: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/sys/xvimage/xvimagesink.c around: /* Handle window deletion by posting an error on the bus */ GST_ELEMENT_ERROR (xvimagesink, RESOURCE, NOT_FOUND, "Output window was closed"), (NULL)); I do not see any ERROR flow return after. Am I missing something or is it a particular case ?
IMHO, this could be solved in the following way. 1. gldisplay should provide some kind of "feature checking" infrastructure: - a flag structure (GstGLFeatureFlags flags) - a bunch of feature checks in gldisplay init function, something like: if (GLEW_ARB_shading_language_100) flags |= GST_GL_FEATURE_GLSL; if (some more complex check) flags |= GST_GL_FEATURE_FBO; - some public utility method for feature checking (gst_gl_feature(s)_available?) that just check the flags structure - replace all "if (GLEW_stuff)" calls with custom flags checking 2. all base elements (upload, filter, download) could then check needed features (glfilter should provide a vmethod for custom subclasses dependencies) in the prepare_output_buffer function return FLOW_ERROR on fail 3. Maybe we could manually handle state changes and do the checks in NULL->READY as Olivier suggested, but 1 and 2 should be enough for now. Note that we could just use glew everywhere but this way we could have some more complex custom check if needed and eventually get rid of glew in the future.
commit ec027145586ff1cbb528ff0c77e824a8d1c4ef7b Author: Julien Isorce <julien.isorce@gmail.com> Date: Thu Nov 24 16:02:32 2011 +0100 feature checking: error out instead of doing nothing if an OpenGL feature is not present Fix bug #572767