Created attachment 373108 [details] [review]
compositor: Use 255 as maximum alpha instead of 256

255 will easily become 0 in the blending function as they expect
the maximum value to be 255.

Can be reproduce with

gst-launch-1.0 videotestsrc pattern=ball ! c.sink_0 \
               videotestsrc pattern=snow ! c.sink_1 \
               compositor name=c \
                 sink_0::zorder=0 sink_1::zorder=1 sink_0::crossfade-ratio=0.5 \
                 background=black ! \
               videoconvert ! xvimagesink

crossfade-ratio +/- 0.001 makes it work correctly and the same happens
at e.g. 0.25, 0.75, N*0.0625

Review of attachment 373108 [details] [review]:

That makes sense, but then, this bug should be trigerable without using crossfade-ratio too right?

Review of attachment 373108 [details] [review]:

Looks good to me, except:

::: gst/compositor/blend.c
@@ +34,3 @@
 #include <gst/video/video.h>
 
+#define BLEND(D,S,alpha) (((D) * (255 - (alpha)) + (S) * (alpha)) >> 8)

If your multiplier here is 255, you should divide by 255, not 256.

On 2nd look though, this macro isn't used anywhere and can just be deleted.

Created attachment 373120 [details] [review]
compositor: Use 255 as maximum alpha instead of 256

255 will easily become 0 in the blending function as they expect
the maximum value to be 255.

Can be reproduce with

gst-launch-1.0 videotestsrc pattern=ball ! c.sink_0 \
               videotestsrc pattern=snow ! c.sink_1 \
               compositor name=c \
                 sink_0::zorder=0 sink_1::zorder=1 sink_0::crossfade-ratio=0.5 \
                 background=black ! \
               videoconvert ! xvimagesink

crossfade-ratio +/- 0.001 makes it work correctly and the same happens
at e.g. 0.25, 0.75, N*0.0625

(In reply to Jan Schmidt from comment #3)
> 
> On 2nd look though, this macro isn't used anywhere and can just be deleted.

Done, removed :)

(In reply to Thibault Saunier from comment #2)
> Review of attachment 373108 [details] [review] [review]:
> 
> That makes sense, but then, this bug should be trigerable without using
> crossfade-ratio too right?

Probably, I didn't try very hard though. You basically would need to get alpha of 256 in the C code before passing it to orc, which happens more easily with crossfading because there you have two alphas that are individually controllable

https://bugzilla.gnome.org/show_bug.cgi?id=746247 is related to this

Attachment 373120 [details] pushed as b0ae6a5 - compositor: Use 255 as maximum alpha instead of 256