In the actual gst-plugins-gl 1.0 can we can create a gl context without a window ? If no then we have to add this feature if it's really possible. I mean can we share 2 gl contexts if one is setup with off-screen and the other with on-screen surface ?

Anyway I'm wondering if we could use the streaming thread as the gl thread (except for the final drawing thing done by glimagesink). See solution 2.

For example in videotestsrc ! gleffect ! queue ! glfiltercube ! glimagesink there are 2 streaming threads, the first one (called A) involved in videotestsrc ! gleffect, and the second (called B) involved in queue ! glfiltercube ! glimagesink


1) The first improvement would be to create one GstGLContext A for the streaming thread A, shared with the GstGLContext B for the streaming thread B.
In this case it would have 2 streamings thread and 2 GL threads (1 GL thread per GstGLContext)

2) The second improvement would be to use the streaming thread as a GL thread. In our example, streaming thread A would be the GL thread associated with GstGLContext A. Streaming thread B would be the GL thread associated with GstGLContext B. Then we would still need another GL Thread with GstGLContext created by glimagesink for drawing purpose.

With this second solution we would need the ability to be able to create a GstGLContext without creating a new thread and without running a loop into it. Just call make current and other gl calls directly in that thread :)


** With solution 1) and 2) we would need to know in which streaming thread we are in GST_QUERY_CONTEXT. If we are in the same then we use it directly but if we are in an other streaming thread then we share. How to do that ? Improve GstContext ? Is there a GST_QUERY_STREAMING_STATUS ? I only saw examples with messages.

With solution 2) we would also need to be able to ask for calling a callback into the streaming thread. I mean if we need to ask for calling a GL function in the streaming thread from set_caps (or get_caps or any other function of an gl element) . Not sure if it is possible as the GstTask does not run g_main_loop so we cannot use g_timeout_callback for example.

An other question, in GstBaseTransform, which virtual methods are "100% garanti" to be called in the streaming thread the element belong  ?http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer-libs/html/GstBaseTransform.html
Only (*transform) ?

(In reply to comment #1)
> In the actual gst-plugins-gl 1.0 can we can create a gl context without a
> window ? If no then we have to add this feature if it's really possible. I mean
> can we share 2 gl contexts if one is setup with off-screen and the other with
> on-screen surface ?

In theory yes, it probably depends a bit on the backend. For WGL you can only create invisible window surfaces, not real offscreen surfaces AFAIK.

Sharing between contexts should work in theory, Matthew checked that. It has to be implemented though ;) And there should also be a fallback path that downloads&uploads when moving between contexts if nothing more optimal is possible.

> Anyway I'm wondering if we could use the streaming thread as the gl thread
> (except for the final drawing thing done by glimagesink). See solution 2.
> 
> For example in videotestsrc ! gleffect ! queue ! glfiltercube ! glimagesink
> there are 2 streaming threads, the first one (called A) involved in
> videotestsrc ! gleffect, and the second (called B) involved in queue !
> glfiltercube ! glimagesink
> 
> 
> 1) The first improvement would be to create one GstGLContext A for the
> streaming thread A, shared with the GstGLContext B for the streaming thread B.
> In this case it would have 2 streamings thread and 2 GL threads (1 GL thread
> per GstGLContext)
> 
> 2) The second improvement would be to use the streaming thread as a GL thread.
> In our example, streaming thread A would be the GL thread associated with
> GstGLContext A. Streaming thread B would be the GL thread associated with
> GstGLContext B. Then we would still need another GL Thread with GstGLContext
> created by glimagesink for drawing purpose.
> 
> With this second solution we would need the ability to be able to create a
> GstGLContext without creating a new thread and without running a loop into it.
> Just call make current and other gl calls directly in that thread :)

Sounds like a plan, but 1) as a first step would already make me happy enough :)

> ** With solution 1) and 2) we would need to know in which streaming thread we
> are in GST_QUERY_CONTEXT. If we are in the same then we use it directly but if
> we are in an other streaming thread then we share. How to do that ? Improve
> GstContext ? Is there a GST_QUERY_STREAMING_STATUS ? I only saw examples with
> messages.

