Skip to content

Commit

Permalink
Merge r270184 - [GStreamer] AudioSourceProvider can potentially invok…
Browse files Browse the repository at this point in the history
…e 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.
  • Loading branch information
philn authored and carlosgcampos committed Feb 11, 2021
1 parent dfff805 commit 64fc76b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
11 changes: 11 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,14 @@
2020-11-27 Philippe Normand <pnormand@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 Michael Catanzaro <mcatanzaro@gnome.org>

[WPE][GTK] Use Internet Explorer quirk for Google Docs
Expand Down
Expand Up @@ -127,15 +127,14 @@ AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer()
{
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);
g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadRemovedHandlerId);
}

if (m_pipeline)
gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
setClient(nullptr);

g_object_unref(m_frontLeftAdapter);
g_object_unref(m_frontRightAdapter);
Expand Down Expand Up @@ -231,20 +230,24 @@ GstFlowReturn AudioSourceProviderGStreamer::handleAudioBuffer(GstAppSink* sink)

void AudioSourceProviderGStreamer::setClient(AudioSourceProviderClient* client)
{
if (m_client)
if (m_client == client)
return;

ASSERT(client);
m_client = client;

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);

// 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);
Expand Down Expand Up @@ -375,11 +378,9 @@ void AudioSourceProviderGStreamer::handleRemovedDeinterleavePad(GstPad* pad)

void AudioSourceProviderGStreamer::deinterleavePadsConfigured()
{
m_notifier->notify(MainThreadNotification::DeinterleavePadsConfigured, [this] {
ASSERT(m_client);
ASSERT(m_deinterleaveSourcePads == gNumberOfChannels);

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);
});
}

Expand Down

0 comments on commit 64fc76b

Please sign in to comment.