GNOME Bugzilla – Bug 797091
cairooverlay: passes un-premultiplied ARGB to Cairo
Last modified: 2018-10-03 20:25:34 UTC
Cairo expects pre-multiplied alpha, we don't. So before and afterwards we need to convert between the two for every frame, which is kind of suboptimal.
This is not an enhancement but a bug. We provide wrong data to cairo, and cairo gives us even worse data back
Reworded, the way you wrote it initially, it sounded more like it was "suboptimal" rather then broken. For textoverlay we use premultiply alpha support from GstVideoOverlayComposition helpers. So in that case, if it's broken it's a regression.
It's a simple bug in cairooverlay, unrelated to any other code :) Our ARGB is un-pre-multipled, Cairo's is pre-multiplied. As such we need to convert before passing to Cairo, and another time in reverse after getting the data from Cairo. In textoverlay (where we also use Cairo) it's handled correctly.
I don't kow what iss faster, but we could render to a separate RGB buffer, and then blend (with our existing blend function) on top of the video frame. This reduces at least the color washing caused by de-pre-multiplying.
The problem is that the signal handler might look at the image surface pixels we pass to it, and that wouldn't be possible anymore then.
This should probably go together with defining a new set of video formats for RGB with pre-multiplied alpha so that in some cases the double-conversion can be prevented. Let's call them aRGB, i.e. small a instead of capital A? :)
Naming is hard, I'm worried it's too subtle, but I like that it's short. New formats seems like the most straightforward, then we can implement the conversion in videoconvert, and avoid over converting.
(In reply to Nicolas Dufresne (ndufresne) from comment #4) > I don't kow what iss faster, but we could render to a separate RGB buffer, > and then blend (with our existing blend function) on top of the video frame. > This reduces at least the color washing caused by de-pre-multiplying. I think we would want this behind a property. By first rendering to a transparent surface and then blending it over the video you also get different blending behaviour. But it would also allow for using the overlay meta and e.g. not downloading/uploading GL memories all the time.
Created attachment 373755 [details] [review] cairooverlay: Add property for drawing on a transparent surface and then blending This allows us to use the GstVideoOverlayComposition API and correctly handle pre-multiplied alpha, while also only doing the alpha conversion once instead of twice for the whole frame. At a later point we can attach the meta to the buffer instead of blending ourselves if downstream supports that.
Created attachment 373756 [details] [review] cairooverlay: Pre-multiply and un-premultiply alpha in case of ARGB32 Cairo expects pre-multiplied alpha, we work on un-premultiplied alpha.
Created attachment 373757 [details] [review] cairooverlay: Add overlay as meta to the buffers if we can This requires that downstream supports it and draw-on-transparent-surface is enabled.
Created attachment 373758 [details] [review] cairooverlay: Don't map input buffers if we just attach the overlay as meta
Created attachment 373819 [details] [review] cairooverlay: Add overlay as meta to the buffers if we can This requires that downstream supports it and draw-on-transparent-surface is enabled.
Created attachment 373820 [details] [review] cairooverlay: Don't map input buffers if we just attach the overlay as meta
Attachment 373755 [details] pushed as 9844bdb - cairooverlay: Add property for drawing on a transparent surface and then blending Attachment 373756 [details] pushed as f1c5a22 - cairooverlay: Pre-multiply and un-premultiply alpha in case of ARGB32 Attachment 373819 [details] pushed as 10446bd - cairooverlay: Add overlay as meta to the buffers if we can Attachment 373820 [details] pushed as a766dce - cairooverlay: Don't map input buffers if we just attach the overlay as meta