There's nothing yet for that, we need to add API for that. I think it should be part of the ALLOCATION query (as said in the other bug, GstContext should here probably only be used to share GstGLDisplay, not GstGLContext and thus not the threads).

> With solution 2) we would also need to be able to ask for calling a callback
> into the streaming thread. I mean if we need to ask for calling a GL function
> in the streaming thread from set_caps (or get_caps or any other function of an
> gl element) . Not sure if it is possible as the GstTask does not run
> g_main_loop so we cannot use g_timeout_callback for example.

Problem with that is that it's not even guaranteed that the streaming thread is ever active again. I'm not sure how feasible this is going to be.

> An other question, in GstBaseTransform, which virtual methods are "100%
> garanti" to be called in the streaming thread the element belong 
> ?http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer-libs/html/GstBaseTransform.html
> Only (*transform) ?

And set_caps at least, and the event ones for serialized events.

(In reply to comment #2)
> (In reply to comment #1)
> > In the actual gst-plugins-gl 1.0 can we can create a gl context without a
> > window ? If no then we have to add this feature if it's really possible. I mean
> > can we share 2 gl contexts if one is setup with off-screen and the other with
> > on-screen surface ?
> 
> In theory yes, it probably depends a bit on the backend. For WGL you can only
> create invisible window surfaces, not real offscreen surfaces AFAIK.

Ok this is what I remind now :)

> 
> Sharing between contexts should work in theory, 

Could you clarify ? I mean what should work compared to gst-gl-0.10 ?

> Matthew checked that. It has to
> be implemented though ;) And there should also be a fallback path that
> downloads&uploads when moving between contexts if nothing more optimal is
> possible.
> 

> > ** With solution 1) and 2) we would need to know in which streaming thread we
> > are in GST_QUERY_CONTEXT. If we are in the same then we use it directly but if
> > we are in an other streaming thread then we share. How to do that ? Improve
> > GstContext ? Is there a GST_QUERY_STREAMING_STATUS ? I only saw examples with
> > messages.
> 
> There's nothing yet for that, we need to add API for that. I think it should be
> part of the ALLOCATION query (as said in the other bug, GstContext should here
> probably only be used to share GstGLDisplay, not GstGLContext and thus not the
> threads).

I'm not convinced :)
I think GstGLDisplay should be removed, and merged into GstGLContext. I mean in the gstgl history gstgldisplay should had been called gstglcontext from the beginning.

Currently in HEAD what is the goal of GstGLDisplay ? Almost just be able to call gst_gl_display_thread_add. Why not just move it to GstGLContext ?

Also I do not see a use case where we would need to use a GstGLDisplay without a GstGLContext.

> 
> > With solution 2) we would also need to be able to ask for calling a callback
> > into the streaming thread. I mean if we need to ask for calling a GL function
> > in the streaming thread from set_caps (or get_caps or any other function of an
> > gl element) . Not sure if it is possible as the GstTask does not run
> > g_main_loop so we cannot use g_timeout_callback for example.
> 
> Problem with that is that it's not even guaranteed that the streaming thread is
> ever active again. I'm not sure how feasible this is going to be.

