GNOME Bugzilla – Bug 766993
Android decodebin: zero copy between hardware decoder and glimagesink doesn't work (regression in 1.9)
Last modified: 2016-06-21 23:12:39 UTC
A regression has occurred in decodebin plugin. It's not working any more to force zero-copy by linking hardware decoder and glimagesink in 1.9 with the right autoplug_query callback to mimick the behavior of playbin. It's working in 1.8.1. The same code leads to a black screen in 1.9. Might be linked to https://bugzilla.gnome.org/show_bug.cgi?id=766201. Different from https://bugzilla.gnome.org/show_bug.cgi?id=742924.
Created attachment 328703 [details] working-1.8.1
Created attachment 328704 [details] not working 1.9
Could you please get a debug log with *:6 of the failing 1.9.1?
Created attachment 328709 [details] not working 1.9 detailed
I have provided the requested level-6 log. Shall we change the status? Do you have any idea what could be the problem?
fragment shader compilation failed:0(21) : error C1115: unable to find compatible overloaded function "texture(struct samplerExternalOES, mediump vec2)" This points to: 57a4494528e60c2bcb63616300e72b90c1a8bf5f glcolorconvert: GLES3 deprecates texture2D() and it does not work at all in newer versions than 3.3 Use the newer texture() function instead. This fixes glimagesink and other things on various Android devices. and bug 765266.
OK. Which newer function shall I use? Where to get it? Which part of my Android code is it? Is it in the autoplug-query callback or way before somewhere else?
It's in the GLSL shader code, where it is mangled for the different GL versions we need a special case for ExternalOES textures. Matthew was looking into fixing that.
I'm confused because Matthew says "Use the newer texture() function instead", implying that I need to change things in my application. Should I change my code or should I wait for a newer version of gst-plugin-bad?
That's the commit + description that caused your issue and it was not directed directly at you Gregoire. It will need to be fixed in gst-plugins-bad. No change of code required from your side.
Got it! Thanks. So I will continue watching the bug then.
So this is a regression introduced during 1.9 and needs to be fixed before 1.9.1. Let's mark it as such :)
Gregoire, I can't tell from the log as the full GL_EXTENSIONS value is cut off but is the https://www.khronos.org/registry/gles/extensions/OES/OES_EGL_image_external_essl3.txt extension advertised on your device? You can check by inspecting that value yourself somewhere or with a GL capabilities app in the Google Play store.
I mentioned that it was not working on both PixelC and Nexus5x. OpenGL Check on playstore gives me: OpenGL ES version 3.1 on PixelC.
Does it tell you that the specific extension Matthew mentioned is supported?
dumpsys | grep GLES reports: GLES: NVIDIA Corporation, NVIDIA Tegra, OpenGL ES 3.2 NVIDIA 355.00
The rendering doesn't work on Nexus5x and you can read this page: http://delphigl.de/glcapsviewer/gles_listreports.php?extension=OES_EGL_image_external_essl3
And this page too: http://delphigl.de/glcapsviewer/gles_generatereport.php?reportID=780
I want to amend the results of my QA tests: Nexus 5x: WORKING Nexus 7 2013: WORKING Pixel C: NOT WORKING So, it seems now (git clone as this morning, June 7), only the Pixel C shows the problem.
And to be super complete: Nexus 5x: Android 6.x Nexus 7 2013: Android 5.x Pixel C: Android 6.x
I would like to accelerate on this issue as I need to wrap up a release. As you don't have a Pixel C, you will have to guide me to feed you with info you need. So what do you need to understand why it's not working on Pixel C?
To make sure that my hunch is correct, you could test if changing the: "#extension GL_OES_EGL_image_external : require\n" at https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/gstglshaderstrings.c#n72 to "#extension GL_OES_EGL_image_external_essl3 : require\n" fixes the issue on the Pixel C.
No, still not working with the suggested one-line modification.
With the same error? What does it say now?
Created attachment 329481 [details] Log level5 essl3
Attached log with the suggested one-line hack
It fails the same way. Could you get a gl*:7 log of that please? And another option is to remove the entire if statement from https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/gstglsl.c#n572 and only run with only the else statement. I would need a gl*:7 log of that as well.
Created attachment 329522 [details] gl-level7-essl3 gl level7 with essl3 modif
Still not working with second suggestion around https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/gstglsl.c#n572
Created attachment 329524 [details] gl-level7-second suggestion
(In reply to Gregoire from comment #28) > Created attachment 329522 [details] > gl-level7-essl3 > > gl level7 with essl3 modif It's not using your changes. If you're using cerbero, you have to create a commit and point the gst-plugins-bad-1.0-static recipe at that branch/commit. 0:00:16.857413066 0xe91ef7b0 gstglslstage.c:464:_compile_shader:<glslstage1> #extension GL_OES_EGL_image_external : require (In reply to Gregoire from comment #30) > Created attachment 329524 [details] > gl-level7-second suggestion Same here: 06-09 20:54:52.583 10629 10671 V GStreamer+glslstage: vec4 t = texture(tex, texcoord * tex_scale0);
No! My compilation was correct. But!... there are other occurrences of #extension GL_OES_EGL_image_external in gstglviewconvert.c and gstglcolorconvert.c which are more important. Indeed, it's working (no black screen with zero-copy) if I add essl3 on line 156 and 134 of those 2 files. I haven't tested more the second suggestion but I'm pretty sure my compilation was right too. We should be close now.
Matthew, any chance you prepare a patch? I think that we (you) know where the modifications need to be done to support essl3 with my last comment or do you need any other Pixel C-related test?
Created attachment 329831 [details] [review] glsl: fixup external-oes textures on GLES3 by mangling the required extension Gregoire, this patch should fix your issue. Could you please test that it works on the Pixel C?
Created attachment 329834 [details] [review] glsl: fixup external-oes textures on GLES3 by mangling the required extension Updated patch that only works with external-oes targets and only uses texture() where explicitly advertised by GL_OES_EGL_image_external_essl3 for external-oes textures.
First (obsolete) patch makes it work on Pixel C.
Actullay, second version does NOT work on Pixel C.
Actually, first version is not working either. Perhaps I was mistaken in the test of comment #36. Are you very sure of your patch?
If you get a glsl*:7,gl*:5,3 debug log, it should tell you 1. the shader it tried to compile, 2. the GL_VERSION/GL_EXTENSIONS info for further debugging and 3. if it errored out the same way.
Created attachment 329836 [details] Log of glsl*:7,gl*:5,3 with second-version patch
Oh blegh, it needs to be the other way around to get the shader correctly, i.e. 3,gl*:5,glsl*:7
Created attachment 329838 [details] Log of 3,gl*:5,glsl*:7 with second-version patch
Created attachment 329840 [details] [review] glsl: fixup external-oes textures on GLES3 by mangling the required extension (with debugging) Huh? That log doesn't seem to make sense if you've applied the second patch. As you would have texture2D or the essl3 extension would have changed in the shader. This one adds an error debugging which will tell me which condition failed the check for the essl3 extension. Please get a 3,gl*:7,glsl*:7 log with this patch.
Created attachment 329841 [details] [review] glsl: fixup external-oes textures on GLES3 by mangling the required extension (with debugging) And the wrong patch was uploaded. This one has the debugging.
Created attachment 329842 [details] log
The debug is not there. 1. You need to point cerbero to the new commit for both gst-plugins-bad-1.0 and gst-plugins-bad-1.0-static 2. You need to rebuild both gst-plugins-bad-1.0 and gst-plugins-bad-1.0-static in cerbero. As this is library code, the static libraries are taken from gst-plugins-bad-1.0. Static plugins are taken from gst-plugins-bad-1.0-static.
With latest patch, it's working fine on Pixel C but Nexus5X is broken for zerocopy.
Created attachment 329867 [details] Log of 3,gl*:5,glsl*:7 with second-version patch on Nexus5X
So any idea why it's not working any more on Nexus5x with the patch? I have other bugs to report for zerocopy on android and I don't want to mix things.
Essentially because it's a grey area with GLES3 and not having the essl3 extension. Does one use texture() or texture2D()? Different implementations seems to require different things here. About the only thing we can do is to fallback to GLES2 shaders if essl3 isn't supported.
Created attachment 329925 [details] [review] glsl: fixup external-oes textures on GLES3 by mangling the required extension (with debugging) This version also only uses GLES3 shaders if the essl3 extension is supported. Gregoire, could you test this against the Pixel C and the Nexus 5x please?
Latest patch: working on Nexus5x, NOT WORKING on PixelC.
Created attachment 329928 [details] Log pixelc
Latest version is working on Nexus 7 2013
Created attachment 329933 [details] [review] glsl: fixup external-oes textures on GLES3 by mangling the required extension (with debugging) Oops. Sorry, found a bug in the patch. Try this one instead.
This latest version is working on my three test devices: Pixel C, Nexus 5x, Nexus 7 2013. Thanks! Can you please commit so that we can write off this bug?
Any chance you commit your patch? Or do you have in mind more tests before?
Pushed a slightly modified version (only debugging). commit d3f3787d64bd0adb56bd2052e733e61f10f55acf Author: Matthew Waters <matthew@centricular.com> Date: Wed Jun 15 12:47:05 2016 +1000 glsl: fixup external-oes shaders by mangling the required extension Newer devices require using a different GLSL extension for accessing external-oes textures in a shader using the texture() functions. While the GL_OES_EGL_image_external_essl3 should supposedly be supported on a any GLES3 android device, the extension was defined after a lot of the older drivers were built so they will not know about it. Thus there are two possible interpretations of which of texture[2D]() should be supported for external-oes textures. Strict adherence to the GL_OES_EGL_image_external extension spec which uses texture2D() or following GLES3's pattern, also allowing texture() as a function for accessing external-oes textures This adds another mangling pass to convert #extension GL_OES_EGL_image_external : ... into #extension GL_OES_EGL_image_external_essl3 : ... on GLES3 and when the GL_OES_EGL_image_external_essl3 extension is supported. Only uses texture() when the GLES3 and the GL_OES_EGL_image_external_essl3 extension is supported for external-oes textures. Uses GLES2 + texture2D() + GL_OES_EGL_image_external in all other external-oes cases. https://bugzilla.gnome.org/show_bug.cgi?id=766993
Working now with a full clean rebuild.