player: inhibit signals after gst_player_stop() has been called

Also wait for the state change to STOP to have been announced before
destroying the player so it won't appear as leaked by leak detector
tools.

https://bugzilla.gnome.org/show_bug.cgi?id=766607

diff --git a/gst-libs/gst/player/gstplayer.c b/gst-libs/gst/player/gstplayer.c
index 60521a0..56368e5 100644 (file)
--- a/gst-libs/gst/player/gstplayer.c
+++ b/gst-libs/gst/player/gstplayer.c
@@ -166,6 +166,10 @@ struct _GstPlayer
   GstClockTime last_seek_time;  /* Only set from main context */
   GSource *seek_source;
   GstClockTime seek_position;
+  /* If TRUE, all signals are inhibited except the
+   * state-changed:GST_PLAYER_STATE_STOPPED/PAUSED. This ensures that no signal
+   * is emitted after gst_player_stop/pause() has been called by the user. */
+  gboolean inhibit_sigs;
 };
 
 struct _GstPlayerClass
@@ -243,6 +247,7 @@ gst_player_init (GstPlayer * self)
   self->seek_pending = FALSE;
   self->seek_position = GST_CLOCK_TIME_NONE;
   self->last_seek_time = GST_CLOCK_TIME_NONE;
+  self->inhibit_sigs = FALSE;
 
   GST_TRACE_OBJECT (self, "Initialized");
 }
@@ -788,6 +793,10 @@ state_changed_dispatch (gpointer user_data)
 {
   StateChangedSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs && data->state != GST_PLAYER_STATE_STOPPED
+      && data->state != GST_PLAYER_STATE_PAUSED)
+    return;
+
   g_signal_emit (data->player, signals[SIGNAL_STATE_CHANGED], 0, data->state);
 }
 
