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 759867 - gl: Add support to compose VideoOverlayCompositionMeta before the display sink
gl: Add support to compose VideoOverlayCompositionMeta before the display sink
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-25 19:39 UTC by Florent Thiéry
Modified: 2018-10-04 04:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gl: add a new overlay compositor element (14.67 KB, patch)
2018-09-27 06:41 UTC, Matthew Waters (ystreet00)
committed Details | Review
glmixerbin: add gloverlaycompositor to each input stream (2.37 KB, patch)
2018-09-27 06:43 UTC, Matthew Waters (ystreet00)
committed Details | Review

Description Florent Thiéry 2015-12-25 19:39:09 UTC
My use case is to render text overlays (textoverlay/timeoverlay) with GLES on Raspberrypi ; i apply a fragment shader and the timeoverlay is currently directly rendered by the sink and does not get processed through the shader:

gst-launch-1.0 videotestsrc ! glupload ! timeoverlay ! glshader location=oculus.frag ! glimagesink

(shader here: https://github.com/fthiery/rpi-oculus-fpv/blob/master/rpi-oculus-fpv/oculus.frag)

However, for testing sake, the issue can be reproduced using any gl effect like glfiltercube; the expected result is to see the timeoverlay on every cube face instead of the top left sink corner:

gst-launch-1.0 videotestsrc ! glupload ! timeoverlay ! glcolorconvert ! glfiltercube ! glimagesink

The expected result can be obained by moving timeoverlay before glupload (but in that case, text rendering is done in software, which i cannot afford and may require unnecessary texture download/upload):

gst-launch-1.0 videotestsrc ! timeoverlay ! glupload ! glcolorconvert ! glfiltercube ! glimagesink

Also, why is the following failing to link (resulting in requiring a glcolorconvert element) ?
gst-launch-1.0 videotestsrc ! glupload ! timeoverlay ! glfiltercube ! glimagesink
WARNING: erroneous pipeline: could not link timeoverlay0 to glfiltercube0
Comment 1 Matthew Waters (ystreet00) 2015-12-28 02:11:17 UTC
> gst-launch-1.0 videotestsrc ! glupload ! timeoverlay ! glfiltercube ! glimagesink

This doesn't work because timeoverlay doesn't support memory:GLMemory *or* glfiltercube doesn't support meta:GstVideoOverlayComposition.  Which is expected at this point.

Also, what's the difference between this bug and bug #759860 ?
Comment 2 Nicolas Dufresne (ndufresne) 2015-12-28 02:52:59 UTC
It's a feature request, what I have in mind, is that we could add meta:GstVideoOverlayComposition support to glvideomixer, or add new element like glvideooverlaycomposer that will input memory:GLMemory,meta:GstVideoOverlayComposition and will output memory:GLMemory.

Composing the overlay is similar to what video mixer does, except that the overlays need to be uploaded. A seperate element is probably much simpler as we have an helper library, though being able to compose and mix in a single pass will be more future proof for performance and pipeline simplicity.
Comment 3 Florent Thiéry 2015-12-28 12:05:09 UTC
Are you saying that the font rendering will be rendered in software, then uploaded to the GPU as RGBA ? If so, why not adding meta:GstVideoOverlayComposition support to the existing gloverlay element (which is, if i'm not mistaken, currently targeted at image files overlays).

The only aspect that troubles me with the approach that Nicolas suggests is the requirement for developers to know/guess that a non-dedicated element (whose name isn't text-related) is required for the actual rendering to be effective. Ease of use / elegance-wise, i think it would be better to add memory:GLMemory support to text/timeoverlay elements directly, so that if the element is added within a GL section, it will perform adequately.
Comment 4 Matthew Waters (ystreet00) 2015-12-28 12:20:50 UTC
Adding memory:GLMemory to text/timeoverlay would require that text/timeoverlay perform a blend with GL and gain a dependency on GL which is a no-go.
Comment 5 Nicolas Dufresne (ndufresne) 2015-12-28 20:30:01 UTC
We could of course opt for a text renderer in GL, but that's another subject imho.
Comment 6 Florent Thiéry 2016-01-02 23:37:17 UTC
(In reply to Matthew Waters (ystreet00) from comment #1)
> > gst-launch-1.0 videotestsrc ! glupload ! timeoverlay ! glfiltercube ! glimagesink
> 
> This doesn't work because timeoverlay doesn't support memory:GLMemory *or*
> glfiltercube doesn't support meta:GstVideoOverlayComposition.  Which is
> expected at this point.

Yes, but then why is glcolorconvert allowing this to link/work in 1.6 but not master ?

gst-launch-1.0 videotestsrc ! glupload ! timeoverlay ! glcolorconvert ! glfiltercube ! glimagesink

In 1.6, what was actually performing the text rendering (pango in the timeoverlay element ?). Was timeoverlay generating small (i.e. resolution of just the label dimensions) raw video buffers (RGBA) which are then passed downstream as GstVideoOverlayCompositionMeta, and the first downstream element that supports GstVideoOverlayCompositionMeta uploads the buffer to the GL context and blends it ?

Documentation states "it can be used to attach overlay information (subtitles or logos) to non-raw video buffers such as GL/VAAPI/VDPAU surfaces. The actual blending of the overlay can then be done by e.g. the video sink that processes these non-raw buffers"; by having the timeoverlay in the middle of the GL pipeline, what is actually happening with regards to text rendering ? Is the time overlay considered a subtitle, or a texture ? 

Sure, accelerated font rendering would require dedicated gltext/timeoverlay elements (pango/cairo...).

As a workaround, i could render text in software to RGBA/PNG and then overlay manually using gloverlay. Not really an option for time overlays but maybe for OSD stuff...
Comment 7 Matthew Waters (ystreet00) 2018-09-27 06:41:37 UTC
Created attachment 373780 [details] [review]
gl: add a new overlay compositor element
Comment 8 Matthew Waters (ystreet00) 2018-09-27 06:42:56 UTC
Above patch depends on a couple of other fixes available from: https://github.com/ystreet/gst-plugins-base/commits/glcomposition
Comment 9 Matthew Waters (ystreet00) 2018-09-27 06:43:36 UTC
Created attachment 373783 [details] [review]
glmixerbin: add gloverlaycompositor to each input stream
Comment 10 Matthew Waters (ystreet00) 2018-09-27 06:48:08 UTC
A separate element is much simpler than attempting to deal with compositing multiple-sources-as-one in a single pass in glvideomixer with the funky blend parameters.
Comment 11 Sebastian Dröge (slomo) 2018-09-27 09:54:12 UTC
Comment on attachment 373783 [details] [review]
glmixerbin: add gloverlaycompositor to each input stream

Maybe mention in the commit message that glimagesink (the bin) does not need it. And I don't think we have other bins where this could be useful
Comment 12 Sebastian Dröge (slomo) 2018-09-27 09:57:27 UTC
Review of attachment 373780 [details] [review]:

Looks good to me

::: ext/gl/gstgloverlaycompositorelement.c
@@ +66,3 @@
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE_WITH_FEATURES
+        (GST_CAPS_FEATURE_MEMORY_GL_MEMORY,

If downstream supports the overlay composition meta, should we maybe not render it here and passthrough?
Comment 13 Nicolas Dufresne (ndufresne) 2018-09-27 12:34:32 UTC
Cool stuff, I have one concern though, the font rendering will be low res if input streams are scaled during composition. That's slightly related to Sebastian comment I must admit. We also add one render pass per input streams, while adding composition overlay to the buffer being composed seemed more efficient.
Comment 14 Matthew Waters (ystreet00) 2018-09-28 04:01:36 UTC
(In reply to Nicolas Dufresne (ndufresne) from comment #13)
> Cool stuff, I have one concern though, the font rendering will be low res if
> input streams are scaled during composition. That's slightly related to
> Sebastian comment I must admit. We also add one render pass per input
> streams, while adding composition overlay to the buffer being composed
> seemed more efficient.

Doing overlay compositing in glvideomixer itself is either a load of hurt to deal with the different blending modes or would result in effectively the same thing (render to temporary and then blend).  This gets a nice middle ground that can also be used elsewhere in front of any GL element.

Ideally, one shouldn't scale when compositing at glvideomixer however I can see why that might cause some design/architecture issues.

I don't think a download -> composite -> upload for a full frame is less performant than only uploading the overlays.
Comment 15 Sebastian Dröge (slomo) 2018-10-02 08:46:48 UTC
I think what Nicolas means is the "window size" support in the allocation query for the overlay composition meta. The sink (or in our case glvideomixer) could specify in the allocation query response that it will render the overlays on a surface of size AxB while the surface size at the overlay element is CxD (and usually smaller). The overlay element would then produce overlays at a higher resolution so that they look as nice as possible at the point where they're burned into the actual video frame.

Now if you do the overlay composition before glvideomixer, you have the problem that glvideomixer could scale the inputs itself just before doing its thing. And that would be now *after* burning the overlays into the actual video frame from what I understand. Scaling of inputs in glvideomixer happens at the composition stage, not in the glcolorconvert before, right?


IMHO this shouldn't block this patchset but should just get a FIXME comment somewhere in the code.
Comment 16 Matthew Waters (ystreet00) 2018-10-02 11:16:26 UTC
(In reply to Sebastian Dröge (slomo) from comment #15)
> Now if you do the overlay composition before glvideomixer, you have the
> problem that glvideomixer could scale the inputs itself just before doing
> its thing. And that would be now *after* burning the overlays into the
> actual video frame from what I understand. Scaling of inputs in glvideomixer
> happens at the composition stage, not in the glcolorconvert before, right?

Yea, scaling in glvideomixer (the bin and element) is on composition rather than in a preceding element.

> IMHO this shouldn't block this patchset but should just get a FIXME comment
> somewhere in the code.

I have a small modification for making gloverlaycompositor send the window size upstream.  For glvideomixer, that probably also needs to either prefer the current sized caps (something glimagesink could do with as well) that's not currently easy with caps negotiation or implement a 'render-size event/query'.
Comment 17 Nicolas Dufresne (ndufresne) 2018-10-02 12:12:02 UTC
Yes, this is what Sebastian says. Unlike videos, fonts scales really badly.
Comment 18 Sebastian Dröge (slomo) 2018-10-03 14:51:02 UTC
Comment on attachment 373780 [details] [review]
gl: add a new overlay compositor element

Good to go except for:

_object_set_is_valid_property: object class 'GstGLOverlayCompositor' has no property named 'yinvert'

:)
Comment 19 Matthew Waters (ystreet00) 2018-10-03 15:54:20 UTC
(In reply to Sebastian Dröge (slomo) from comment #18)
> _object_set_is_valid_property: object class 'GstGLOverlayCompositor' has no
> property named 'yinvert'
> 
> :)

(In reply to Matthew Waters (ystreet00) from comment #8)
> Above patch depends on a couple of other fixes available from:
> https://github.com/ystreet/gst-plugins-base/commits/glcomposition

?
Comment 20 Sebastian Dröge (slomo) 2018-10-03 19:54:05 UTC
Comment on attachment 373780 [details] [review]
gl: add a new overlay compositor element

All working fine with the other changes from your branch :)
Comment 21 Matthew Waters (ystreet00) 2018-10-04 04:35:50 UTC
-base
commit f960beaa98ddd144a8f3d1974ef0093d20a00450
Author: Matthew Waters <matthew@centricular.com>
Date:   Thu Sep 27 13:35:15 2018 +1000

    gl: add a new overlay compositor element
    
    Flattens all the overlays from the GstVideoOverlayCompositionMeta into
    the video stream.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759867

-bad
commit ea355ca0a3f78284762a5d3a88912dc6b2418894
Author: Matthew Waters <matthew@centricular.com>
Date:   Thu Sep 27 16:37:28 2018 +1000

    glmixerbin: add gloverlaycompositor to each input stream
    
    Flattens the overlay compositions into the stream before the mixer will
    mix them.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759867