Ok so we could combine 1) and 2),
I mean we could keep 1) (when it's done) and then be able to use the streaming thread as a GL thread only for (*transform). During init and conf it does not matter to use a dedicated GL thread, but in (*transform) this is where we need to be fast :)

It implies de have one more GL context and I think on some platform we have a limited number of gl context ?

Well, for the "every day" pipeline videotestsrc ! gleffects ! glimagesink
it would implies to create 2 GstGLContext: 1 which use the streaming thread, 1 which create a GL thread

For videotestsrc ! gleffects ! queue ! glfiltercube ! glimagesink  : 4 GstGLContext, 2 thats use a streaming thread, 2 that create a gl thread.

(In reply to comment #3)

> > 
> > Sharing between contexts should work in theory, 
> 
> Could you clarify ? I mean what should work compared to gst-gl-0.10 ?

What do you mean? If you have a pipeline where upstream uses a different context than downstream, sharing of buffers between them is supposed to work.

> > > ** With solution 1) and 2) we would need to know in which streaming thread we
> > > are in GST_QUERY_CONTEXT. If we are in the same then we use it directly but if
> > > we are in an other streaming thread then we share. How to do that ? Improve
> > > GstContext ? Is there a GST_QUERY_STREAMING_STATUS ? I only saw examples with
> > > messages.
> > 
> > There's nothing yet for that, we need to add API for that. I think it should be
> > part of the ALLOCATION query (as said in the other bug, GstContext should here
> > probably only be used to share GstGLDisplay, not GstGLContext and thus not the
> > threads).
> 
> I'm not convinced :)
> I think GstGLDisplay should be removed, and merged into GstGLContext. I mean in
> the gstgl history gstgldisplay should had been called gstglcontext from the
> beginning.
> 
> Currently in HEAD what is the goal of GstGLDisplay ? Almost just be able to
> call gst_gl_display_thread_add. Why not just move it to GstGLContext ?
> 
> Also I do not see a use case where we would need to use a GstGLDisplay without
> a GstGLContext.

It's work-in-progress, see the related bug reports. GstGLDisplay will become smaller and will just share whatever is required to create a context (and thus a surface). You want to share that between elements because otherwise they most likely can't interoperate in a fast way.

(In reply to comment #4)
> (In reply to comment #3)
> 
> > > 
> > > Sharing between contexts should work in theory, 
> > 
> > Could you clarify ? I mean what should work compared to gst-gl-0.10 ?
> 
> What do you mean? If you have a pipeline where upstream uses a different
> context than downstream, sharing of buffers between them is supposed to work.
> 

oh this is already working since gst-gl-0.10, see glmosaic,  it uses 7 gl contexts shared together if I remember right. (1 for each face of the cube et 1 for the mixed result)

See also cluttershare and sdlshare examples.

What I see in gst-gl-1.0 is it seems that we still use unvisible window (even on glx I think) for the 6 others contexts in glmosaic example.

I mean a gl context is created with unvisible window by default, and then only made visible when calling gst_gl_window_x11_draw.

So what is working right now is on-screen gl context sharing (visible and/or unvisible)

So I'm wondering if real off-screen gl context can be shared with on-screen gl context.

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > 
> > > > 
> > > > Sharing between contexts should work in theory, 
> > > 
> > > Could you clarify ? I mean what should work compared to gst-gl-0.10 ?
> > 
> > What do you mean? If you have a pipeline where upstream uses a different
> > context than downstream, sharing of buffers between them is supposed to work.
> > 
> 
> oh this is already working since gst-gl-0.10, see glmosaic,  it uses 7 gl
> contexts shared together if I remember right. (1 for each face of the cube et 1
> for the mixed result)
> 
> See also cluttershare and sdlshare examples.

That case might already work, see Matthews latest changes with the external context.

> What I see in gst-gl-1.0 is it seems that we still use unvisible window (even
> on glx I think) for the 6 others contexts in glmosaic example.
> 
> I mean a gl context is created with unvisible window by default, and then only
> made visible when calling gst_gl_window_x11_draw.
> 
> So what is working right now is on-screen gl context sharing (visible and/or
> unvisible)
> 
> So I'm wondering if real off-screen gl context can be shared with on-screen gl
> context.

In theory yes, at least with EGL.

(In reply to comment #3)

Julien said:
> > 
> > > With solution 2) we would also need to be able to ask for calling a callback
> > > into the streaming thread. I mean if we need to ask for calling a GL function
> > > in the streaming thread from set_caps (or get_caps or any other function of an
> > > gl element) . Not sure if it is possible as the GstTask does not run
> > > g_main_loop so we cannot use g_timeout_callback for example.
> > 

Sebastian said:
> > Problem with that is that it's not even guaranteed that the streaming thread is
> > ever active again. I'm not sure how feasible this is going to be.
> 

Julien said:
> Ok so we could combine 1) and 2),
> I mean we could keep 1) (when it's done) and then be able to use the streaming
> thread as a GL thread only for (*transform). During init and conf it does not
> matter to use a dedicated GL thread, but in (*transform) this is where we need
> to be fast :)
> 
> It implies de have one more GL context and I think on some platform we have a
> limited number of gl context ?
> 
> Well, for the "every day" pipeline videotestsrc ! gleffects ! glimagesink
> it would implies to create 2 GstGLContext: 1 which use the streaming thread, 1
> which create a GL thread
> 
> For videotestsrc ! gleffects ! queue ! glfiltercube ! glimagesink  : 4
> GstGLContext, 2 thats use a streaming thread, 2 that create a gl thread.