@@ -832,6 +841,9 @@ position_updated_dispatch (gpointer user_data)
 {
   PositionUpdatedSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   if (data->player->target_state >= GST_STATE_PAUSED) {
     g_signal_emit (data->player, signals[SIGNAL_POSITION_UPDATED], 0,
         data->position);
@@ -948,6 +960,9 @@ error_dispatch (gpointer user_data)
 {
   ErrorSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   g_signal_emit (data->player, signals[SIGNAL_ERROR], 0, data->err);
 }
 
@@ -1034,6 +1049,9 @@ warning_dispatch (gpointer user_data)
 {
   WarningSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   g_signal_emit (data->player, signals[SIGNAL_WARNING], 0, data->err);
 }
 
@@ -1146,7 +1164,12 @@ warning_cb (G_GNUC_UNUSED GstBus * bus, GstMessage * msg, gpointer user_data)
 static void
 eos_dispatch (gpointer user_data)
 {
-  g_signal_emit (user_data, signals[SIGNAL_END_OF_STREAM], 0);
+  GstPlayer *player = user_data;
+
+  if (player->inhibit_sigs)
+    return;
+
+  g_signal_emit (player, signals[SIGNAL_END_OF_STREAM], 0);
 }
 
 static void
@@ -1181,6 +1204,9 @@ buffering_dispatch (gpointer user_data)
 {
   BufferingSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   if (data->player->target_state >= GST_STATE_PAUSED) {
     g_signal_emit (data->player, signals[SIGNAL_BUFFERING], 0, data->percent);
   }
@@ -1296,6 +1322,9 @@ video_dimensions_changed_dispatch (gpointer user_data)
 {
   VideoDimensionsChangedSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   if (data->player->target_state >= GST_STATE_PAUSED) {
     g_signal_emit (data->player, signals[SIGNAL_VIDEO_DIMENSIONS_CHANGED], 0,
         data->width, data->height);
@@ -1381,6 +1410,9 @@ duration_changed_dispatch (gpointer user_data)
 {
   DurationChangedSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   if (data->player->target_state >= GST_STATE_PAUSED) {
     g_signal_emit (data->player, signals[SIGNAL_DURATION_CHANGED], 0,
         data->duration);
@@ -1425,6 +1457,9 @@ seek_done_dispatch (gpointer user_data)
 {
   SeekDoneSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   g_signal_emit (data->player, signals[SIGNAL_SEEK_DONE], 0, data->position);
 }
 
@@ -1767,6 +1802,9 @@ media_info_updated_dispatch (gpointer user_data)
 {
   MediaInfoUpdatedSignalData *data = user_data;
 
+  if (data->player->inhibit_sigs)
+    return;
+
   if (data->player->target_state >= GST_STATE_PAUSED) {
     g_signal_emit (data->player, signals[SIGNAL_MEDIA_INFO_UPDATED], 0,
         data->info);
@@ -2405,8 +2443,13 @@ subtitle_tags_changed_cb (G_GNUC_UNUSED GstElement * playbin, gint stream_index,
 static void
 volume_changed_dispatch (gpointer user_data)
 {
-  g_signal_emit (user_data, signals[SIGNAL_VOLUME_CHANGED], 0);
-  g_object_notify_by_pspec (G_OBJECT (user_data), param_specs[PROP_VOLUME]);
+  GstPlayer *player = user_data;
+
+  if (player->inhibit_sigs)
+    return;
+
+  g_signal_emit (player, signals[SIGNAL_VOLUME_CHANGED], 0);
+  g_object_notify_by_pspec (G_OBJECT (player), param_specs[PROP_VOLUME]);
 }
 
 static void
@@ -2424,8 +2467,13 @@ volume_notify_cb (G_GNUC_UNUSED GObject * obj, G_GNUC_UNUSED GParamSpec * pspec,
 static void
 mute_changed_dispatch (gpointer user_data)
 {
-  g_signal_emit (user_data, signals[SIGNAL_MUTE_CHANGED], 0);
-  g_object_notify_by_pspec (G_OBJECT (user_data), param_specs[PROP_MUTE]);
+  GstPlayer *player = user_data;
+
+  if (player->inhibit_sigs)
+    return;
+
+  g_signal_emit (player, signals[SIGNAL_MUTE_CHANGED], 0);
+  g_object_notify_by_pspec (G_OBJECT (player), param_specs[PROP_MUTE]);
 }
 
 static void
@@ -2692,6 +2740,10 @@ gst_player_play (GstPlayer * self)
 {
   g_return_if_fail (GST_IS_PLAYER (self));
 
+  g_mutex_lock (&self->lock);
+  self->inhibit_sigs = FALSE;
+  g_mutex_unlock (&self->lock);
+
   g_main_context_invoke_full (self->context, G_PRIORITY_DEFAULT,
       gst_player_play_internal, self, NULL);
 }
@@ -2759,6 +2811,10 @@ gst_player_pause (GstPlayer * self)
 {
   g_return_if_fail (GST_IS_PLAYER (self));
 
+  g_mutex_lock (&self->lock);
+  self->inhibit_sigs = TRUE;
+  g_mutex_unlock (&self->lock);
+
   g_main_context_invoke_full (self->context, G_PRIORITY_DEFAULT,
       gst_player_pause_internal, self, NULL);
 }
@@ -2819,6 +2875,10 @@ gst_player_stop (GstPlayer * self)
 {
   g_return_if_fail (GST_IS_PLAYER (self));
 
+  g_mutex_lock (&self->lock);
+  self->inhibit_sigs = TRUE;
+  g_mutex_unlock (&self->lock);
+
   g_main_context_invoke_full (self->context, G_PRIORITY_DEFAULT,
       gst_player_stop_internal, self, NULL);
 }

diff --git a/tests/check/libs/player.c b/tests/check/libs/player.c
index a63ed99..52a7ca0 100644 (file)
--- a/tests/check/libs/player.c
+++ b/tests/check/libs/player.c
@@ -179,6 +179,7 @@ struct _TestPlayerState
   gint width, height;
   GstPlayerMediaInfo *media_info;
   gchar *uri_loaded;
+  gboolean stopping;
 
   void (*test_callback) (GstPlayer * player, TestPlayerStateChange change,
       TestPlayerState * old_state, TestPlayerState * new_state);
@@ -226,6 +227,7 @@ test_player_state_reset (TestPlayerState * state)
   state->state = GST_PLAYER_STATE_STOPPED;
   state->width = state->height = 0;
   state->media_info = NULL;
+  state->stopping = FALSE;
   g_clear_pointer (&state->uri_loaded, g_free);
 }
 
@@ -234,6 +236,8 @@ buffering_cb (GstPlayer * player, gint percent, TestPlayerState * state)
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->buffering_percent = percent;
   test_player_state_change_debug (player, STATE_CHANGE_BUFFERING, &old_state,
       state);
@@ -246,6 +250,8 @@ duration_changed_cb (GstPlayer * player, guint64 duration,
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->duration = duration;
   test_player_state_change_debug (player, STATE_CHANGE_DURATION_CHANGED,
       &old_state, state);
@@ -258,6 +264,8 @@ end_of_stream_cb (GstPlayer * player, TestPlayerState * state)
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->end_of_stream = TRUE;
   test_player_state_change_debug (player, STATE_CHANGE_END_OF_STREAM,
       &old_state, state);
@@ -269,6 +277,8 @@ error_cb (GstPlayer * player, GError * error, TestPlayerState * state)
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->error = TRUE;
   test_player_state_change_debug (player, STATE_CHANGE_ERROR, &old_state,
       state);
@@ -280,6 +290,8 @@ warning_cb (GstPlayer * player, GError * error, TestPlayerState * state)
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->warning = TRUE;
   test_player_state_change_debug (player, STATE_CHANGE_WARNING, &old_state,
       state);
@@ -292,6 +304,8 @@ position_updated_cb (GstPlayer * player, guint64 position,
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->position = position;
   test_player_state_change_debug (player, STATE_CHANGE_POSITION_UPDATED,
       &old_state, state);
@@ -305,6 +319,8 @@ media_info_updated_cb (GstPlayer * player, GstPlayerMediaInfo * media_info,
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->media_info = media_info;
 
   test_player_state_change_debug (player, STATE_CHANGE_MEDIA_INFO_UPDATED,
@@ -319,6 +335,8 @@ state_changed_cb (GstPlayer * player, GstPlayerState player_state,
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping || player_state == GST_PLAYER_STATE_STOPPED);
+
   state->state = player_state;
 
   if (player_state == GST_PLAYER_STATE_STOPPED)
@@ -335,6 +353,8 @@ video_dimensions_changed_cb (GstPlayer * player, gint width, gint height,
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->width = width;
   state->height = height;
   test_player_state_change_debug (player, STATE_CHANGE_VIDEO_DIMENSIONS_CHANGED,
@@ -348,6 +368,8 @@ seek_done_cb (GstPlayer * player, guint64 position, TestPlayerState * state)
 {
   TestPlayerState old_state = *state;
 
+  g_assert (!state->stopping);
+
   state->seek_done = TRUE;
   state->seek_done_position = position;
   test_player_state_change_debug (player, STATE_CHANGE_SEEK_DONE,
@@ -408,6 +430,28 @@ test_player_new (TestPlayerState * state)
 }
 
 static void
+test_player_stopped_cb (GstPlayer * player, TestPlayerStateChange change,
+    TestPlayerState * old_state, TestPlayerState * new_state)
+{
+  if (new_state->state == GST_PLAYER_STATE_STOPPED) {
+    g_main_loop_quit (new_state->loop);
+  }
+}
+
+static void
+stop_player (GstPlayer * player, TestPlayerState * state)
+{
+  if (state->state != GST_PLAYER_STATE_STOPPED) {
+    /* Make sure all pending operations are finished so the player won't be
+     * appear as 'leaked' to leak detection tools. */
+    state->test_callback = test_player_stopped_cb;
+    gst_player_stop (player);
+    state->stopping = TRUE;
+    g_main_loop_run (state->loop);
+  }
+}
+
+static void
 test_play_audio_video_eos_cb (GstPlayer * player, TestPlayerStateChange change,
     TestPlayerState * old_state, TestPlayerState * new_state)
 {
@@ -522,6 +566,7 @@ START_TEST (test_play_audio_eos)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 9);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -700,6 +745,7 @@ START_TEST (test_play_media_info)
   g_main_loop_run (state.loop);
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 1);
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -792,6 +838,7 @@ START_TEST (test_play_stream_disable)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 0x33);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -856,6 +903,7 @@ START_TEST (test_play_stream_switch_audio)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 2);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -920,6 +968,7 @@ START_TEST (test_play_stream_switch_subtitle)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 2);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -951,6 +1000,7 @@ START_TEST (test_play_error_invalid_external_suburi)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 2);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1029,6 +1079,7 @@ START_TEST (test_play_external_suburi)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 2);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1100,6 +1151,7 @@ START_TEST (test_play_forward_rate)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data) & 0xf, 10);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1131,6 +1183,7 @@ START_TEST (test_play_backward_rate)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data) & 0xf, 10);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1162,6 +1215,7 @@ START_TEST (test_play_audio_video_eos)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data) & (~0x10), 9);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1225,6 +1279,7 @@ START_TEST (test_play_error_invalid_uri)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 4);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1334,6 +1389,7 @@ START_TEST (test_play_error_invalid_uri_and_play)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 11);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1384,6 +1440,7 @@ START_TEST (test_play_audio_video_seek_done)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data) & (~0x10), 2);
 
+  stop_player (player, &state);
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1468,6 +1525,87 @@ START_TEST (test_play_position_update_interval)
 
   fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 5);
 
