Skip to content
Permalink
Browse files
REGRESSION(r288092): Deadlock when the playback is stopping just righ…
…t after startup

https://bugs.webkit.org/show_bug.cgi?id=240661

Patch by Yacine Bandou <yacine.bandou@softathome.com> on 2022-05-20
Reviewed by Philippe Normand.

r288092 causes a deadlock with the "28. ChangeType.H264.VP9" test in "https://ytlr-cert.appspot.com/2020/main.html".

The background thread locks the GStPad and sends a notify::caps signal which is caught by
VideoTrackPrivateGStreamer that calls callOnMainThreadAndWait.

At the same time the main thread destroys MediaPlayerPrivateGStreamer and sets the pipeline to NULL
that calls "gst_pad_set_active" which wants to lock the GStPad.

This commit uses AbortableTaskQueue and calls enqueueTask instead of callOnMainThreadAndWait
in order to be able to abort the waiting job and avoid using the "AndWait" variant, consequently
we avoid the deadlock.

* platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp:
(WebCore::AudioTrackPrivateGStreamer::AudioTrackPrivateGStreamer):
(WebCore::AudioTrackPrivateGStreamer::updateConfigurationFromTags):
(WebCore::AudioTrackPrivateGStreamer::updateConfigurationFromCaps):
(WebCore::AudioTrackPrivateGStreamer::disconnect):
* platform/graphics/gstreamer/AudioTrackPrivateGStreamer.h:
* platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:
(WebCore::VideoTrackPrivateGStreamer::VideoTrackPrivateGStreamer):
(WebCore::VideoTrackPrivateGStreamer::updateConfigurationFromTags):
(WebCore::VideoTrackPrivateGStreamer::updateConfigurationFromCaps):
(WebCore::VideoTrackPrivateGStreamer::disconnect):
* platform/graphics/gstreamer/VideoTrackPrivateGStreamer.h:

Canonical link: https://commits.webkit.org/250792@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294537 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
ybandou authored and webkit-commit-queue committed May 20, 2022
1 parent 45e9179 commit 431e588d92dd86aafa9a951118dfd4cbde81a47e
Showing 4 changed files with 32 additions and 16 deletions.
@@ -53,10 +53,14 @@ AudioTrackPrivateGStreamer::AudioTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivat
}

g_signal_connect_swapped(m_stream, "notify::caps", G_CALLBACK(+[](AudioTrackPrivateGStreamer* track) {
track->updateConfigurationFromCaps();
track->m_taskQueue.enqueueTask([track]() {
track->updateConfigurationFromCaps();
});
}), this);
g_signal_connect_swapped(m_stream, "notify::tags", G_CALLBACK(+[](AudioTrackPrivateGStreamer* track) {
track->updateConfigurationFromTags();
track->m_taskQueue.enqueueTask([track]() {
track->updateConfigurationFromTags();
});
}), this);

updateConfigurationFromCaps();
@@ -65,20 +69,20 @@ AudioTrackPrivateGStreamer::AudioTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivat

void AudioTrackPrivateGStreamer::updateConfigurationFromTags()
{
ASSERT(isMainThread());
auto tags = adoptGRef(gst_stream_get_tags(m_stream));
unsigned bitrate;
if (!tags || !gst_tag_list_get_uint(tags.get(), GST_TAG_BITRATE, &bitrate))
return;

auto configuration = this->configuration();
configuration.bitrate = bitrate;
callOnMainThreadAndWait([&] {
setConfiguration(WTFMove(configuration));
});
setConfiguration(WTFMove(configuration));
}

void AudioTrackPrivateGStreamer::updateConfigurationFromCaps()
{
ASSERT(isMainThread());
auto caps = adoptGRef(gst_stream_get_caps(m_stream));
if (!caps || !gst_caps_is_fixed(caps.get()))
return;
@@ -95,9 +99,7 @@ void AudioTrackPrivateGStreamer::updateConfigurationFromCaps()
configuration.codec = String::fromLatin1(codec.get());
#endif

callOnMainThreadAndWait([&] {
setConfiguration(WTFMove(configuration));
});
setConfiguration(WTFMove(configuration));
}

AudioTrackPrivate::Kind AudioTrackPrivateGStreamer::kind() const
@@ -110,11 +112,15 @@ AudioTrackPrivate::Kind AudioTrackPrivateGStreamer::kind() const

