Title: [270184] trunk/Source/WebCore
Revision
270184
Author
ph...@webkit.org
Date
2020-11-27 01:08:39 -0800 (Fri, 27 Nov 2020)

Log Message

[GStreamer] AudioSourceProvider can potentially invoke an already-freed client
https://bugs.webkit.org/show_bug.cgi?id=217952

Reviewed by Xabier Rodriguez-Calvar.

* platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
(WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured): Check the provider has
a client before setting up the audio format.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (270183 => 270184)


--- trunk/Source/WebCore/ChangeLog	2020-11-27 08:51:45 UTC (rev 270183)
+++ trunk/Source/WebCore/ChangeLog	2020-11-27 09:08:39 UTC (rev 270184)
@@ -1,3 +1,14 @@
+2020-11-27  Philippe Normand  <pnorm...@igalia.com>
+
+        [GStreamer] AudioSourceProvider can potentially invoke an already-freed client
+        https://bugs.webkit.org/show_bug.cgi?id=217952
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
+        (WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured): Check the provider has
+        a client before setting up the audio format.
+
 2020-11-26  Lauro Moura  <lmo...@igalia.com>
 
         [GTK4] Build fix. Add cast when taking const data ownership

Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp (270183 => 270184)


--- trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp	2020-11-27 08:51:45 UTC (rev 270183)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp	2020-11-27 09:08:39 UTC (rev 270184)
@@ -108,7 +108,7 @@
 {
     m_notifier->invalidate();
 
-    GRefPtr<GstElement> deinterleave = adoptGRef(gst_bin_get_by_name(GST_BIN(m_audioSinkBin.get()), "deinterleave"));
+    auto deinterleave = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "deinterleave"));
     if (deinterleave && m_client) {
         g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadAddedHandlerId);
         g_signal_handler_disconnect(deinterleave.get(), m_deinterleaveNoMorePadsHandlerId);
@@ -115,10 +115,7 @@
         g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadRemovedHandlerId);
     }
 
-#if ENABLE(MEDIA_STREAM)
-    if (m_pipeline)
-        gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
-#endif
+    setClient(nullptr);
 }
 
 void AudioSourceProviderGStreamer::configureAudioBin(GstElement* audioBin, GstElement* audioSink)
@@ -189,22 +186,26 @@
 
 void AudioSourceProviderGStreamer::setClient(AudioSourceProviderClient* client)
 {
-    if (m_client)
+    if (m_client == client)
         return;
 
-    ASSERT(client);
     m_client = client;
 
 #if ENABLE(MEDIA_STREAM)
     if (m_pipeline)
-        gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING);
+        gst_element_set_state(m_pipeline.get(), m_client ? GST_STATE_PLAYING : GST_STATE_NULL);
 #endif
 
+    // FIXME: This early return should ideally be replaced by a removal of the m_audioSinkBin from
+    // its parent pipeline. https://bugs.webkit.org/show_bug.cgi?id=219245
+    if (!m_client)
+        return;
+
     // The volume element is used to mute audio playback towards the
     // autoaudiosink. This is needed to avoid double playback of audio
     // from our audio sink and from the WebAudio AudioDestination node
     // supposedly configured already by application side.
-    GRefPtr<GstElement> volumeElement = adoptGRef(gst_bin_get_by_name(GST_BIN(m_audioSinkBin.get()), "volume"));
+    auto volumeElement = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "volume"));
 
     if (volumeElement)
         g_object_set(volumeElement.get(), "mute", TRUE, nullptr);
@@ -320,9 +321,9 @@
 void AudioSourceProviderGStreamer::deinterleavePadsConfigured()
 {
     GST_DEBUG("Deinterleave configured, notifying client");
-    m_notifier->notify(MainThreadNotification::DeinterleavePadsConfigured, [this] {
-        ASSERT(m_client);
-        m_client->setFormat(m_deinterleaveSourcePads, gSampleBitRate);
+    m_notifier->notify(MainThreadNotification::DeinterleavePadsConfigured, [numberOfChannels = m_deinterleaveSourcePads, sampleRate = gSampleBitRate, client = m_client] {
+        if (client)
+            client->setFormat(numberOfChannels, sampleRate);
     });
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to