GNOME Bugzilla – Bug 681447
video overlay composition: fix video blending over transparent frame
Last modified: 2015-11-04 21:23:21 UTC
Created attachment 220668 [details] [review] blending fix patch The gst_video_blend function does not work as expected when blending over transparent pixels, the destination alpha is not changed currently. The attached patch implements standard OVER blending operation, and adds support for missing premultiplied/non-premultiplied combinations for source/destination frames.
Any chance you could add unit test bits for this as well?
Comment on attachment 220668 [details] [review] blending fix patch According to comments on IRC this patch is not quite right yet.
*** Bug 708882 has been marked as a duplicate of this bug. ***
It would be very interesting to know what has been said on IRC about why this patch is not right yet :)
No details just rawoul: __tim: https://bugzilla.gnome.org/show_bug.cgi?id=681447, the patch is actually wrong... rawoul: sorry I haven't tested properly... Arnaud - do you remember?
Created attachment 263194 [details] [review] video: blend using OVER operation This version should work in all cases. Hopefully the compiler can optimize some cases, for example when the global alpha is 255, it should remove all the * 255 / 255 in the code... I'm not sure how orc could be used properly here, maybe it would be easier to apply the global alpha first on the source line if needed, and then do the blending. The non-premultiplied to non-premultiplied orc blending function can be copied from videomixer, and the other versions can be adapted from there.
I pretty sure the compiler won't be able to do that. Performance whise, this code is pretty terrible, but as it did not work in most cases before, I'll all for it. Unit tests would be nice.
Yes, the code is probably not very fast :) However, in the new code, no operation is done when the source pixel has zero alpha, so it's probably faster than the previous code since most overlays have a lot of transparent pixels.
Comment on attachment 263194 [details] [review] video: blend using OVER operation Looks good except for the performance... maybe think about optimising it a bit ;) But apart from that I think it's good to be merged once you provide a unit test.
Ping? :)
*** Bug 739695 has been marked as a duplicate of this bug. ***
Created attachment 290325 [details] [review] unit test for video blending over transparent frame basically blending a 100x50 white (0xFF) box in the right half of a 200x50 transparent (0x00) frame and checking to see whether the alpha byte is set to 0xFF. without this patch, the alpha byte stays 0x00 and thus the test will fail.
Note that this basically converts ARGB to xRGB and makes all frames opaque. Not sure if that is generally the expected behaviour.
this is just how assrender does it, too.
(In reply to comment #13) > Note that this basically converts ARGB to xRGB and makes all frames opaque. Not > sure if that is generally the expected behaviour. This is clearly not what we expect. In GES we have: videotestsrc ! video/x-raw,format=ARGB ! textoverlay ! .... to create title clips and we definitely want the output of textoverlay to also be ARGB in that case so that it is transparent when mixed with the compositor downstream.
Is the patch in bug 708882 is correct? https://bugzilla.gnome.org/attachment.cgi?id=255889&action=diff
(In reply to Alex Băluț from comment #16) > Is the patch in bug 708882 is correct? > https://bugzilla.gnome.org/attachment.cgi?id=255889&action=diff Patch does not have enough context line for me to confirm now. I'll have to look at where is "alpha" computed. Just bring that patch over here, and we'll try and sort out. To all, remember that colors are premultiplied by their alpha.
Created attachment 308218 [details] [review] https://bug708882.bugzilla-attachments.gnome.org/attachment.cgi?id=255889 This is Mathieu's patch to be discussed whether it makes sense.
Created attachment 313852 [details] [review] video: blend using OVER operation Also support all premultiplied/non-premultiplied source/destination configurations
Created attachment 313853 [details] [review] unit test for video blending over transparent frame, refs https://bugzilla.gnome.org/show_bug.cgi?id=681447
Just rebased arnaud's patches. We really need that bug to be fixed in Pitivi, it looks like those patches are correct even though they deserve some optimization.
Review of attachment 308218 [details] [review]: See https://bugzilla.gnome.org/show_bug.cgi?id=708882#c3 why it's not good.
Attachment 313852 [details] pushed as dfe250d - video: blend using OVER operation