I think I have an other idea; instead of having a dedicated GL thread for each streaming thread,  just call "make_current" when we need to use a GL function. When changing element states, the cost of switching context does not matter. In  
(*transform) function, where we need to be fast, then calling "make_current" would have no effect because it's already current (except for the first time, but I think this case would happen when going to STATE_PAUSED)

So to resume for pipeline videotestsrc ! gleffects ! queue ! glfiltercube ! glimagesink:  2 GstGLContexts that are switchable (one "associated" with each streaming thread, but can be set current in other thread if needed (*get_caps), anything else)function), + 1 GstGLContext that creates a GL thread associated with glimageinsk for drawing and windowing purpose.
The 3 contexts would be shared together.

Also when initializing GstGLContexts, I mean when using GST_QUERY_CONTEXT / GST_QUERY_ALLOCATION, would it be possible to know we are using another streaming thread by looking at if the src pad is creating a GstTask ?

For example when a query goes from queue_pad_src to queue_pad_sink it should have a way to know that queue_pad_src uses a GstTask. Maybe when calling gst_pad_start_task (a_pad, ) then a_pad should be marked as handling a new streaming thread.

(In reply to comment #7)

> I think I have an other idea; instead of having a dedicated GL thread for each
> streaming thread,  just call "make_current" when we need to use a GL function.
> When changing element states, the cost of switching context does not matter. In 
> (*transform) function, where we need to be fast, then calling "make_current"
> would have no effect because it's already current (except for the first time,
> but I think this case would happen when going to STATE_PAUSED)
> 
> So to resume for pipeline videotestsrc ! gleffects ! queue ! glfiltercube !
> glimagesink:  2 GstGLContexts that are switchable (one "associated" with each
> streaming thread, but can be set current in other thread if needed (*get_caps),
> anything else)function), + 1 GstGLContext that creates a GL thread associated
> with glimageinsk for drawing and windowing purpose.
> The 3 contexts would be shared together.

You mean one context for gleffects, one for glfiltercube!glimagesink and one for glimagesink rendering? Or I misunderstand completely :)

> Also when initializing GstGLContexts, I mean when using GST_QUERY_CONTEXT /
> GST_QUERY_ALLOCATION, would it be possible to know we are using another
> streaming thread by looking at if the src pad is creating a GstTask ?
> 
> For example when a query goes from queue_pad_src to queue_pad_sink it should
> have a way to know that queue_pad_src uses a GstTask. Maybe when calling
> gst_pad_start_task (a_pad, ) then a_pad should be marked as handling a new
> streaming thread.

Unfortunately not currently, the pad task could also be created after the query in theory. The element owning the pad would know though.

(In reply to comment #8)
> You mean one context for gleffects, one for glfiltercube!glimagesink and one
> for glimagesink rendering?

Correct :)

Yeah then initial reason of having a dedicated GL thread was to avoid effectively switching  GL context from threads for each frame. And because of we needed a thread for window event handling, why not using this thread as the GL thread. So that's the story.

Now if we can know which streaming thread a glelement belongs to then we could use glcontext sharing.

