From acc294bf5adc95cc8bfcad19a6626357023f1586 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Fri, 2 Nov 2018 14:32:04 -0300 Subject: [PATCH] ges: Check the thread from which our API is used And add some missing API guards --- ges/ges-pipeline.c | 14 ++++++++++++++ ges/ges-timeline.c | 48 +++++++++++++++++++++++++++++++++++++++++++++- ges/ges-track.c | 16 ++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/ges/ges-pipeline.c b/ges/ges-pipeline.c index c82f4ffc35..9dda9307c3 100644 --- a/ges/ges-pipeline.c +++ b/ges/ges-pipeline.c @@ -44,6 +44,7 @@ GST_DEBUG_CATEGORY_STATIC (ges_pipeline_debug); #define DEFAULT_TIMELINE_MODE GES_PIPELINE_MODE_PREVIEW #define IN_RENDERING_MODE(timeline) ((timeline->priv->mode) & (GES_PIPELINE_MODE_RENDER | GES_PIPELINE_MODE_SMART_RENDER)) +#define CHECK_THREAD(pipeline) g_assert(pipeline->priv->valid_thread == g_thread_self()) /* Structure corresponding to a timeline - sink link */ @@ -74,6 +75,8 @@ struct _GESPipelinePrivate GList *not_rendered_tracks; GstEncodingProfile *profile; + + GThread *valid_thread; }; enum @@ -367,6 +370,7 @@ ges_pipeline_init (GESPipeline * self) { GST_INFO_OBJECT (self, "Creating new 'playsink'"); self->priv = ges_pipeline_get_instance_private (self); + self->priv->valid_thread = g_thread_self (); self->priv->playsink = gst_element_factory_make ("playsink", "internal-sinks"); @@ -1012,6 +1016,7 @@ ges_pipeline_set_timeline (GESPipeline * pipeline, GESTimeline * timeline) g_return_val_if_fail (GES_IS_PIPELINE (pipeline), FALSE); g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); g_return_val_if_fail (pipeline->priv->timeline == NULL, FALSE); + CHECK_THREAD (pipeline); GST_DEBUG ("pipeline:%p, timeline:%p", timeline, pipeline); @@ -1055,6 +1060,7 @@ ges_pipeline_set_render_settings (GESPipeline * pipeline, GstEncodingProfile *set_profile; g_return_val_if_fail (GES_IS_PIPELINE (pipeline), FALSE); + CHECK_THREAD (pipeline); /* FIXME Properly handle multi track, for now GESPipeline * only hanles single track per type, so we should just set the @@ -1154,6 +1160,7 @@ ges_pipeline_set_mode (GESPipeline * pipeline, GESPipelineFlags mode) GList *tmp; g_return_val_if_fail (GES_IS_PIPELINE (pipeline), FALSE); + CHECK_THREAD (pipeline); GST_DEBUG_OBJECT (pipeline, "current mode : %d, mode : %d", pipeline->priv->mode, mode); @@ -1280,6 +1287,7 @@ ges_pipeline_get_thumbnail (GESPipeline * self, GstCaps * caps) GstElement *sink; g_return_val_if_fail (GES_IS_PIPELINE (self), FALSE); + CHECK_THREAD (self); sink = self->priv->playsink; @@ -1317,6 +1325,7 @@ ges_pipeline_save_thumbnail (GESPipeline * self, int width, int gboolean res = TRUE; g_return_val_if_fail (GES_IS_PIPELINE (self), FALSE); + CHECK_THREAD (self); caps = gst_caps_from_string (format); @@ -1373,6 +1382,7 @@ ges_pipeline_get_thumbnail_rgb24 (GESPipeline * self, gint width, gint height) GstCaps *caps; g_return_val_if_fail (GES_IS_PIPELINE (self), FALSE); + CHECK_THREAD (self); caps = gst_caps_new_simple ("video/x-raw", "format", G_TYPE_STRING, "RGB", NULL); @@ -1406,6 +1416,7 @@ ges_pipeline_preview_get_video_sink (GESPipeline * self) GstElement *sink = NULL; g_return_val_if_fail (GES_IS_PIPELINE (self), FALSE); + CHECK_THREAD (self); g_object_get (self->priv->playsink, "video-sink", &sink, NULL); @@ -1424,6 +1435,7 @@ void ges_pipeline_preview_set_video_sink (GESPipeline * self, GstElement * sink) { g_return_if_fail (GES_IS_PIPELINE (self)); + CHECK_THREAD (self); g_object_set (self->priv->playsink, "video-sink", sink, NULL); }; @@ -1446,6 +1458,7 @@ ges_pipeline_preview_get_audio_sink (GESPipeline * self) GstElement *sink = NULL; g_return_val_if_fail (GES_IS_PIPELINE (self), FALSE); + CHECK_THREAD (self); g_object_get (self->priv->playsink, "audio-sink", &sink, NULL); @@ -1464,6 +1477,7 @@ void ges_pipeline_preview_set_audio_sink (GESPipeline * self, GstElement * sink) { g_return_if_fail (GES_IS_PIPELINE (self)); + CHECK_THREAD (self); g_object_set (self->priv->playsink, "audio-sink", sink, NULL); }; diff --git a/ges/ges-timeline.c b/ges/ges-timeline.c index de5880fe67..4394146dd2 100644 --- a/ges/ges-timeline.c +++ b/ges/ges-timeline.c @@ -81,6 +81,8 @@ GST_DEBUG_CATEGORY_STATIC (ges_timeline_debug); g_thread_self()); \ } G_STMT_END +#define CHECK_THREAD(timeline) g_assert(timeline->priv->valid_thread == g_thread_self()) + typedef struct TrackObjIters { GSequenceIter *iter_start; @@ -207,6 +209,8 @@ struct _GESTimelinePrivate /* For ges_timeline_commit_sync */ GMutex commited_lock; GCond commited_cond; + + GThread *valid_thread; }; /* private structure to contain our track-related information */ @@ -696,6 +700,7 @@ ges_timeline_init (GESTimeline * self) g_rec_mutex_init (&priv->dyn_mutex); g_mutex_init (&priv->commited_lock); + priv->valid_thread = g_thread_self (); } /* Private methods */ @@ -2961,6 +2966,9 @@ ges_timeline_save_to_uri (GESTimeline * timeline, const gchar * uri, GList * ges_timeline_get_groups (GESTimeline * timeline) { + g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + CHECK_THREAD (timeline); + return timeline->priv->groups; } @@ -2980,6 +2988,9 @@ ges_timeline_append_layer (GESTimeline * timeline) guint32 priority; GESLayer *layer; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + CHECK_THREAD (timeline); + layer = ges_layer_new (); priority = g_list_length (timeline->layers); ges_layer_set_priority (layer, priority); @@ -3005,6 +3016,10 @@ ges_timeline_add_layer (GESTimeline * timeline, GESLayer * layer) gboolean auto_transition; GList *objects, *tmp; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + g_return_val_if_fail (GES_IS_LAYER (layer), FALSE); + CHECK_THREAD (timeline); + GST_DEBUG ("timeline:%p, layer:%p", timeline, layer); /* We can only add a layer that doesn't already belong to another timeline */ @@ -3084,6 +3099,10 @@ ges_timeline_remove_layer (GESTimeline * timeline, GESLayer * layer) { GList *layer_objects, *tmp; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + g_return_val_if_fail (GES_IS_LAYER (layer), FALSE); + CHECK_THREAD (timeline); + GST_DEBUG ("timeline:%p, layer:%p", timeline, layer); if (G_UNLIKELY (!g_list_find (timeline->layers, layer))) { @@ -3144,6 +3163,10 @@ ges_timeline_add_track (GESTimeline * timeline, GESTrack * track) TrackPrivate *tr_priv; GList *tmp; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + g_return_val_if_fail (GES_IS_TRACK (track), FALSE); + CHECK_THREAD (timeline); + GST_DEBUG ("timeline:%p, track:%p", timeline, track); /* make sure we don't already control it */ @@ -3235,6 +3258,7 @@ ges_timeline_remove_track (GESTimeline * timeline, GESTrack * track) g_return_val_if_fail (GES_IS_TRACK (track), FALSE); g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + CHECK_THREAD (timeline); GST_DEBUG ("timeline:%p, track:%p", timeline, track); @@ -3306,6 +3330,8 @@ ges_timeline_get_track_for_pad (GESTimeline * timeline, GstPad * pad) { GList *tmp; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + LOCK_DYN (timeline); for (tmp = timeline->priv->priv_tracks; tmp; tmp = g_list_next (tmp)) { TrackPrivate *tr_priv = (TrackPrivate *) tmp->data; @@ -3365,6 +3391,7 @@ GList * ges_timeline_get_tracks (GESTimeline * timeline) { g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + CHECK_THREAD (timeline); return g_list_copy_deep (timeline->tracks, (GCopyFunc) gst_object_ref, NULL); } @@ -3384,6 +3411,9 @@ ges_timeline_get_layers (GESTimeline * timeline) { GList *tmp, *res = NULL; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + CHECK_THREAD (timeline); + for (tmp = timeline->layers; tmp; tmp = g_list_next (tmp)) { res = g_list_insert_sorted (res, gst_object_ref (tmp->data), (GCompareFunc) sort_layers); @@ -3481,6 +3511,8 @@ ges_timeline_commit (GESTimeline * timeline) { gboolean ret; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + LOCK_DYN (timeline); ret = ges_timeline_commit_unlocked (timeline); UNLOCK_DYN (timeline); @@ -3524,6 +3556,8 @@ ges_timeline_commit_sync (GESTimeline * timeline) gboolean ret; gboolean wait_for_signal; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + /* Let's make sure our state is stable */ gst_element_get_state (GST_ELEMENT (timeline), NULL, NULL, GST_CLOCK_TIME_NONE); @@ -3565,6 +3599,7 @@ GstClockTime ges_timeline_get_duration (GESTimeline * timeline) { g_return_val_if_fail (GES_IS_TIMELINE (timeline), GST_CLOCK_TIME_NONE); + CHECK_THREAD (timeline); return timeline->priv->duration; } @@ -3581,7 +3616,8 @@ ges_timeline_get_duration (GESTimeline * timeline) gboolean ges_timeline_get_auto_transition (GESTimeline * timeline) { - g_return_val_if_fail (GES_IS_TIMELINE (timeline), 0); + g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + CHECK_THREAD (timeline); return timeline->priv->auto_transition; } @@ -3602,6 +3638,7 @@ ges_timeline_set_auto_transition (GESTimeline * timeline, GESLayer *layer; g_return_if_fail (GES_IS_TIMELINE (timeline)); + CHECK_THREAD (timeline); timeline->priv->auto_transition = auto_transition; g_object_notify (G_OBJECT (timeline), "auto-transition"); @@ -3627,6 +3664,7 @@ GstClockTime ges_timeline_get_snapping_distance (GESTimeline * timeline) { g_return_val_if_fail (GES_IS_TIMELINE (timeline), GST_CLOCK_TIME_NONE); + CHECK_THREAD (timeline); return timeline->priv->snapping_distance; @@ -3645,6 +3683,7 @@ ges_timeline_set_snapping_distance (GESTimeline * timeline, GstClockTime snapping_distance) { g_return_if_fail (GES_IS_TIMELINE (timeline)); + CHECK_THREAD (timeline); timeline->priv->snapping_distance = snapping_distance; } @@ -3664,6 +3703,7 @@ ges_timeline_get_element (GESTimeline * timeline, const gchar * name) GESTimelineElement *ret; g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + CHECK_THREAD (timeline); ret = g_hash_table_lookup (timeline->priv->all_elements, name); @@ -3702,6 +3742,7 @@ ges_timeline_is_empty (GESTimeline * timeline) gpointer key, value; g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); + CHECK_THREAD (timeline); if (g_hash_table_size (timeline->priv->all_elements) == 0) return TRUE; @@ -3734,6 +3775,9 @@ ges_timeline_get_layer (GESTimeline * timeline, guint priority) GList *tmp; GESLayer *layer = NULL; + g_return_val_if_fail (GES_IS_TIMELINE (timeline), NULL); + CHECK_THREAD (timeline); + for (tmp = timeline->layers; tmp; tmp = tmp->next) { GESLayer *tmp_layer = GES_LAYER (tmp->data); guint tmp_priority; @@ -3772,6 +3816,7 @@ ges_timeline_paste_element (GESTimeline * timeline, g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); g_return_val_if_fail (GES_IS_TIMELINE_ELEMENT (element), FALSE); + CHECK_THREAD (timeline); element_class = GES_TIMELINE_ELEMENT_GET_CLASS (element); copied_from = ges_timeline_element_get_copied_from (element); @@ -3825,6 +3870,7 @@ ges_timeline_move_layer (GESTimeline * timeline, GESLayer * layer, g_return_val_if_fail (GES_IS_TIMELINE (timeline), FALSE); g_return_val_if_fail (GES_IS_LAYER (layer), FALSE); g_return_val_if_fail (ges_layer_get_timeline (layer) == timeline, FALSE); + CHECK_THREAD (timeline); current_priority = ges_layer_get_priority (layer); diff --git a/ges/ges-track.c b/ges/ges-track.c index df5fbfdb79..534a7af63a 100644 --- a/ges/ges-track.c +++ b/ges/ges-track.c @@ -35,6 +35,8 @@ #include "ges-video-track.h" #include "ges-audio-track.h" +#define CHECK_THREAD(track) g_assert(track->priv->valid_thread == g_thread_self()) + static GstStaticPadTemplate ges_track_src_pad_template = GST_STATIC_PAD_TEMPLATE ("src", GST_PAD_SRC, @@ -77,6 +79,8 @@ struct _GESTrackPrivate /* Virtual method to create GstElement that fill gaps */ GESCreateElementForGapFunc create_element_for_gaps; + + GThread *valid_thread; }; enum @@ -683,6 +687,7 @@ static void ges_track_init (GESTrack * self) { self->priv = ges_track_get_instance_private (self); + self->priv->valid_thread = g_thread_self (); self->priv->composition = gst_element_factory_make ("nlecomposition", NULL); self->priv->capsfilter = gst_element_factory_make ("capsfilter", NULL); @@ -787,6 +792,7 @@ ges_track_set_caps (GESTrack * track, const GstCaps * caps) gint i; g_return_if_fail (GES_IS_TRACK (track)); + CHECK_THREAD (track); GST_DEBUG ("track:%p, caps:%" GST_PTR_FORMAT, track, caps); g_return_if_fail (GST_IS_CAPS (caps)); @@ -817,6 +823,7 @@ ges_track_set_restriction_caps (GESTrack * track, const GstCaps * caps) GESTrackPrivate *priv; g_return_if_fail (GES_IS_TRACK (track)); + CHECK_THREAD (track); GST_DEBUG ("track:%p, restriction caps:%" GST_PTR_FORMAT, track, caps); g_return_if_fail (GST_IS_CAPS (caps)); @@ -852,6 +859,7 @@ ges_track_update_restriction_caps (GESTrack * self, const GstCaps * caps) GstCaps *new_restriction_caps; g_return_if_fail (GES_IS_TRACK (self)); + CHECK_THREAD (self); if (!self->priv->restriction_caps) { ges_track_set_restriction_caps (self, caps); @@ -886,6 +894,7 @@ void ges_track_set_mixing (GESTrack * track, gboolean mixing) { g_return_if_fail (GES_IS_TRACK (track)); + CHECK_THREAD (track); if (!track->priv->mixing_operation) { GST_DEBUG_OBJECT (track, "Track will be set to mixing = %d", mixing); @@ -935,6 +944,7 @@ ges_track_add_element (GESTrack * track, GESTrackElement * object) { g_return_val_if_fail (GES_IS_TRACK (track), FALSE); g_return_val_if_fail (GES_IS_TRACK_ELEMENT (object), FALSE); + CHECK_THREAD (track); GST_DEBUG ("track:%p, object:%p", track, object); @@ -1001,6 +1011,7 @@ ges_track_get_elements (GESTrack * track) GList *ret = NULL; g_return_val_if_fail (GES_IS_TRACK (track), NULL); + CHECK_THREAD (track); g_sequence_foreach (track->priv->trackelements_by_start, (GFunc) add_trackelement_to_list_foreach, &ret); @@ -1030,6 +1041,7 @@ ges_track_remove_element (GESTrack * track, GESTrackElement * object) g_return_val_if_fail (GES_IS_TRACK (track), FALSE); g_return_val_if_fail (GES_IS_TRACK_ELEMENT (object), FALSE); + CHECK_THREAD (track); priv = track->priv; @@ -1064,6 +1076,7 @@ const GstCaps * ges_track_get_caps (GESTrack * track) { g_return_val_if_fail (GES_IS_TRACK (track), NULL); + CHECK_THREAD (track); return track->priv->caps; } @@ -1080,6 +1093,7 @@ const GESTimeline * ges_track_get_timeline (GESTrack * track) { g_return_val_if_fail (GES_IS_TRACK (track), NULL); + CHECK_THREAD (track); return track->priv->timeline; } @@ -1119,6 +1133,7 @@ gboolean ges_track_commit (GESTrack * track) { g_return_val_if_fail (GES_IS_TRACK (track), FALSE); + CHECK_THREAD (track); track_resort_and_fill_gaps (track); @@ -1141,6 +1156,7 @@ ges_track_set_create_element_for_gap_func (GESTrack * track, GESCreateElementForGapFunc func) { g_return_if_fail (GES_IS_TRACK (track)); + CHECK_THREAD (track); track->priv->create_element_for_gaps = func; }