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 639763 - [dvbsuboverlay] Green borders around subtitles
[dvbsuboverlay] Green borders around subtitles
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Windows
: Normal normal
: 0.10.22
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-01-17 17:11 UTC by Raimo Järvi
Modified: 2011-03-18 08:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Subtitle screen shot (240.21 KB, image/png)
2011-01-17 17:11 UTC, Raimo Järvi
  Details
Fix using alpha values in blitting. (4.69 KB, patch)
2011-03-16 22:43 UTC, Raimo Järvi
reviewed Details | Review
Updated patch (5.91 KB, patch)
2011-03-17 19:07 UTC, Raimo Järvi
committed Details | Review

Description Raimo Järvi 2011-01-17 17:11:25 UTC
Created attachment 178532 [details]
Subtitle screen shot

DVB subtitle overlay draws green, transparent borders around subtitles, see the attached screen shot for an example. I don't see similar borders in other software, e.g. VLC, when I play the same streams.

Here's some transport stream files that produce the problem:

https://bugs.launchpad.net/me-tv/+bug/624781
Comment 1 Mart Raudsepp 2011-02-01 00:04:19 UTC
Interestingly commit 994156c1b - "dvbsuboverlay: remove unnecessary RGB -> YUV conversion by using YUV palettes" seemingly made the green borders even thicker than before in initial code and also changed the tones of the white fill.

The sample media does not seem to use alpha values other than 0 or 255.
When observing the look of it before commit 994156c1b, it seems as if it has a green tint next to subtitle overlayed pixels at places where no transparency changes to full transparency in the middle of a 2x2 subsampled block. E.g, the top side of both lines are nicely aligned to the 2x2 blocks, and I can't observe any green tinting above, while it doesn't align with the bottom of a 2x2 block at the bottom of the text, and there's one pixel of green shade. Same with various places on the right and left edges. Looked at it in magnification of a screenshot in gimp.
So there it smells like there's some issue with alpha change handling in the I420 blending code, but the shading seemingly being strengthened to go across the 2x2 blocks after change to YUV CLUT palette is weird.
Comment 2 Raimo Järvi 2011-03-16 22:40:25 UTC
Apparently this is caused by using the average alpha value for all pixels in the 2x2 block, instead of using each pixel's own alpha value. Background color is transparent green, hence the green tint if there are both transparent and opaque pixels in the block.
Comment 3 Raimo Järvi 2011-03-16 22:43:48 UTC
Created attachment 183582 [details] [review]
Fix using alpha values in blitting.
Comment 4 Mart Raudsepp 2011-03-17 02:21:24 UTC
Review of attachment 183582 [details] [review]:

Nice work! I was debugging this today as well, and got to alpha values being the problem, but after a break fortunately noticed bugmail from you instead of continuing :)
Got around to looking through it, thinking about it and testing some, and indeed this is the issue and the obvious fix. Now I also remember (and re-confirmed) that Finnish DVB Subtitle streams tend to contain green YUV values for the parts where full transparency is declared in the alpha channel, back from the decoding work when I spit those out in raw PNGs.

I see you already work inside GIT - maybe you could also commit it with a nice commit message and send that (after possibly tweaking something per this or other comments), so it's easily applied with proper authorship information?

::: gst/dvbsuboverlay/gstdvbsuboverlay.c
@@ +657,1 @@
 

I see you simplify the logic of dealing with this by pre-multiplying U and V channels above. Maybe the same premultiplying logic would be nice for the last block too (for odd height and width), where there's no alpha issue concerns, just for consistencies sake?
Comment 5 Raimo Järvi 2011-03-17 19:07:59 UTC
Created attachment 183667 [details] [review]
Updated patch
Comment 6 Sebastian Dröge (slomo) 2011-03-18 08:34:55 UTC
Yes this change absolutely makes sense. I wonder why I thought that averaging the alpha was a good idea at all :)

commit 9e7d1ba888af466a2625484d3091f701c70e9906
Author: Raimo Järvi <raimo.jarvi@gmail.com>
Date:   Thu Mar 17 20:19:27 2011 +0200

    dvbsuboverlay: Fix using alpha values in blitting.
    
    Use each pixel's own alpha value instead of average alpha value when
    calculating color components. Fixes bug #639763.