For example in pipeline videotestsrc ! gleffects ! queue ! glfiltercube ! glimagesink, 
gleffects would create a GstGLContext without a dedicated GL thread. When initializing gleffects then just call "gl_context_make_current" when we need to use gl functions. I think the cost of the context switch would not be a pb during init and change state.
Once in STATE_PLAYING, gleffects::(*transform) will no cause any switch at each frame.

Not sure if I'm clear :)

> > Also when initializing GstGLContexts, I mean when using GST_QUERY_CONTEXT /
> > GST_QUERY_ALLOCATION, would it be possible to know we are using another
> > streaming thread by looking at if the src pad is creating a GstTask ?
> > 
> > For example when a query goes from queue_pad_src to queue_pad_sink it should
> > have a way to know that queue_pad_src uses a GstTask. Maybe when calling
> > gst_pad_start_task (a_pad, ) then a_pad should be marked as handling a new
> > streaming thread.
> 
> Unfortunately not currently, the pad task could also be created after the query
> in theory. The element owning the pad would know though.

Maybe the pad should be required to set "able_to_start_task" when creating the pad. Then the marker would be set before the query ?

I think there is something to do with SERIALIZED queries.

We could do that in a generic way, so only based on if a query is serialized or not.

But concretely in "propose_allocation" a field "thread_id" (= g_thread_self () ) would be added so that in "decide_allocation" we would know if the streaming thread of downstream elements will be difference than here. So if different we would create another one a share, or something like that.

The approach is based on the fact that we know what would the streaming thread (value returned by g_thread_self () )  in decide_allocation and propose_allocation because the allocation query is serialized.

I means that when calling g_thread_self() in GstBaseTransform::decide_allocation, and GstBaseTransform::propose it would return the same value as if you call it in GstBaseTransform::transform.

And for sinks, calling g_thread_self() from GstBaseSink::propose_allocation returns the same value as the one returned when you call it from GstBaseSink::show_frame

I'll try to do a first draft if you think the approach make senses. (only a draft in gstreamer core api, first)

Ah, good idea. That should indeed work

Created attachment 265391 [details] [review]
query: new API to count streaming threads

Created attachment 265392 [details] [review]
pad: let any serialized query knows the current streaming thread

Created attachment 265393 [details] [review]
check: validate new streaming thread API for queries

Created attachment 265481 [details] [review]
egl: use one GstGLContext per streaming thread

No need to review this one, it only works on egl as we have not yet implemented offscreen surfaces on other platforms. But that gives the idea about how we will use the new "gst_query_get_n_streaming_threads" API

Review of attachment 265391 [details] [review]:

::: gst/gstquery.c
@@ +2610,3 @@
+ */
+gboolean
+gst_query_add_streaming_thread (GstQuery * query)

Maybe a different name to imply that it does not take a parameter for the thread but adds the *current* thread

@@ +2626,3 @@
+  array =
+      ensure_array (structure, GST_QUARK (STREAMING_THREAD), sizeof (gpointer),
+      (GDestroyNotify) NULL);

You probably want to use g_thread_ref/unref here

Please discuss in #gstreamer if this is a good idea, especially with Wim :)

Created attachment 270356 [details]
initial discussion with Wim

