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 748425 - GL context on android has 16bits colors
GL context on android has 16bits colors
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2015-04-24 17:03 UTC by Xavier Claessens
Modified: 2015-04-27 14:48 UTC
See Also:
GNOME target: ---
GNOME version: ---

incomplete patch (1010 bytes, patch)
2015-04-24 17:55 UTC, Xavier Claessens
rejected Details | Review
patch (3.05 KB, patch)
2015-04-24 19:28 UTC, Xavier Claessens
needs-work Details | Review
egl: Use maximum bits per color instead of minimum (1.46 KB, patch)
2015-04-27 14:12 UTC, Xavier Claessens
committed Details | Review

Description Xavier Claessens 2015-04-24 17:03:55 UTC
gst_gl_context_egl_choose_config() does not specify which color format to use. By default android often gives 16bits colors, even if the screen does support 24bits colors.

I've tested 3 devices:

1) nexus4: with android 5.0 gstreamer displays 16bits colors with no dithering, but with android 5.1 it gets 24bits colors and look perfect.

2) Galaxy S5: 16bits colors, no dithering.

3) Tab4: 16bits colors, with good dithering.
Comment 1 Xavier Claessens 2015-04-24 17:55:45 UTC
Created attachment 302312 [details] [review]
incomplete patch

This trivial patch fix the problem, but we probably want to verify that 24bits is supported otherwise we could be compatibility issues I guess.
Comment 2 Sebastian Dröge (slomo) 2015-04-24 17:57:22 UTC
Yeah, see :)
Comment 3 Xavier Claessens 2015-04-24 19:28:22 UTC
Created attachment 302316 [details] [review]

Choosing one config returned by eglGetConfigs seems complicated, so I opted to try with 24bits then fallback it failed.

I'm not sure the fallback is actually required here, I doubt there are platforms where 24bits colors are not supported. Even on devices that have 16bits screens the EGL platforms should support 24bits AFAIK.
Comment 4 Sebastian Dröge (slomo) 2015-04-25 10:39:09 UTC
I think what you usually do there is to just iterate over all configs and select the one that is "best". I'll take a look at implementing this in a few minutes :)
Comment 5 Sebastian Dröge (slomo) 2015-04-25 10:46:49 UTC
Actually that's what eglChooseConfig() is doing. It gives all configs that are compatible with the attributes we provided. Nevermind :)
Comment 6 Sebastian Dröge (slomo) 2015-04-25 10:49:15 UTC
Comment on attachment 302316 [details] [review]

Changing a static variable here is not a good idea. The supported configs are different depending on the display.
Comment 7 Sebastian Dröge (slomo) 2015-04-25 10:52:08 UTC
Review of attachment 302316 [details] [review]:

::: gst-libs/gst/gl/egl/gstglcontext_egl.c
@@ +177,3 @@
+      EGL_DEPTH_SIZE, 16,

Might make sense to also try with that being set to 0 if all fails

@@ +179,3 @@
+      EGL_DEPTH_SIZE, 16,
+      EGL_BUFFER_SIZE, 24,
+      EGL_ALPHA_SIZE, 1,

We only care about alpha size on RPI/Wayland. In general it does not matter much for us if the config supports alpha or not.
Comment 8 Xavier Claessens 2015-04-27 14:12:53 UTC
Created attachment 302446 [details] [review]
egl: Use maximum bits per color instead of minimum
Comment 9 Xavier Claessens 2015-04-27 14:18:19 UTC
Reading the eglChooseConfig doc again, turns out it's much easier than my previous patch. When specifying EGL_RED/GREEN/BLUE/_SIZE instead of EGL_BUFFER_SIZE, it will take the MAX possible value instead of the MIN. So setting them to 1 should work in all cases. That's also what the GLX context does.
Comment 10 Nicolas Dufresne (ndufresne) 2015-04-27 14:47:34 UTC
Attachment 302446 [details] pushed as 5bfaaf4 - egl: Use maximum bits per color instead of minimum
Comment 11 Nicolas Dufresne (ndufresne) 2015-04-27 14:48:01 UTC
Review of attachment 302446 [details] [review]:

Comment 12 Nicolas Dufresne (ndufresne) 2015-04-27 14:48:20 UTC