diff --git a/ext/gl/gstglbasemixer.c b/ext/gl/gstglbasemixer.c index fc61e84b35..7459572043 100644 --- a/ext/gl/gstglbasemixer.c +++ b/ext/gl/gstglbasemixer.c @@ -99,13 +99,35 @@ gst_gl_base_mixer_pad_set_property (GObject * object, guint prop_id, } } - static gboolean -_find_local_gl_context (GstGLBaseMixer * mix) +_find_local_gl_context_unlocked (GstGLBaseMixer * mix) { - GstGLContext *context = mix->context; + GstGLContext *context, *prev_context; + gboolean ret; + + if (mix->context && mix->context->display == mix->display) + return TRUE; + + context = prev_context = mix->context; + g_rec_mutex_unlock (&mix->priv->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SRC, &context); + g_rec_mutex_lock (&mix->priv->context_lock); + if (ret) { + if (mix->context != prev_context) { + /* we need to recheck everything since we dropped the lock and the + * context has changed */ + if (mix->context && mix->context->display == mix->display) { + if (context != mix->context) + gst_clear_object (&context); + return TRUE; + } + } - if (gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SRC, &context)) { if (context->display == mix->display) { mix->context = context; return TRUE; @@ -113,7 +135,26 @@ _find_local_gl_context (GstGLBaseMixer * mix) if (context != mix->context) gst_clear_object (&context); } - if (gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SINK, &context)) { + + context = prev_context = mix->context; + g_rec_mutex_unlock (&mix->priv->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SINK, &context); + g_rec_mutex_lock (&mix->priv->context_lock); + if (ret) { + if (mix->context != prev_context) { + /* we need to recheck everything now that we dropped the lock */ + if (mix->context && mix->context->display == mix->display) { + if (context != mix->context) + gst_clear_object (&context); + return TRUE; + } + } + if (context->display == mix->display) { mix->context = context; return TRUE; @@ -121,6 +162,7 @@ _find_local_gl_context (GstGLBaseMixer * mix) if (context != mix->context) gst_clear_object (&context); } + return FALSE; } @@ -140,7 +182,7 @@ _get_gl_context_unlocked (GstGLBaseMixer * mix) gst_gl_display_filter_gl_api (mix->display, mix_class->supported_gl_api); - _find_local_gl_context (mix); + _find_local_gl_context_unlocked (mix); GST_OBJECT_LOCK (mix->display); if (!mix->context) { @@ -243,11 +285,26 @@ gst_gl_base_mixer_sink_query (GstAggregator * agg, GstAggregatorPad * bpad, switch (GST_QUERY_TYPE (query)) { case GST_QUERY_CONTEXT: { + GstGLDisplay *display = NULL; + GstGLContext *other = NULL, *local = NULL; gboolean ret; + g_rec_mutex_lock (&mix->priv->context_lock); - ret = gst_gl_handle_context_query ((GstElement *) mix, query, - mix->display, mix->context, mix->priv->other_context); + if (mix->display) + display = gst_object_ref (mix->display); + if (mix->context) + local = gst_object_ref (mix->context); + if (mix->priv->other_context) + local = gst_object_ref (mix->priv->other_context); g_rec_mutex_unlock (&mix->priv->context_lock); + + ret = gst_gl_handle_context_query ((GstElement *) mix, query, + display, local, other); + + gst_clear_object (&display); + gst_clear_object (&other); + gst_clear_object (&local); + if (ret) return ret; break; @@ -438,7 +495,7 @@ gst_gl_base_mixer_activate (GstGLBaseMixer * mix, gboolean active) g_rec_mutex_lock (&mix->priv->context_lock); if (!gst_gl_ensure_element_data (mix, &mix->display, &mix->priv->other_context)) { - g_rec_mutex_lock (&mix->priv->context_lock); + g_rec_mutex_unlock (&mix->priv->context_lock); return FALSE; } @@ -478,11 +535,26 @@ gst_gl_base_mixer_src_query (GstAggregator * agg, GstQuery * query) switch (GST_QUERY_TYPE (query)) { case GST_QUERY_CONTEXT: { + GstGLDisplay *display = NULL; + GstGLContext *other = NULL, *local = NULL; gboolean ret; + g_rec_mutex_lock (&mix->priv->context_lock); - ret = gst_gl_handle_context_query ((GstElement *) mix, query, - mix->display, mix->context, mix->priv->other_context); + if (mix->display) + display = gst_object_ref (mix->display); + if (mix->context) + local = gst_object_ref (mix->context); + if (mix->priv->other_context) + local = gst_object_ref (mix->priv->other_context); g_rec_mutex_unlock (&mix->priv->context_lock); + + ret = gst_gl_handle_context_query ((GstElement *) mix, query, + display, local, other); + + gst_clear_object (&display); + gst_clear_object (&other); + gst_clear_object (&local); + if (ret) return ret; break; diff --git a/ext/gl/gstglmixerbin.c b/ext/gl/gstglmixerbin.c index 541469f093..a5f155fdc9 100644 --- a/ext/gl/gstglmixerbin.c +++ b/ext/gl/gstglmixerbin.c @@ -121,6 +121,7 @@ enum PROP_LATENCY, PROP_START_TIME_SELECTION, PROP_START_TIME, + PROP_CONTEXT, }; enum @@ -204,6 +205,12 @@ gst_gl_mixer_bin_class_init (GstGLMixerBinClass * klass) G_MAXUINT64, DEFAULT_START_TIME, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); + g_object_class_install_property (gobject_class, PROP_CONTEXT, + g_param_spec_object ("context", + "OpenGL context", + "Get OpenGL context", + GST_TYPE_GL_CONTEXT, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS)); + /** * GstMixerBin::create-element: * @object: the #GstGLMixerBin diff --git a/ext/gl/gstglstereosplit.c b/ext/gl/gstglstereosplit.c index 421772a613..e990ce0d13 100644 --- a/ext/gl/gstglstereosplit.c +++ b/ext/gl/gstglstereosplit.c @@ -411,6 +411,9 @@ stereosplit_set_output_caps (GstGLStereoSplit * split, GstCaps * sinkcaps) goto fail; } + /* FIXME: Provide left and right caps to do_bufferpool */ + stereosplit_do_bufferpool (split, left); + g_rec_mutex_lock (&split->context_lock); gst_gl_view_convert_set_context (split->viewconvert, split->context); @@ -424,9 +427,6 @@ stereosplit_set_output_caps (GstGLStereoSplit * split, GstCaps * sinkcaps) GST_ERROR_OBJECT (split, "Failed to set caps on converter"); goto fail; } - - /* FIXME: Provide left and right caps to do_bufferpool */ - stereosplit_do_bufferpool (split, left); g_rec_mutex_unlock (&split->context_lock); res = TRUE; @@ -444,10 +444,33 @@ fail: static gboolean _find_local_gl_context_unlocked (GstGLStereoSplit * split) { - GstGLContext *context = split->context; + GstGLContext *context, *prev_context; + gboolean ret; + + if (split->context && split->context->display == split->display) + return TRUE; + + context = prev_context = split->context; + g_rec_mutex_unlock (&split->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SRC, + &context); + g_rec_mutex_lock (&split->context_lock); + if (ret) { + if (split->context != prev_context) { + /* we need to recheck everything since we dropped the lock and the + * context has changed */ + if (split->context && split->context->display == split->display) { + if (context != split->context) + gst_clear_object (&context); + return TRUE; + } + } - if (gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SRC, - &context)) { if (context->display == split->display) { split->context = context; return TRUE; @@ -455,9 +478,27 @@ _find_local_gl_context_unlocked (GstGLStereoSplit * split) if (context != split->context) gst_clear_object (&context); } - context = split->context; - if (gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SINK, - &context)) { + + context = prev_context = split->context; + g_rec_mutex_unlock (&split->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SINK, + &context); + g_rec_mutex_lock (&split->context_lock); + if (ret) { + if (split->context != prev_context) { + /* we need to recheck everything now that we dropped the lock */ + if (split->context && split->context->display == split->display) { + if (context != split->context) + gst_clear_object (&context); + return TRUE; + } + } + if (context->display == split->display) { split->context = context; return TRUE; @@ -465,6 +506,7 @@ _find_local_gl_context_unlocked (GstGLStereoSplit * split) if (context != split->context) gst_clear_object (&context); } + return FALSE; } @@ -552,7 +594,6 @@ stereosplit_decide_allocation (GstGLStereoSplit * self, GstQuery * query) return FALSE; return TRUE; - } static gboolean @@ -670,8 +711,26 @@ stereosplit_src_query (GstPad * pad, GstObject * parent, GstQuery * query) switch (GST_QUERY_TYPE (query)) { case GST_QUERY_CONTEXT: { - if (gst_gl_handle_context_query ((GstElement *) split, query, - split->display, split->context, split->other_context)) + GstGLDisplay *display = NULL; + GstGLContext *other = NULL, *local = NULL; + gboolean ret; + + g_rec_mutex_lock (&split->context_lock); + if (split->display) + display = gst_object_ref (split->display); + if (split->context) + local = gst_object_ref (split->context); + if (split->other_context) + local = gst_object_ref (split->other_context); + g_rec_mutex_unlock (&split->context_lock); + + ret = gst_gl_handle_context_query ((GstElement *) split, query, + display, local, other); + + gst_clear_object (&display); + gst_clear_object (&other); + gst_clear_object (&local); + if (ret) return TRUE; return gst_pad_query_default (pad, parent, query); @@ -699,8 +758,26 @@ stereosplit_sink_query (GstPad * pad, GstObject * parent, GstQuery * query) switch (GST_QUERY_TYPE (query)) { case GST_QUERY_CONTEXT: { - if (gst_gl_handle_context_query ((GstElement *) split, query, - split->display, split->context, split->other_context)) + GstGLDisplay *display = NULL; + GstGLContext *other = NULL, *local = NULL; + gboolean ret; + + g_rec_mutex_lock (&split->context_lock); + if (split->display) + display = gst_object_ref (split->display); + if (split->context) + local = gst_object_ref (split->context); + if (split->other_context) + local = gst_object_ref (split->other_context); + g_rec_mutex_unlock (&split->context_lock); + + ret = gst_gl_handle_context_query ((GstElement *) split, query, + display, local, other); + + gst_clear_object (&display); + gst_clear_object (&other); + gst_clear_object (&local); + if (ret) return TRUE; return gst_pad_query_default (pad, parent, query); diff --git a/gst-libs/gst/gl/gstglbasefilter.c b/gst-libs/gst/gl/gstglbasefilter.c index 1244653b68..0524503207 100644 --- a/gst-libs/gst/gl/gstglbasefilter.c +++ b/gst-libs/gst/gl/gstglbasefilter.c @@ -180,31 +180,83 @@ gst_gl_base_filter_get_property (GObject * object, guint prop_id, } } +static gboolean +_find_local_gl_context_unlocked (GstGLBaseFilter * filter) +{ + GstGLContext *context, *prev_context; + gboolean ret; + + if (filter->context && filter->context->display == filter->display) + return TRUE; + + context = prev_context = filter->context; + g_rec_mutex_unlock (&filter->priv->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SRC, + &context); + g_rec_mutex_lock (&filter->priv->context_lock); + if (ret) { + if (filter->context != prev_context) { + /* we need to recheck everything since we dropped the lock and the + * context has changed */ + if (filter->context && filter->context->display == filter->display) { + if (context != filter->context) + gst_clear_object (&context); + return TRUE; + } + } + + if (context->display == filter->display) { + filter->context = context; + return TRUE; + } + if (context != filter->context) + gst_clear_object (&context); + } + + context = prev_context = filter->context; + g_rec_mutex_unlock (&filter->priv->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SINK, + &context); + g_rec_mutex_lock (&filter->priv->context_lock); + if (ret) { + if (filter->context != prev_context) { + /* we need to recheck everything now that we dropped the lock */ + if (filter->context && filter->context->display == filter->display) { + if (context != filter->context) + gst_clear_object (&context); + return TRUE; + } + } + + if (context->display == filter->display) { + filter->context = context; + return TRUE; + } + if (context != filter->context) + gst_clear_object (&context); + } + + return FALSE; +} + static gboolean _find_local_gl_context (GstGLBaseFilter * filter) { - GstGLContext *context = filter->context; - - if (gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SRC, - &context)) { - if (context->display == filter->display) { - filter->context = context; - return TRUE; - } - if (context != filter->context) - gst_clear_object (&context); - } - context = filter->context; - if (gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SINK, - &context)) { - if (context->display == filter->display) { - filter->context = context; - return TRUE; - } - if (context != filter->context) - gst_clear_object (&context); - } - return FALSE; + gboolean ret; + g_rec_mutex_lock (&filter->priv->context_lock); + ret = _find_local_gl_context_unlocked (filter); + g_rec_mutex_unlock (&filter->priv->context_lock); + return ret; } static gboolean @@ -218,9 +270,7 @@ gst_gl_base_filter_query (GstBaseTransform * trans, GstPadDirection direction, { if (direction == GST_PAD_SINK && gst_base_transform_is_passthrough (trans)) { - g_rec_mutex_lock (&filter->priv->context_lock); _find_local_gl_context (filter); - g_rec_mutex_unlock (&filter->priv->context_lock); return gst_pad_peer_query (GST_BASE_TRANSFORM_SRC_PAD (trans), query); } @@ -228,11 +278,26 @@ gst_gl_base_filter_query (GstBaseTransform * trans, GstPadDirection direction, } case GST_QUERY_CONTEXT: { + GstGLDisplay *display = NULL; + GstGLContext *other = NULL, *local = NULL; gboolean ret; + g_rec_mutex_lock (&filter->priv->context_lock); - ret = gst_gl_handle_context_query ((GstElement *) filter, query, - filter->display, filter->context, filter->priv->other_context); + if (filter->display) + display = gst_object_ref (filter->display); + if (filter->context) + local = gst_object_ref (filter->context); + if (filter->priv->other_context) + local = gst_object_ref (filter->priv->other_context); g_rec_mutex_unlock (&filter->priv->context_lock); + + ret = gst_gl_handle_context_query ((GstElement *) filter, query, + display, local, other); + + gst_clear_object (&display); + gst_clear_object (&other); + gst_clear_object (&local); + if (ret) return TRUE; break; @@ -418,20 +483,9 @@ gst_gl_base_filter_change_state (GstElement * element, switch (transition) { case GST_STATE_CHANGE_READY_TO_NULL: g_rec_mutex_lock (&filter->priv->context_lock); - if (filter->priv->other_context) { - gst_object_unref (filter->priv->other_context); - filter->priv->other_context = NULL; - } - - if (filter->display) { - gst_object_unref (filter->display); - filter->display = NULL; - } - - if (filter->context) { - gst_object_unref (filter->context); - filter->context = NULL; - } + gst_clear_object (&filter->priv->other_context); + gst_clear_object (&filter->display); + gst_clear_object (&filter->context); g_rec_mutex_unlock (&filter->priv->context_lock); break; default: @@ -487,7 +541,7 @@ gst_gl_base_filter_find_gl_context_unlocked (GstGLBaseFilter * filter) if (!filter->context) new_context = TRUE; - _find_local_gl_context (filter); + _find_local_gl_context_unlocked (filter); if (!filter->context) { GST_OBJECT_LOCK (filter->display); diff --git a/gst-libs/gst/gl/gstglbasesrc.c b/gst-libs/gst/gl/gstglbasesrc.c index 28a64448e3..b5a6cc830f 100644 --- a/gst-libs/gst/gl/gstglbasesrc.c +++ b/gst-libs/gst/gl/gstglbasesrc.c @@ -479,11 +479,34 @@ gst_gl_base_src_stop (GstBaseSrc * basesrc) } static gboolean -_find_local_gl_context (GstGLBaseSrc * src) +_find_local_gl_context_unlocked (GstGLBaseSrc * src) { - GstGLContext *context = src->context; + GstGLContext *context, *prev_context; + gboolean ret; + + if (src->context && src->context->display == src->display) + return TRUE; + + context = prev_context = src->context; + g_rec_mutex_unlock (&src->priv->context_lock); + /* we need to drop the lock to query as another element may also be + * performing a context query on us which would also attempt to take the + * context_lock. Our query could block on the same lock in the other element. + */ + ret = + gst_gl_query_local_gl_context (GST_ELEMENT (src), GST_PAD_SRC, &context); + g_rec_mutex_lock (&src->priv->context_lock); + if (ret) { + if (src->context != prev_context) { + /* we need to recheck everything since we dropped the lock and the + * context has changed */ + if (src->context && src->context->display == src->display) { + if (context != src->context) + gst_clear_object (&context); + return TRUE; + } + } - if (gst_gl_query_local_gl_context (GST_ELEMENT (src), GST_PAD_SRC, &context)) { if (context->display == src->display) { src->context = context; return TRUE; @@ -513,7 +536,7 @@ gst_gl_base_src_find_gl_context_unlocked (GstGLBaseSrc * src) gst_gl_display_filter_gl_api (src->display, klass->supported_gl_api); - _find_local_gl_context (src); + _find_local_gl_context_unlocked (src); if (!src->context) { GST_OBJECT_LOCK (src->display); diff --git a/tests/check/elements/glmixer.c b/tests/check/elements/glmixer.c new file mode 100644 index 0000000000..ecd760ca20 --- /dev/null +++ b/tests/check/elements/glmixer.c @@ -0,0 +1,145 @@ +/* GStreamer + * + * Copyright (C) 2020 Matthew Waters + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Library General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Library General Public License for more details. + * + * You should have received a copy of the GNU Library General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, + * Boston, MA 02110-1301, USA. + */ + + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include +#include +#include + +static void +replace_display (GstHarness * h) +{ + GstContext *new_context; + GstGLDisplay *new_display; + GstGLContext *expected, *gl_context; + GstBuffer *buf; + + /* replaces the GstGLDisplay used by @h with verification */ + + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push_from_src (h)); + buf = gst_harness_pull (h); + fail_unless (buf != NULL); + gst_clear_buffer (&buf); + + g_object_get (G_OBJECT (h->element), "context", &gl_context, NULL); + fail_unless (gl_context != NULL); + gst_clear_object (&gl_context); + + new_display = gst_gl_display_new (); + fail_unless (gst_gl_display_create_context (new_display, NULL, &expected, + NULL)); + fail_unless (expected != NULL); + gst_gl_display_add_context (new_display, expected); + + new_context = gst_context_new (GST_GL_DISPLAY_CONTEXT_TYPE, TRUE); + gst_context_set_gl_display (new_context, new_display); + + gst_element_set_context (h->element, new_context); + gst_context_unref (new_context); + new_context = NULL; + + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push_from_src (h)); + buf = gst_harness_pull (h); + fail_unless (buf != NULL); + gst_clear_buffer (&buf); + + g_object_get (G_OBJECT (h->element), "context", &gl_context, NULL); + fail_unless (gl_context != NULL); + + fail_unless (gl_context == expected); + fail_unless (new_display == gl_context->display); + + gst_object_unref (expected); + gst_object_unref (gl_context); + gst_object_unref (new_display); +} + +GST_START_TEST (test_glvideomixer_negotiate) +{ + GstHarness *mix; + GstBuffer *buf; + + mix = gst_harness_new_with_padnames ("glvideomixer", "sink_0", "src"); + gst_harness_use_systemclock (mix); + gst_harness_set_blocking_push_mode (mix); + gst_harness_set_caps_str (mix, + "video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D", + "video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D"); + gst_harness_add_src (mix, "gltestsrc", FALSE); + gst_harness_set_blocking_push_mode (mix->src_harness); + + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push_from_src (mix)); + + buf = gst_harness_pull (mix); + fail_unless (buf != NULL); + gst_clear_buffer (&buf); + + gst_harness_teardown (mix); +} + +GST_END_TEST; + +GST_START_TEST (test_glvideomixer_display_replace) +{ + GstHarness *mix; + + mix = gst_harness_new_with_padnames ("glvideomixer", "sink_0", "src"); + gst_harness_use_systemclock (mix); + gst_harness_set_blocking_push_mode (mix); + gst_harness_set_caps_str (mix, + "video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D", + "video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D"); + gst_harness_add_src (mix, "gltestsrc", FALSE); + gst_harness_set_blocking_push_mode (mix->src_harness); + + replace_display (mix); + + gst_harness_teardown (mix); +} + +GST_END_TEST; + +static Suite * +glmixer_suite (void) +{ + Suite *s = suite_create ("glmixer"); + TCase *tc = tcase_create ("general"); + + tcase_add_test (tc, test_glvideomixer_negotiate); + tcase_add_test (tc, test_glvideomixer_display_replace); + suite_add_tcase (s, tc); + + return s; +} + +int +main (int argc, char **argv) +{ + Suite *s; + g_setenv ("GST_GL_XINITTHREADS", "1", TRUE); + gst_check_init (&argc, &argv); + s = glmixer_suite (); + return gst_check_run_suite (s, "glmixer", __FILE__); +} diff --git a/tests/check/meson.build b/tests/check/meson.build index 3d267febf6..0f7202e8ee 100644 --- a/tests/check/meson.build +++ b/tests/check/meson.build @@ -102,6 +102,7 @@ if build_gstgl and host_machine.system() != 'windows' [ 'pipelines/gl-launch-lines.c', not build_gstgl ], [ 'elements/glfilter.c', not build_gstgl, [gstgl_dep]], [ 'elements/glstereo.c', not build_gstgl, [gstgl_dep]], + [ 'elements/glmixer.c', not build_gstgl, [gstgl_dep]], ] endif