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 723674 - Make external sink(s) compatible with gst-gl
Make external sink(s) compatible with gst-gl
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-02-05 13:17 UTC by Julien Isorce
Modified: 2015-07-16 04:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pseudo code to describe concretely what's the goal (3.37 KB, text/x-csrc)
2014-02-05 13:17 UTC, Julien Isorce
  Details
context: add support for wrapping external contexts (15.16 KB, patch)
2014-02-19 09:14 UTC, Matthew Waters (ystreet00)
reviewed Details | Review
display: add display type enum (2.57 KB, patch)
2014-02-20 06:30 UTC, Matthew Waters (ystreet00)
committed Details | Review
x11: add display subclass (22.47 KB, patch)
2014-02-20 06:30 UTC, Matthew Waters (ystreet00)
committed Details | Review
context: add support for wrapping external contexts (26.12 KB, patch)
2014-02-20 06:34 UTC, Matthew Waters (ystreet00)
committed Details | Review

Description Julien Isorce 2014-02-05 13:17:15 UTC
Created attachment 268170 [details]
pseudo code to describe concretely what's the goal 

external gl sink = clutter (cluttersink/coglsink), cairo (cairosink), Qt (qtglvideosink), webkit (webkitvideosink)

Applications generally just would like to be able to use the gltexture that come from gst-gl and use it to do whatever they would like to do with it. There were several attempts with cluttershare.c, sdlshare.c, qtgltextureshare.c examples. Those demos work but it's not easy for the user.

Also most of the time applications are not using the GstVideoOverlay interface and prefer to use a dedicated video sink. For example cluttersink in totem, pitivi and cheese.


We should be able to do:

gst-launch-1.0 videotestsrc ! gleffects ! cluttersink

gst-launch-1.0 playbin uri=foo video-sink="gleffects ! cluttersink"
Comment 1 Sebastian Dröge (slomo) 2014-02-06 22:18:12 UTC
What is the problem with that? GL buffers should have the GL texture upload meta on them, which then allow e.g. cluttersink to get things into a texture.

Same goes for custom application video sinks, or applications could also directly use the gst-plugins-gl API to handle the memory.
Comment 2 Julien Isorce 2014-02-07 09:44:09 UTC
(In reply to comment #1)
> What is the problem with that? GL buffers should have the GL texture upload
> meta on them, which then allow e.g. cluttersink to get things into a texture.

The GL contexts have to shared to do that. Otherwise the texture that comes from gst-gl would not be usable within the clutter gl context.

> 
> Same goes for custom application video sinks, or applications could also
> directly use the gst-plugins-gl API to handle the memory.

Yup. And at least for sharing it would need to create a GstGLContext to wrap the clutter gl context.  (gst_gl_context_new (clutter_gl_context) , see attached file)
Comment 3 Matthew Waters (ystreet00) 2014-02-07 09:50:15 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > What is the problem with that? GL buffers should have the GL texture upload
> > meta on them, which then allow e.g. cluttersink to get things into a texture.
> 
> The GL contexts have to shared to do that. Otherwise the texture that comes
> from gst-gl would not be usable within the clutter gl context.
> 
> > 
> > Same goes for custom application video sinks, or applications could also
> > directly use the gst-plugins-gl API to handle the memory.
> 
> Yup. And at least for sharing it would need to create a GstGLContext to wrap
> the clutter gl context.  (gst_gl_context_new (clutter_gl_context) , see
> attached file)

Also requires a way to share this information.  Probably through the ALLOCATION query with our own GstGLMeta.
Comment 4 Matthew Waters (ystreet00) 2014-02-19 09:14:27 UTC
Created attachment 269675 [details] [review]
context: add support for wrapping external contexts
Comment 5 Sebastian Dröge (slomo) 2014-02-19 20:44:11 UTC
Comment on attachment 269675 [details] [review]
context: add support for wrapping external contexts

Generally looks good to me but I think it would be nice to have an example for the usage :)

Also the "wrapped" boolean in the struct is not really needed, you could just check GST_IS_GL_CONTEXT_WRAPPED().

Also I think it should be GstGLWrappedContext ;)
Comment 6 Matthew Waters (ystreet00) 2014-02-20 06:30:11 UTC
Created attachment 269768 [details] [review]
display: add display type enum
Comment 7 Matthew Waters (ystreet00) 2014-02-20 06:30:40 UTC
Created attachment 269769 [details] [review]
x11: add display subclass
Comment 8 Matthew Waters (ystreet00) 2014-02-20 06:34:31 UTC
Created attachment 269770 [details] [review]
context: add support for wrapping external contexts

(In reply to comment #5)
> (From update of attachment 269675 [details] [review])
> Generally looks good to me but I think it would be nice to have an example for
> the usage :)

Test now included.

> Also the "wrapped" boolean in the struct is not really needed, you could just
> check GST_IS_GL_CONTEXT_WRAPPED().
>
> Also I think it should be GstGLWrappedContext ;)

sedded :). It reads slightly better.
Comment 9 Matthew Waters (ystreet00) 2014-02-23 03:24:48 UTC
I had a quick look at using cluttersink and found that cogl/clutter does not have a mechanism for extracting the gl context/platform.  We could do some *GetCurrentContext hack to work around that however I don't think we have the guarentee that clutter is using the gstreamer thread as it's gl thread.  I would expect breakage when cluttersink is being used in a clutter application and thus the gstreamer thread is different to the gl thread.
Comment 10 Matthew Waters (ystreet00) 2014-02-23 03:50:00 UTC
See cogl bug I filed https://bugzilla.gnome.org/show_bug.cgi?id=724992
Comment 11 Julien Isorce 2014-02-23 05:42:22 UTC
Well done for the already 3 commited patchs. For the display part it will be also useful for https://bugzilla.gnome.org/show_bug.cgi?id=709747#c10. I will try to split https://bug709747.bugzilla-attachments.gnome.org/attachment.cgi?id=266903.

(In reply to comment #9)
Have a look at the first attached file (external_sink_wrap_gl_callback / "share").
(It assumes one of the clutter gl threads (and I guess there is only one) is running in the main GMainContext but we could add this callback in the right GMainContext if that was not the case.)

Upstream gl elements will response to [GST_GL_TYPE_CONTEXT, sink->context, "share", G_TYPE_BOOLEAN, TRUE] the same way as for https://bugzilla.gnome.org/show_bug.cgi?id=704809. Except here cluttersink explicitly tells downstream it has to share.
Comment 12 Matthew Waters (ystreet00) 2014-02-23 06:04:33 UTC
(In reply to comment #11)
> Well done for the already 3 commited patchs. For the display part it will be
> also useful for https://bugzilla.gnome.org/show_bug.cgi?id=709747#c10. I will
> try to split
> https://bug709747.bugzilla-attachments.gnome.org/attachment.cgi?id=266903.

:)

> (In reply to comment #9)
> Have a look at the first attached file (external_sink_wrap_gl_callback /
> "share").
> (It assumes one of the clutter gl threads (and I guess there is only one) is
> running in the main GMainContext but we could add this callback in the right
> GMainContext if that was not the case.)

It's not guaranteed that there will only ever be one clutter GMainContext.  Clutter has expressed interest in going multithreaded although that doesn't seem to be materialising anytime soon.  Also, there is the policy within GStreamer that we do not rely on there ever being a GMainContext at any point in time.

> Upstream gl elements will response to [GST_GL_TYPE_CONTEXT, sink->context,
> "share", G_TYPE_BOOLEAN, TRUE] the same way as for
> https://bugzilla.gnome.org/show_bug.cgi?id=704809. Except here cluttersink
> explicitly tells downstream it has to share.

The problem here is getting the gl context out of cluttersink cleanly [1] as cogl does not directly support what we are trying to do (yet? :)).  Everything else should work from our point of view.

[1] There are the {egl,glX}GetCurrentContext() family of functions however we cannot really know which to call.  I would prefer to define some minimal integration requirements rather than relying on some heuristics.
Comment 13 Julien Isorce 2014-02-23 09:40:04 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > Well done for the already 3 commited patchs. For the display part it will be
> > also useful for https://bugzilla.gnome.org/show_bug.cgi?id=709747#c10. I will
> > try to split
> > https://bug709747.bugzilla-attachments.gnome.org/attachment.cgi?id=266903.
> 
> :)
> 
> > (In reply to comment #9)
> > Have a look at the first attached file (external_sink_wrap_gl_callback /
> > "share").
> > (It assumes one of the clutter gl threads (and I guess there is only one) is
> > running in the main GMainContext but we could add this callback in the right
> > GMainContext if that was not the case.)
> 
> It's not guaranteed that there will only ever be one clutter GMainContext. 
> Clutter has expressed interest in going multithreaded although that doesn't
> seem to be materialising anytime soon.  Also, there is the policy within
> GStreamer that we do not rely on there ever being a GMainContext at any point
> in time.

Ah right for cluttersink, I only did some experiments with webkitgtk :)
Then for clutter there is clutter_threads_add_timeout_full. I have not looked at cluttersink though.

