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 762764 - green uniform image with the video-sink (pb with glsl i420 shader)
green uniform image with the video-sink (pb with glsl i420 shader)
Status: RESOLVED FIXED
Product: clutter-gst
Classification: Other
Component: general
2.0.x
Other Linux
: Normal normal
: ---
Assigned To: clutter-gst-maint
clutter-gst-maint
: 757332 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-02-27 10:07 UTC by Fabrice Bellet
Modified: 2016-03-04 09:10 UTC
See Also:
GNOME target: 3.20
GNOME version: ---


Attachments
video-sink: fix the glsl i420 shader (1.70 KB, patch)
2016-02-27 10:07 UTC, Fabrice Bellet
none Details | Review
video-sink: fix the i420 arb fragment program (2.03 KB, patch)
2016-03-01 22:30 UTC, Fabrice Bellet
none Details | Review
video-sink: fix shaders with the removal of luminance texture in GL3 (7.05 KB, patch)
2016-03-02 09:46 UTC, Fabrice Bellet
committed Details | Review

Description Fabrice Bellet 2016-02-27 10:07:48 UTC
Created attachment 322519 [details] [review]
video-sink: fix the glsl i420 shader

Hi!

In Fedora 23, I noticed that applications using the clutter-gst 2.0 video sink (for example the thumbnail webcam image in the empathy-call application) show a green uniform image. For example, this is visible with an AMD card :

gst-launch-1.0 playbin uri=http://samples.mplayerhq.hu/V-codecs/h264/NeroAVC.mp4 video-sink=cluttersink

It also causes a lot of cogl warnings:

(gst-launch-1.0:29986): Cogl-WARNING **: driver/gl/gl/cogl-texture-driver-gl.c:488: GL error (1281): Invalid value
(gst-launch-1.0:29986): Cogl-WARNING **: driver/gl/cogl-util-gl.c:96: GL error (1281): Invalid value

It works fine using the video-sink from clutter-gst 3.0 (clutterautovideosink), and it appeared since clutter switched to the GL3 driver with this commit https://git.gnome.org/browse/clutter/commit/?id=9a510c01 moving from ARBfp program to GLSL.

I looked for differences in the video sink between clutter-gst 2.0 and 3.0, and found that the yv12 to rgba shader changed using the alpha component instead of the luminance component. Backporting this modification only in the 2.0 branch makes it work for me, even if I cannot explain the reason _why_.
Comment 1 Fabrice Bellet 2016-03-01 22:30:01 UTC
Created attachment 322807 [details] [review]
video-sink: fix the i420 arb fragment program

the modification needs to be included in the ARB fragment program to preserve compatibility too.
Comment 2 Lionel Landwerlin 2016-03-02 01:16:01 UTC
Review of attachment 322519 [details] [review]:

Looks good.
Comment 3 Fabrice Bellet 2016-03-02 09:46:48 UTC
Created attachment 322831 [details] [review]
video-sink: fix shaders with the removal of luminance texture in GL3

I forgot a couple of other cases in previous patches:

  * there should be two distincts i420_to_rgba_shader and yv12_to_rgba_shader that differ by u and v being swapped. -> required for YV12 with GL3
  * in i420 ARB fragment program, I restore the order of the components near the end with MUL result.color.xyz, fragment.color.primary.w, R0.xwzy.
  * I update the yv12 fragment program in the same way
  * I also fix the nv12_to_rgba_shader that is concerned by the same modification.

I tested the patch with this pipeline :

gst-launch-1.0 videotestsrc ! video/x-raw,format=I420 ! videoconvert ! cluttersink

running all combinations obtained with COGL_DRIVER=gl3, COGL_DRIVER=gl, and with format=I420, format=YV12, format=NV12 and format=AYUV (compared with xvimagesink)

(I didn't update the source files that generate I420.h and YV12.h)
Comment 4 Emmanuele Bassi (:ebassi) 2016-03-03 17:52:51 UTC
Review of attachment 322831 [details] [review]:

LGTM.
Comment 5 Matthias Clasen 2016-03-04 03:20:09 UTC
*** Bug 757332 has been marked as a duplicate of this bug. ***
Comment 6 Lionel Landwerlin 2016-03-04 09:10:02 UTC
Review of attachment 322831 [details] [review]:

Pushed to 2.0 branch.