void AudioTrackPrivateGStreamer::disconnect()
{
m_taskQueue.startAborting();

if (m_stream)
g_signal_handlers_disconnect_matched(m_stream, G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);

m_player = nullptr;
TrackPrivateBaseGStreamer::disconnect();

m_taskQueue.finishAborting();
}

void AudioTrackPrivateGStreamer::setEnabled(bool enabled)
@@ -27,6 +27,7 @@

#if ENABLE(VIDEO) && USE(GSTREAMER)

#include "AbortableTaskQueue.h"
#include "AudioTrackPrivate.h"
#include "TrackPrivateBaseGStreamer.h"

@@ -59,6 +60,7 @@ class AudioTrackPrivateGStreamer final : public AudioTrackPrivate, public TrackP
AtomString id() const final { return m_id; }
AtomString label() const final { return m_label; }
AtomString language() const final { return m_language; }
AbortableTaskQueue m_taskQueue;

protected:
void updateConfigurationFromCaps();
@@ -53,10 +53,14 @@ VideoTrackPrivateGStreamer::VideoTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivat
}

g_signal_connect_swapped(m_stream, "notify::caps", G_CALLBACK(+[](VideoTrackPrivateGStreamer* track) {
track->updateConfigurationFromCaps();
track->m_taskQueue.enqueueTask([track]() {
track->updateConfigurationFromCaps();
});
}), this);
g_signal_connect_swapped(m_stream, "notify::tags", G_CALLBACK(+[](VideoTrackPrivateGStreamer* track) {
track->updateConfigurationFromTags();
track->m_taskQueue.enqueueTask([track]() {
track->updateConfigurationFromTags();
});
}), this);

updateConfigurationFromCaps();
@@ -65,20 +69,20 @@ VideoTrackPrivateGStreamer::VideoTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivat

void VideoTrackPrivateGStreamer::updateConfigurationFromTags()
{
ASSERT(isMainThread());
auto tags = adoptGRef(gst_stream_get_tags(m_stream));
unsigned bitrate;
if (!tags || !gst_tag_list_get_uint(tags.get(), GST_TAG_BITRATE, &bitrate))
return;

auto configuration = this->configuration();
configuration.bitrate = bitrate;
callOnMainThreadAndWait([&] {
setConfiguration(WTFMove(configuration));
});
setConfiguration(WTFMove(configuration));
}

void VideoTrackPrivateGStreamer::updateConfigurationFromCaps()
{
ASSERT(isMainThread());
auto caps = adoptGRef(gst_stream_get_caps(m_stream));
if (!caps || !gst_caps_is_fixed(caps.get()))
return;
@@ -158,9 +162,7 @@ void VideoTrackPrivateGStreamer::updateConfigurationFromCaps()
configuration.codec = String::fromLatin1(codec.get());
#endif

callOnMainThreadAndWait([&] {
setConfiguration(WTFMove(configuration));
});
setConfiguration(WTFMove(configuration));
}

VideoTrackPrivate::Kind VideoTrackPrivateGStreamer::kind() const
@@ -173,11 +175,15 @@ VideoTrackPrivate::Kind VideoTrackPrivateGStreamer::kind() const

void VideoTrackPrivateGStreamer::disconnect()
{
m_taskQueue.startAborting();

if (m_stream)
g_signal_handlers_disconnect_matched(m_stream, G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);

m_player = nullptr;
TrackPrivateBaseGStreamer::disconnect();

m_taskQueue.finishAborting();
}

void VideoTrackPrivateGStreamer::setSelected(bool selected)
@@ -27,6 +27,7 @@

#if ENABLE(VIDEO) && USE(GSTREAMER)

#include "AbortableTaskQueue.h"
#include "GStreamerCommon.h"
#include "TrackPrivateBaseGStreamer.h"
#include "VideoTrackPrivate.h"
@@ -60,6 +61,7 @@ class VideoTrackPrivateGStreamer final : public VideoTrackPrivate, public TrackP
AtomString id() const final { return m_id; }
AtomString label() const final { return m_label; }
AtomString language() const final { return m_language; }
AbortableTaskQueue m_taskQueue;

protected:
void updateConfigurationFromCaps();

0 comments on commit 431e588

Please sign in to comment.