> 
> > Upstream gl elements will response to [GST_GL_TYPE_CONTEXT, sink->context,
> > "share", G_TYPE_BOOLEAN, TRUE] the same way as for
> > https://bugzilla.gnome.org/show_bug.cgi?id=704809. Except here cluttersink
> > explicitly tells upstream it has to share.

> 
> The problem here is getting the gl context out of cluttersink cleanly [1] as
> cogl does not directly support what we are trying to do (yet? :)).  Everything
> else should work from our point of view.
> 
> [1] There are the {egl,glX}GetCurrentContext() family of functions however we
> cannot really know which to call.  I would prefer to define some minimal
> integration requirements rather than relying on some heuristics.

You are entirely right. It would be preferable that the external API already exposes that. If not we could suggest them. In the mean time and for more external API coverage we could wrap those {egl,glX,...}GetCurrentContext() in gstgl API. (gst_gl_context_new_wrapped_from_current() ?)

To go further we could provide some base functions that the external sink could directly uses. (gst_gl_context_new_wrapped_from_g_main_context which would setup and call g_timeout_add_full for you, with a callback that calls gst_gl_context_new_wrapped_from_current. Or gst_gl_context_new_wrapped_from_clutter_thread which would setup and call clutter_threads_add_timeout_full for you, with a callback that calls gst_gl_context_new_wrapped_from_current)
Comment 14 Julien Isorce 2014-10-30 21:21:08 UTC
Just for the record, Matthews continued to improve this support with  http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=81ceca1aea35cfebf2d4b9b994a6c1ec20adf5b4 . Thx!


Matthew have you then try to use all of this in cluttersink to have "gst-launch-1.0 videotestsrc ! gleffects ! cluttersink" working with texture sharing between gleffects and cluttersink ?

Would be also cool to use gstlgl in qtglvideosink !
Comment 15 Matthew Waters (ystreet00) 2014-10-31 01:02:37 UTC
(In reply to comment #14)
> Matthew have you then try to use all of this in cluttersink to have
> "gst-launch-1.0 videotestsrc ! gleffects ! cluttersink" working with texture
> sharing between gleffects and cluttersink ?

Apparently lubosz got it working using some variant of https://github.com/ystreet/clutter-gst 
http://www.youtube.com/watch?v=b8_CkIpVY_w

There's also https://github.com/ystreet/gtkgst which integrates with Gtk+'s new gl support.

I'm pretty sure we have full support for the context sharing case iff the surrounding elements provide the context information:
http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/gstglfilter.c#n937
Comment 16 Julien Isorce 2014-10-31 08:02:45 UTC
That's great !

In clutter-gst I only see support for glx with gstgl (https://github.com/ystreet/clutter-gst/blob/wip-context-share/clutter-gst/clutter-gst-video-sink.c#L2090).
Maybe good to have egl/gles2 support in clutter-gst for gstgl as well.

I'll keep the bug open until something goes upstream. We need to add this in qtglvideosink and sdlvideosink and webkitvideosink.
Comment 17 Philippe Normand 2014-11-10 12:06:10 UTC
WebKit bug: https://bugs.webkit.org/show_bug.cgi?id=138562

I intend to work on it this week :)
Comment 18 Julien Isorce 2015-07-15 21:37:23 UTC
We have now at least upstream webkit and gstgtkglsink that use GstGL API to share resources between gl contexts. So I think we can close this bug.

If someone is interested to do it for qtglvideosink, patches are welcome. (The advantage is that would automatically enable zero-copy with HW decoder)
Comment 19 Matthew Waters (ystreet00) 2015-07-16 04:34:08 UTC
There's a qt5 qml sink implementation as well here:
http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/ext/qt

Closing as the current API seems to be serve us pretty well :)