Skip to content
Permalink
Browse files
REGRESSION(r274527): [GStreamer] media/webaudio-background-playback.h…
…tml now failing

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

Patch by Philippe Normand <pnormand@igalia.com> on 2021-03-28
Reviewed by Chris Dumez.

r274527 actually exposed a bug that was present since r271197. The AudioDestination pipeline
was not stopping properly because webkitGstSetElementStateSynchronously() was returning too
early.

This patch also includes a few improvements in the webaudiosrc element, most notably
regarding its preroll state. The element now stops emitting buffers downstream as soon as
the AudioDestination is scheduled to stop.

* platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:
(webKitWebAudioSrcRenderAndPushFrames): Return early as soon as the destination is scheduled to stop.
(webKitWebAudioSrcChangeState): Synchronize preroll state with element state.
* platform/graphics/gstreamer/GStreamerCommon.cpp:
(WebCore::webkitGstSetElementStateSynchronously): targetState might be lower than current
state, e.g, when stopping a running pipeline, so we can't return early for those cases,
otherwise the pipeline won't stop.

Canonical link: https://commits.webkit.org/235852@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275149 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
philn authored and webkit-commit-queue committed Mar 28, 2021
1 parent 6f4b134 commit 3c5902db9e02535684ebe8aa8a346eb825de5595
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
@@ -1,3 +1,26 @@
2021-03-28 Philippe Normand <pnormand@igalia.com>

REGRESSION(r274527): [GStreamer] media/webaudio-background-playback.html now failing
https://bugs.webkit.org/show_bug.cgi?id=223840

Reviewed by Chris Dumez.

r274527 actually exposed a bug that was present since r271197. The AudioDestination pipeline
was not stopping properly because webkitGstSetElementStateSynchronously() was returning too
early.

This patch also includes a few improvements in the webaudiosrc element, most notably
regarding its preroll state. The element now stops emitting buffers downstream as soon as
the AudioDestination is scheduled to stop.

* platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:
(webKitWebAudioSrcRenderAndPushFrames): Return early as soon as the destination is scheduled to stop.
(webKitWebAudioSrcChangeState): Synchronize preroll state with element state.
* platform/graphics/gstreamer/GStreamerCommon.cpp:
(WebCore::webkitGstSetElementStateSynchronously): targetState might be lower than current
state, e.g, when stopping a running pipeline, so we can't return early for those cases,
otherwise the pipeline won't stop.

2021-03-28 Chris Dumez <cdumez@apple.com>

Unreviewed, add an exception for a heap allocation on the WebAudio thread.
@@ -33,6 +33,7 @@
#include <gst/pbutils/missing-plugins.h>
#include <wtf/Condition.h>
#include <wtf/Lock.h>
#include <wtf/Scope.h>
#include <wtf/glib/GUniquePtr.h>
#include <wtf/glib/WTFGType.h>

@@ -337,6 +338,16 @@ static void webKitWebAudioSrcRenderAndPushFrames(GRefPtr<GstElement>&& element,
auto* src = WEBKIT_WEB_AUDIO_SRC(element.get());
auto* priv = src->priv;

auto notifyDispatchOnExit = makeScopeExit([priv] {
LockHolder lock(priv->dispatchLock);
priv->dispatchDone = true;
priv->dispatchCondition.notifyOne();
});

GST_TRACE_OBJECT(element.get(), "Playing: %d", priv->destination->isPlaying());
if (priv->hasRenderedAudibleFrame && !priv->destination->isPlaying())
return;

ASSERT(channelBufferList.size() == priv->sources.size());

GstClockTime timestamp = gst_util_uint64_scale(priv->numberOfSamples, GST_SECOND, priv->sampleRate);
@@ -381,12 +392,6 @@ static void webKitWebAudioSrcRenderAndPushFrames(GRefPtr<GstElement>&& element,
failed = true;
}
}

{
LockHolder lock(priv->dispatchLock);
priv->dispatchDone = true;
priv->dispatchCondition.notifyOne();
}
}

static void webKitWebAudioSrcRenderIteration(WebKitWebAudioSrc* src)
@@ -473,6 +478,7 @@ static GstStateChangeReturn webKitWebAudioSrcChangeState(GstElement* element, Gs

gst_buffer_pool_set_active(priv->pool.get(), FALSE);
priv->pool = nullptr;
priv->hasRenderedAudibleFrame = false;
break;
default:
break;
@@ -468,7 +468,7 @@ bool webkitGstSetElementStateSynchronously(GstElement* pipeline, GstState target

GstState currentState;
auto result = gst_element_get_state(pipeline, &currentState, nullptr, 10);
if (result == GST_STATE_CHANGE_SUCCESS && currentState >= targetState) {
if (result == GST_STATE_CHANGE_SUCCESS && currentState == targetState) {
GST_DEBUG_OBJECT(pipeline, "Target state already reached");
return true;
}

0 comments on commit 3c5902d

Please sign in to comment.