+  stop_player (player, &state);
+  g_object_unref (player);
+  g_main_loop_unref (state.loop);
+}
+
+END_TEST;
+
+static void
+test_restart_cb (GstPlayer * player,
+    TestPlayerStateChange change, TestPlayerState * old_state,
+    TestPlayerState * new_state)
+{
+  gint steps = GPOINTER_TO_INT (new_state->test_data);
+
+  if (!steps && change == STATE_CHANGE_URI_LOADED) {
+    fail_unless (g_str_has_suffix (new_state->uri_loaded, "sintel.mkv"));
+    new_state->test_data = GINT_TO_POINTER (steps + 1);
+  } else if (change == STATE_CHANGE_STATE_CHANGED
+      && new_state->state == GST_PLAYER_STATE_BUFFERING) {
+    new_state->test_data = GINT_TO_POINTER (steps + 1);
+    g_main_loop_quit (new_state->loop);
+  }
+}
+
+static void
+test_restart_cb2 (GstPlayer * player,
+    TestPlayerStateChange change, TestPlayerState * old_state,
+    TestPlayerState * new_state)
+{
+  gint steps = GPOINTER_TO_INT (new_state->test_data);
+
+  if (!steps && change == STATE_CHANGE_URI_LOADED) {
+    fail_unless (g_str_has_suffix (new_state->uri_loaded, "audio-short.ogg"));
+    new_state->test_data = GINT_TO_POINTER (steps + 1);
+  } else if (change == STATE_CHANGE_STATE_CHANGED
+      && new_state->state == GST_PLAYER_STATE_BUFFERING) {
+    new_state->test_data = GINT_TO_POINTER (steps + 1);
+    g_main_loop_quit (new_state->loop);
+  }
+}
+
+
+START_TEST (test_restart)
+{
+  GstPlayer *player;
+  TestPlayerState state;
+  gchar *uri;
+
+  memset (&state, 0, sizeof (state));
+  state.loop = g_main_loop_new (NULL, FALSE);
+  state.test_callback = test_restart_cb;
+  state.test_data = GINT_TO_POINTER (0);
+
+  player = test_player_new (&state);
+
+  fail_unless (player != NULL);
+
+  uri = gst_filename_to_uri (TEST_PATH "/sintel.mkv", NULL);
+  fail_unless (uri != NULL);
+  gst_player_set_uri (player, uri);
+  g_free (uri);
+
+  gst_player_play (player);
+  g_main_loop_run (state.loop);
+  fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 2);
+  stop_player (player, &state);
+
+  /* Try again with another URI */
+  state.test_data = GINT_TO_POINTER (0);
+  state.test_callback = test_restart_cb2;
+
+  uri = gst_filename_to_uri (TEST_PATH "/audio-short.ogg", NULL);
+  fail_unless (uri != NULL);
+  gst_player_set_uri (player, uri);
+  g_free (uri);
+
+  gst_player_play (player);
+  g_main_loop_run (state.loop);
+  fail_unless_equals_int (GPOINTER_TO_INT (state.test_data), 2);
+  stop_player (player, &state);
+
   g_object_unref (player);
   g_main_loop_unref (state.loop);
 }
@@ -1515,6 +1653,7 @@ player_suite (void)
   tcase_add_test (tc_general, test_play_forward_rate);
   tcase_add_test (tc_general, test_play_backward_rate);
   tcase_add_test (tc_general, test_play_audio_video_seek_done);
+  tcase_add_test (tc_general, test_restart);
 
   suite_add_tcase (s, tc_general);