-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Thread affinity for gstreamer-gl contexts? #309
Comments
It seems strange that this has not appeared to be a problem on Linux, given this similar code: media/backends/gstreamer/render-unix/lib.rs Line 165 in f9aed4e
|
Indeed, I'm hoping to try this on a linux box when I get home. It would be that the threading requirements are different between GL/EGL? |
The threading requirements at https://gstreamer.freedesktop.org/documentation/gl/gstglcontext.html say "As a result of the limitation on OpenGL context, this object is not thread safe unless specified and must only be activated in a single thread." I think what we're doing is creating it in the script thread, then activating it in the gstreamer render thread, which according to ^ is safe, but apparently not. |
void
gst_gl_context_thread_add (GstGLContext * context,
GstGLContextThreadFunc func, gpointer data)
{
GstGLWindow *window;
RunGenericData rdata;
g_return_if_fail (GST_IS_GL_CONTEXT (context));
g_return_if_fail (func != NULL);
if (GST_IS_GL_WRAPPED_CONTEXT (context))
g_return_if_fail (context->priv->active_thread == g_thread_self ()); That last assertion is what's failing, and is due to the test |
As far so know the problem is not in which thread you create the GST gl context, but where you activate it. |
@xclaesse yes, but it looks like you can only use |
OK, if the problem is that we're using a wrapped context, perhaps the thing to do is create an unwrapped context which shares with the wrapped context, and use that as the context for the player? |
As @xclaesse said, the problem is in which thread the wrapped gl context is activated: https://github.com/servo/media/pull/308/files#diff-c7f05fb6a8c34d92da3ac21df409cee9R106 We should look por a place where to activate it, but inside media-servo. The problem is, if we don't activate it before the gst pipeline uses it, the pipeline will probably fail (since gstgl cannot access the gl vtable) |
So replacing that wrapped GL context with an unwrapped context makes that problem go away: let wrapped_context = unsafe {
gst_gl::GLContext::new_wrapped(
&display,
context,
gst_gl::GLPlatform::EGL,
gl_api,
)
};
let unwrapped_context = gst_gl::GLContext::new(&display);
let _ = unwrapped_context.create(wrapped_context.as_ref());
(Some(unwrapped_context), Some(display)) but then this produces an error:
My guess is there are some points where the GL context known in the script thread is required to be the same GL context as the one used by the GL render thread? |
Doing a search for media/backends/gstreamer/render-unix/lib.rs Line 131 in 6e864ed
|
android backend also activates the context on the scripth thread: you can try delete that block of code |
Oh good point, the android back end is in a PR, not checked in yet. I'm going to see if I can replicate the issue on a linux box. |
OK, I'm failing to work out why this works on unix but not magicleap. I understand why it's failing on magicleap, it's the "why does it work on unix" part that I'm struggling with. We create a wrapped context, but since it's a wrapped context we can't call |
OK, setting a couple of breakpoints and running the unix program in lldb produces:
that is the context that is activated in the script thread is not the one being activated in the render thread and having |
Sigh, that would be too easy wouldn't it? The magicleap build has no symbol
|
|
@xclaesse the build is the one produced by https://github.com/servo/servo/tree/master/support/magicleap/gstreamer, which is I believe is optimized but not stripped. |
Spent all of today working out how to get gdb to play ball with gstreamer on magicleap. servo/servo@b711d12 |
Argh, and after all that, yes I can find where the error is happening, but no I'm not sure it's any more use because the backtrace is still garbled:
There is a bit of a clue, in that Servo happily runs along with context |
Confirming that on Linux there's only one |
The fact that it doesn't have the function symbol for the caller of gst_gl_context_thread_add() indicates it's probably called from Servo and not from GStreamer, otherwise it would have the symbol like all others. Could you grep the code base to see if that's called anywhere? Or try teaching gdb where to look for Servo symbols too. You could add breakpoint in gst_gl_context_new() and gst_gl_context_new() and print the returned pointer, so you could have an indication who created that context. |
It's a bit annoying that there's no debug symbol for the caller. I'll have a look to see if I can find the context creator. |
OK, here's the trace featuring the calls to
The context that is working came from I'm back to not understanding how this is working in Linux, since it's essentially the same code. |
Ah, a different error, this one looking more sensible...
so now I need to work out why the display provided by magicleap (which is display 1) isn't making it to gstreamer. |
And that would be because https://github.com/servo/servo/blob/540a73ed2970948e916db0c53880ca20cb5083e9/ports/libmlservo/src/lib.rs#L176-L177 doesn't pass along the display and context it was given as arguments. |
OK, back to the original problem...
so the problem is
if self.gst_context.lock().unwrap().is_some() {
if let Some(sync_meta) = frame.buffer().get_meta::<gst_gl::GLSyncMeta>() {
sync_meta.wait(&self.app_context);
}
} since void
gst_gl_sync_meta_wait (GstGLSyncMeta * sync_meta, GstGLContext * context)
{
if (sync_meta->wait)
sync_meta->wait (sync_meta, context);
else
gst_gl_context_thread_add (context,
(GstGLContextThreadFunc) _wait, sync_meta);
} and oh dear |
You're waiting on the wrong context. See here how to get GStreamer's context: https://gitlab.freedesktop.org/xclaesse/gstreamer_demo/blob/master/VideoScene.cpp#L296 |
OK, a workaround is #311 but I'm not convinced it's the Right Thing To Do, since it's only sync'ing the render thread, not the main thread, but since the main thread is pretty oblivious to gstreamer I don't see how to achieve that. Should the media stack have an API similar to webrender's for sync'ing textures? |
@jdm raises a good point over at https://mozilla.logbot.info/servo/20190917#c16622943
|
Doing this implies to solve servo/servo#24211 too |
Building magicleap servo with the render-android PR (#308) works, but running it and viewing video produces errors:
According to @xclaesse this is caused by the gstreamer-gl context being created in one thread and used in another. The code that creates the context is https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/blob/master/gstreamer-gl/src/gl_context.rs#L25-30
which is called at https://github.com/servo/media/pull/308/files#diff-c7f05fb6a8c34d92da3ac21df409cee9R88-R93
The problem is that that initialization code is called from the script thread, as you can see from this backtrace:
So we have the gstreamer-gl context being created in the script thread, but then used in the gstreamer render thread.
The text was updated successfully, but these errors were encountered: