|
@@ -0,0 +1,292 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Raymond Toy <[email protected]>
|
|
|
+Date: Tue, 29 Sep 2020 17:56:16 +0000
|
|
|
+Subject: Add mutex for allowing the graph to be pulled
|
|
|
+
|
|
|
+Basically add a mutex to protect access to
|
|
|
+allow_pulling_audio_graph_. The main thread waits until it has the
|
|
|
+lock before setting this. This prevents the main thread from deleting
|
|
|
+things while the audio thread is pulling on the graph.
|
|
|
+
|
|
|
+A try lock is used so as not to block the audio thread if it can't get
|
|
|
+lock.
|
|
|
+
|
|
|
+This is applied to both real time and offline contexts which required
|
|
|
+moving the original real-time-only implementation to
|
|
|
+audio_destination_node so we can use the same methods for the offline
|
|
|
+context.
|
|
|
+
|
|
|
+Tested the repro case from 1125635, and the issue does not reproduce.
|
|
|
+We're assuming this will fix 1115901, but I've not been able to
|
|
|
+reproduce that locally.
|
|
|
+
|
|
|
+(cherry picked from commit 6785a406fd652f7a2f40933620ef4c979643c56b)
|
|
|
+
|
|
|
+Bug: 1125635, 1115901
|
|
|
+Change-Id: I1037d8c44225c6dcc8fe906c29a5a86740a15e1d
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410743
|
|
|
+Reviewed-by: Hongchan Choi <[email protected]>
|
|
|
+Commit-Queue: Raymond Toy <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/heads/master@{#809393}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436324
|
|
|
+Reviewed-by: Raymond Toy <[email protected]>
|
|
|
+Cr-Commit-Position: refs/branch-heads/4240@{#1083}
|
|
|
+Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
|
|
|
+
|
|
|
+diff --git a/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc b/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
|
|
|
+index c85f6d2e26f2e34b8633c29754028afbd8f2cbb9..9b1a5ec495f8f3614e8e77fdeaf9eaa159b3e3ae 100644
|
|
|
+--- a/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
|
|
|
++++ b/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
|
|
|
+@@ -30,7 +30,8 @@
|
|
|
+ namespace blink {
|
|
|
+
|
|
|
+ AudioDestinationHandler::AudioDestinationHandler(AudioNode& node)
|
|
|
+- : AudioHandler(kNodeTypeDestination, node, 0) {
|
|
|
++ : AudioHandler(kNodeTypeDestination, node, 0),
|
|
|
++ allow_pulling_audio_graph_(false) {
|
|
|
+ AddInput();
|
|
|
+ }
|
|
|
+
|
|
|
+diff --git a/third_party/blink/renderer/modules/webaudio/audio_destination_node.h b/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
|
|
|
+index c35722c9296b6c98832444799124334d382225df..ce7bf61ef9ad18a5aa9669edb990542d2899acc2 100644
|
|
|
+--- a/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
|
|
|
++++ b/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
|
|
|
+@@ -71,6 +71,53 @@ class AudioDestinationHandler : public AudioHandler {
|
|
|
+ return is_execution_context_destroyed_;
|
|
|
+ }
|
|
|
+
|
|
|
++ // Should only be called from
|
|
|
++ // RealtimeAudioDestinationHandler::StartPlatformDestination for a realtime
|
|
|
++ // context or OfflineAudioDestinationHandler::StartRendering for an offline
|
|
|
++ // context---basically wherever the context has started rendering.
|
|
|
++ // TODO(crbug.com/1128121): Consider removing this if possible
|
|
|
++ void EnablePullingAudioGraph() {
|
|
|
++ MutexLocker lock(allow_pulling_audio_graph_mutex_);
|
|
|
++ allow_pulling_audio_graph_.store(true, std::memory_order_release);
|
|
|
++ }
|
|
|
++
|
|
|
++ // Should only be called from
|
|
|
++ // RealtimeAudioDestinationHandler::StopPlatformDestination for a realtime
|
|
|
++ // context or from OfflineAudioDestinationHandler::Uninitialize for an offline
|
|
|
++ // context---basically whenever the context is being stopped.
|
|
|
++ // TODO(crbug.com/1128121): Consider removing this if possible
|
|
|
++ void DisablePullingAudioGraph() {
|
|
|
++ MutexLocker lock(allow_pulling_audio_graph_mutex_);
|
|
|
++ allow_pulling_audio_graph_.store(false, std::memory_order_release);
|
|
|
++ }
|
|
|
++
|
|
|
++ // TODO(crbug.com/1128121): Consider removing this if possible
|
|
|
++ bool IsPullingAudioGraphAllowed() const {
|
|
|
++ return allow_pulling_audio_graph_.load(std::memory_order_acquire);
|
|
|
++ }
|
|
|
++
|
|
|
++ // If true, the audio graph will be pulled to get new data. Otherwise, the
|
|
|
++ // graph is not pulled, even if the audio thread is still running and
|
|
|
++ // requesting data.
|
|
|
++ //
|
|
|
++ // For an AudioContext, this MUST be modified only in
|
|
|
++ // RealtimeAudioDestinationHandler::StartPlatformDestination (via
|
|
|
++ // AudioDestinationHandler::EnablePullingAudioGraph) or
|
|
|
++ // RealtimeAudioDestinationHandler::StopPlatformDestination (via
|
|
|
++ // AudioDestinationHandler::DisablePullingAudioGraph), including destroying
|
|
|
++ // the the realtime context.
|
|
|
++ //
|
|
|
++ // For an OfflineAudioContext, this MUST be modified only when the the context
|
|
|
++ // is started or it has stopped rendering, including destroying the offline
|
|
|
++ // context.
|
|
|
++
|
|
|
++ // TODO(crbug.com/1128121): Consider removing this if possible
|
|
|
++ std::atomic_bool allow_pulling_audio_graph_;
|
|
|
++
|
|
|
++ // Protects allow_pulling_audio_graph_ from race conditions. Must use try
|
|
|
++ // lock on the audio thread.
|
|
|
++ mutable Mutex allow_pulling_audio_graph_mutex_;
|
|
|
++
|
|
|
+ protected:
|
|
|
+ void AdvanceCurrentSampleFrame(size_t number_of_frames) {
|
|
|
+ current_sample_frame_.fetch_add(number_of_frames,
|
|
|
+diff --git a/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc b/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
|
|
|
+index 2b0d87395cef31793273ef69f61eb540be897fcf..697a3e684e9bc582a93b20ac163ceb790dfb62cc 100644
|
|
|
+--- a/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
|
|
|
++++ b/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
|
|
|
+@@ -92,6 +92,7 @@ void OfflineAudioDestinationHandler::Uninitialize() {
|
|
|
+
|
|
|
+ render_thread_.reset();
|
|
|
+
|
|
|
++ DisablePullingAudioGraph();
|
|
|
+ AudioHandler::Uninitialize();
|
|
|
+ }
|
|
|
+
|
|
|
+@@ -111,6 +112,7 @@ void OfflineAudioDestinationHandler::StartRendering() {
|
|
|
+ // Rendering was not started. Starting now.
|
|
|
+ if (!is_rendering_started_) {
|
|
|
+ is_rendering_started_ = true;
|
|
|
++ EnablePullingAudioGraph();
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *render_thread_task_runner_, FROM_HERE,
|
|
|
+ CrossThreadBindOnce(
|
|
|
+@@ -292,20 +294,34 @@ bool OfflineAudioDestinationHandler::RenderIfNotSuspended(
|
|
|
+
|
|
|
+ DCHECK_GE(NumberOfInputs(), 1u);
|
|
|
+
|
|
|
+- // This will cause the node(s) connected to us to process, which in turn will
|
|
|
+- // pull on their input(s), all the way backwards through the rendering graph.
|
|
|
+- scoped_refptr<AudioBus> rendered_bus =
|
|
|
+- Input(0).Pull(destination_bus, number_of_frames);
|
|
|
++ {
|
|
|
++ // The entire block that relies on |IsPullingAudioGraphAllowed| needs
|
|
|
++ // locking to prevent pulling audio graph being disallowed (i.e. a
|
|
|
++ // destruction started) in the middle of processing
|
|
|
++ MutexTryLocker try_locker(allow_pulling_audio_graph_mutex_);
|
|
|
++
|
|
|
++ if (IsPullingAudioGraphAllowed() && try_locker.Locked()) {
|
|
|
++ // This will cause the node(s) connected to us to process, which in turn
|
|
|
++ // will pull on their input(s), all the way backwards through the
|
|
|
++ // rendering graph.
|
|
|
++ scoped_refptr<AudioBus> rendered_bus =
|
|
|
++ Input(0).Pull(destination_bus, number_of_frames);
|
|
|
++
|
|
|
++ if (!rendered_bus) {
|
|
|
++ destination_bus->Zero();
|
|
|
++ } else if (rendered_bus != destination_bus) {
|
|
|
++ // in-place processing was not possible - so copy
|
|
|
++ destination_bus->CopyFrom(*rendered_bus);
|
|
|
++ }
|
|
|
+
|
|
|
+- if (!rendered_bus) {
|
|
|
+- destination_bus->Zero();
|
|
|
+- } else if (rendered_bus != destination_bus) {
|
|
|
+- // in-place processing was not possible - so copy
|
|
|
+- destination_bus->CopyFrom(*rendered_bus);
|
|
|
++ } else {
|
|
|
++ // Not allowed to pull on the graph or couldn't get the lock.
|
|
|
++ destination_bus->Zero();
|
|
|
++ }
|
|
|
+ }
|
|
|
+
|
|
|
+- // Process nodes which need a little extra help because they are not connected
|
|
|
+- // to anything, but still need to process.
|
|
|
++ // Process nodes which need a little extra help because they are not
|
|
|
++ // connected to anything, but still need to process.
|
|
|
+ Context()->GetDeferredTaskHandler().ProcessAutomaticPullNodes(
|
|
|
+ number_of_frames);
|
|
|
+
|
|
|
+diff --git a/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc b/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc
|
|
|
+index 743cddb2d4bd7f04d8bdef7ce70241cbe2255430..dd183350f603d2c5997e89a62756a92c8ad566dc 100644
|
|
|
+--- a/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc
|
|
|
++++ b/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc
|
|
|
+@@ -52,8 +52,7 @@ RealtimeAudioDestinationHandler::RealtimeAudioDestinationHandler(
|
|
|
+ base::Optional<float> sample_rate)
|
|
|
+ : AudioDestinationHandler(node),
|
|
|
+ latency_hint_(latency_hint),
|
|
|
+- sample_rate_(sample_rate),
|
|
|
+- allow_pulling_audio_graph_(false) {
|
|
|
++ sample_rate_(sample_rate) {
|
|
|
+ // Node-specific default channel count and mixing rules.
|
|
|
+ channel_count_ = 2;
|
|
|
+ SetInternalChannelCountMode(kExplicit);
|
|
|
+@@ -198,28 +197,38 @@ void RealtimeAudioDestinationHandler::Render(
|
|
|
+ // Only pull on the audio graph if we have not stopped the destination. It
|
|
|
+ // takes time for the destination to stop, but we want to stop pulling before
|
|
|
+ // the destination has actually stopped.
|
|
|
+- if (IsPullingAudioGraphAllowed()) {
|
|
|
+- // Renders the graph by pulling all the inputs to this node. This will in
|
|
|
+- // turn pull on their inputs, all the way backwards through the graph.
|
|
|
+- scoped_refptr<AudioBus> rendered_bus =
|
|
|
+- Input(0).Pull(destination_bus, number_of_frames);
|
|
|
+-
|
|
|
+- DCHECK(rendered_bus);
|
|
|
+- if (!rendered_bus) {
|
|
|
+- // AudioNodeInput might be in the middle of destruction. Then the internal
|
|
|
+- // summing bus will return as nullptr. Then zero out the output.
|
|
|
++ {
|
|
|
++ // The entire block that relies on |IsPullingAudioGraphAllowed| needs
|
|
|
++ // locking to prevent pulling audio graph being disallowed (i.e. a
|
|
|
++ // destruction started) in the middle of processing.
|
|
|
++ MutexTryLocker try_locker(allow_pulling_audio_graph_mutex_);
|
|
|
++
|
|
|
++ if (IsPullingAudioGraphAllowed() && try_locker.Locked()) {
|
|
|
++ // Renders the graph by pulling all the inputs to this node. This will in
|
|
|
++ // turn pull on their inputs, all the way backwards through the graph.
|
|
|
++ scoped_refptr<AudioBus> rendered_bus =
|
|
|
++ Input(0).Pull(destination_bus, number_of_frames);
|
|
|
++
|
|
|
++ DCHECK(rendered_bus);
|
|
|
++ if (!rendered_bus) {
|
|
|
++ // AudioNodeInput might be in the middle of destruction. Then the
|
|
|
++ // internal summing bus will return as nullptr. Then zero out the
|
|
|
++ // output.
|
|
|
++ destination_bus->Zero();
|
|
|
++ } else if (rendered_bus != destination_bus) {
|
|
|
++ // In-place processing was not possible. Copy the rendered result to the
|
|
|
++ // given |destination_bus| buffer.
|
|
|
++ destination_bus->CopyFrom(*rendered_bus);
|
|
|
++ }
|
|
|
++ } else {
|
|
|
++ // Not allowed to pull on the graph or couldn't get the lock.
|
|
|
+ destination_bus->Zero();
|
|
|
+- } else if (rendered_bus != destination_bus) {
|
|
|
+- // In-place processing was not possible. Copy the rendered result to the
|
|
|
+- // given |destination_bus| buffer.
|
|
|
+- destination_bus->CopyFrom(*rendered_bus);
|
|
|
+ }
|
|
|
+- } else {
|
|
|
+- destination_bus->Zero();
|
|
|
+ }
|
|
|
+
|
|
|
+- // Processes "automatic" nodes that are not connected to anything. This can
|
|
|
+- // be done after copying because it does not affect the rendered result.
|
|
|
++ // Processes "automatic" nodes that are not connected to anything. This
|
|
|
++ // can be done after copying because it does not affect the rendered
|
|
|
++ // result.
|
|
|
+ context->GetDeferredTaskHandler().ProcessAutomaticPullNodes(number_of_frames);
|
|
|
+
|
|
|
+ context->HandlePostRenderTasks();
|
|
|
+diff --git a/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h b/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h
|
|
|
+index 9c8653880c37a02c87b2e82558dd2940a1b9a425..89e33ec6a9eb2b60ad1c2d46dcb748415bf2be33 100644
|
|
|
+--- a/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h
|
|
|
++++ b/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h
|
|
|
+@@ -81,10 +81,6 @@ class RealtimeAudioDestinationHandler final : public AudioDestinationHandler,
|
|
|
+ // Returns a given frames-per-buffer size from audio infra.
|
|
|
+ int GetFramesPerBuffer() const;
|
|
|
+
|
|
|
+- bool IsPullingAudioGraphAllowed() const {
|
|
|
+- return allow_pulling_audio_graph_.load(std::memory_order_acquire);
|
|
|
+- }
|
|
|
+-
|
|
|
+ private:
|
|
|
+ explicit RealtimeAudioDestinationHandler(AudioNode&,
|
|
|
+ const WebAudioLatencyHint&,
|
|
|
+@@ -94,32 +90,12 @@ class RealtimeAudioDestinationHandler final : public AudioDestinationHandler,
|
|
|
+ void StartPlatformDestination();
|
|
|
+ void StopPlatformDestination();
|
|
|
+
|
|
|
+- // Should only be called from StartPlatformDestination.
|
|
|
+- void EnablePullingAudioGraph() {
|
|
|
+- allow_pulling_audio_graph_.store(true, std::memory_order_release);
|
|
|
+- }
|
|
|
+-
|
|
|
+- // Should only be called from StopPlatformDestination.
|
|
|
+- void DisablePullingAudioGraph() {
|
|
|
+- allow_pulling_audio_graph_.store(false, std::memory_order_release);
|
|
|
+- }
|
|
|
+-
|
|
|
+ const WebAudioLatencyHint latency_hint_;
|
|
|
+
|
|
|
+ // Holds the audio device thread that runs the real time audio context.
|
|
|
+ scoped_refptr<AudioDestination> platform_destination_;
|
|
|
+
|
|
|
+ base::Optional<float> sample_rate_;
|
|
|
+-
|
|
|
+- // If true, the audio graph will be pulled to get new data. Otherwise, the
|
|
|
+- // graph is not pulled, even if the audio thread is still running and
|
|
|
+- // requesting data.
|
|
|
+- //
|
|
|
+- // Must be modified only in StartPlatformDestination (via
|
|
|
+- // EnablePullingAudioGraph) or StopPlatformDestination (via
|
|
|
+- // DisablePullingAudioGraph) . This is modified only by the main threda and
|
|
|
+- // the audio thread only reads this.
|
|
|
+- std::atomic_bool allow_pulling_audio_graph_;
|
|
|
+ };
|
|
|
+
|
|
|
+ // -----------------------------------------------------------------------------
|