playsink: Hold a reference to the soft volume element

Always hold a reference to the soft volume element
provided by the playsinkaudioconvert bin helper, the
same as when volume is provided by a sink element,
or the soft volume element gets unreffed too soon.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3108>

diff --git a/subprojects/gst-plugins-base/gst/playback/gstplaysink.c b/subprojects/gst-plugins-base/gst/playback/gstplaysink.c
index 9bed98d..a5619e9 100644 (file)
--- a/subprojects/gst-plugins-base/gst/playback/gstplaysink.c
+++ b/subprojects/gst-plugins-base/gst/playback/gstplaysink.c
@@ -2873,11 +2873,9 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw)
     prev = chain->conv;
 
     if (!have_volume && (playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME)) {
-      GstPlaySinkAudioConvert *conv =
-          GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv);
+      g_object_get (chain->conv, "volume-element", &chain->volume, NULL);
 
-      if (conv->volume) {
-        chain->volume = conv->volume;
+      if (chain->volume) {
         have_volume = TRUE;
 
         chain->notify_volume_id =
@@ -2988,10 +2986,8 @@ setup_audio_chain (GstPlaySink * playsink, gboolean raw)
   GstElement *elem;
   GstPlayAudioChain *chain;
   GstStateChangeReturn ret;
-  GstPlaySinkAudioConvert *conv;
 
   chain = playsink->audiochain;
-  conv = GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv);
 
   /* if we have a filter, and raw-ness changed, we have to force a rebuild */
   if (chain->filter && chain->chain.raw != raw)
@@ -3015,6 +3011,11 @@ setup_audio_chain (GstPlaySink * playsink, gboolean raw)
 
   /* Disconnect signals */
   disconnect_audio_chain (chain, playsink);
+  /* Drop any existing volume handler and check again */
+  if (chain->volume) {
+    gst_object_unref (chain->volume);
+    playsink->audiochain->volume = NULL;
+  }
 
   /* check if the sink, or something within the sink, implements the
    * streamvolume interface. If it does we don't need to add a volume element.  */
@@ -3045,28 +3046,28 @@ setup_audio_chain (GstPlaySink * playsink, gboolean raw)
     playsink->mute_changed = FALSE;
 
     g_object_set (chain->conv, "use-volume", FALSE, NULL);
-  } else if (conv) {
+  } else if (chain->conv) {
     /* no volume, we need to add a volume element when we can */
     g_object_set (chain->conv, "use-volume",
         ! !(playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME), NULL);
     GST_DEBUG_OBJECT (playsink, "the sink has no volume property");
 
-    if (conv->volume && (playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME)) {
-      chain->volume = conv->volume;
-
-      chain->notify_volume_id =
-          g_signal_connect (chain->volume, "notify::volume",
-          G_CALLBACK (notify_volume_cb), playsink);
+    if (playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME) {
+      g_object_get (chain->conv, "volume-element", &chain->volume, NULL);
+      if (chain->volume) {
+        chain->notify_volume_id =
+            g_signal_connect (chain->volume, "notify::volume",
+            G_CALLBACK (notify_volume_cb), playsink);
 
-      chain->notify_mute_id = g_signal_connect (chain->volume, "notify::mute",
-          G_CALLBACK (notify_mute_cb), playsink);
+        chain->notify_mute_id = g_signal_connect (chain->volume, "notify::mute",
+            G_CALLBACK (notify_mute_cb), playsink);
 
-      /* configure with the latest volume and mute */
-      g_object_set (G_OBJECT (chain->volume), "volume", playsink->volume, NULL);
-      g_object_set (G_OBJECT (chain->volume), "mute", playsink->mute, NULL);
+        /* configure with the latest volume and mute */
+        g_object_set (G_OBJECT (chain->volume), "volume", playsink->volume,
+            NULL);
+        g_object_set (G_OBJECT (chain->volume), "mute", playsink->mute, NULL);
+      }
     }
-
-    GST_DEBUG_OBJECT (playsink, "reusing existing volume element");
   }
   return TRUE;
 }
@@ -4996,6 +4997,7 @@ gst_play_sink_change_state (GstElement * element, GstStateChange transition)
       GST_PLAY_SINK_UNLOCK (playsink);
       /* fall through */
     case GST_STATE_CHANGE_READY_TO_NULL:
+      GST_PLAY_SINK_LOCK (playsink);
       if (playsink->audiochain && playsink->audiochain->sink_volume) {
         /* remove our links to the volume elements when they were
          * provided by a sink */
@@ -5014,6 +5016,7 @@ gst_play_sink_change_state (GstElement * element, GstStateChange transition)
         gst_object_unref (playsink->videochain->ts_offset);
         playsink->videochain->ts_offset = NULL;
       }
+      GST_PLAY_SINK_UNLOCK (playsink);
 
       GST_OBJECT_LOCK (playsink);
       if (playsink->overlay_element)

diff --git a/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.c b/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.c
index 0eeea55..585ea50 100644 (file)
--- a/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.c
+++ b/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.c
@@ -39,6 +39,7 @@ enum
   PROP_0,
   PROP_USE_CONVERTERS,
   PROP_USE_VOLUME,
+  PROP_VOLUME_ELEMENT,
 };
 
 static gboolean
@@ -153,6 +154,9 @@ gst_play_sink_audio_convert_get_property (GObject * object, guint prop_id,
     case PROP_USE_VOLUME:
       g_value_set_boolean (value, self->use_volume);
       break;
+    case PROP_VOLUME_ELEMENT:
+      g_value_set_object (value, self->volume);
+      break;
     default:
       break;
   }
@@ -185,6 +189,11 @@ gst_play_sink_audio_convert_class_init (GstPlaySinkAudioConvertClass * klass)
           "Whether to use a volume element", FALSE,
           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
 
+  g_object_class_install_property (gobject_class, PROP_VOLUME_ELEMENT,
+      g_param_spec_object ("volume-element", "Volume element",
+          "Retrieve the soft-volume element used when use-volume=TRUE",
+          GST_TYPE_ELEMENT, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
+
   gst_element_class_set_static_metadata (gstelement_class,
       "Player Sink Audio Converter", "Audio/Bin/Converter",
       "Convenience bin for audio conversion",

diff --git a/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.h b/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.h
index 18a776e..4250478 100644 (file)
--- a/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.h
+++ b/subprojects/gst-plugins-base/gst/playback/gstplaysinkaudioconvert.h
@@ -43,7 +43,7 @@ struct _GstPlaySinkAudioConvert
 {
   GstPlaySinkConvertBin parent;
 
-  /* < pseudo public > */
+  /* < private > */
   GstElement *volume;
   gboolean use_converters;
   gboolean use_volume;