(In reply to comment #17)
> Please discuss in #gstreamer if this is a good idea, especially with Wim :)

If I just focus on the main idea which is to know if there is a thread boundary or not between producer and consumer (I mean no matter how many threads there are between them) then I fall into the following:

If I replace
gboolean gst_query_add_streaming_thread (GstQuery * query) by gboolean
gst_query_check_streaming_thread (GstQuery * query) which would just call g_thread_self and keep the last value. So that for A ! queue ! B , this https://bug704809.bugzilla-attachments.gnome.org/attachment.cgi?id=265392 will call it twice and see that the thread is not the same on the second call.

But if I have A ! queue ! B ! C , then it would be the same when reaching C.
So the information we would keep is if the thread has ever changed when the serialized query transited from A to C. I mean the thread just needs to change one time. Then I could replace gst_query_get_n_streaming_threads by gst_query_has_thread_boundary.

Doing that is ok for my need but then I do not see why not moving to the first solution gst_query_add_current_streaming_thread / gst_query_get_n_streaming_threads as we would need to retain the return value of g_thread_self at least one time in both cases.

Also I agree to do the changes pointed in comment #16.

Also see the unit test https://bug704809.bugzilla-attachments.gnome.org/attachment.cgi?id=265393 it can gives some ideas.

We really need to have this new API to go further with gst-plugins-gl.

ping ?

Created attachment 278933 [details] [review]
gl: use one GstGLContext per streaming thread

Let's discuss about this new solution that does not add a new API in gst-core. The thread boundary is done through the allocation query. Also fallback to un-visible window for the surface when we have not added support for pBuffer-surface or surface-less for the given platform yet (ex: glx).

There are still a major issue:

It's required to unbound the gl context to the previous thread before to make it current in another thread. For now I unbound the gl context from the streaming thread using the serialized GST_EVENT_EOS. I also do the assumption that all gl calls from state NULL to PLAYING are done in the streaming thread.

Created attachment 278934 [details] [review]
gl: avoid to unbound the gl context when playing

Try to handle the case:
videotestsrc ! queue ! gleffects ! fakesink
where there is 1 GstGLContext common to 2 GstGLBufferPool, used in 2 different streaming thread.

I just realize that we should actually ensure 1 GstGLContext per streaming thread in this case too :)

Review of attachment 278933 [details] [review]:

This seems to combine three independent changes that should be split into separate patches
1. offscreen signalling
2. changes to the offscreen EGL code
3. separate GstGLContext per streaming thread

There should be no MakeCurrent dances with GST_EVENT_EOS required to get this to work properly, each thread has their own context (with optional gl thread) and each element owns a ref on a context that is current in its thread.  When the element is done with the context in the streaming thread, it unrefs the context.

The other concern that I currently have with this is the integration with other non libgstgl elements and even signalling between libgstgl aware elements.  e.g. the thread parameter passed through the allocation query currently describes the current streaming thread rather than the actual gl thread used.  

The other approach is to make the streaming thread == gl thread.  This has the advantage of performance by removing the current function marshalling for all GL functions.  The downside is that if you have a pipeline like so:
glfilter ! non libgstgl aware gl element ! glfilter 
all attempting to share the same streaming thread, then you need to figure out some way to share gl state or force a new streaming thread at each library boundary in the pipeline.  Or we just ignore this compatability problem.

Review of attachment 278934 [details] [review]:

MakeCurrent is expensive pretty much everywhere.

This probably would be better solved using thread local storage by storing the GLContext's hanging off the end of a gl element thread.  The only objects that have the potential for crossing thread boundaries are GLMemory which could be handled by GLBufferPool.

(In reply to comment #23)
> The other approach is to make the streaming thread == gl thread. 

Sorry I do not understand why this would be an other approach :) This is what I am trying to achieve with this bug.  (and more as this is for all streaming threads). Maybe I am missing something in your remark ? :)

I think I was close last time I tried but I get annoyed by the major issue described in the end of #23. But I now think it was a bug in the gl driver I used at that moment. So it should simply the patch.
I also should split it as Matthew suggested.

(In reply to comment #25)
> (In reply to comment #23)
> > The other approach is to make the streaming thread == gl thread. 
> 
> Sorry I do not understand why this would be an other approach :) This is what I
> am trying to achieve with this bug.  (and more as this is for all streaming
> threads). Maybe I am missing something in your remark ? :)
> 
> I think I was close last time I tried but I get annoyed by the major issue
> described in the end of #23. But I now think it was a bug in the gl driver I
> used at that moment. So it should simply the patch.
> I also should split it as Matthew suggested.

I mean the removal of a function marshalling from GstGLContext/GstGLWindow (except for *maybe* helper for GMainContext) so that gst_gl_context_thread_add and gst_gl_window_send_message* are not needed at least for every frame.

I think this is done right ?

No, and probably won't ever be unfortunately.