Diff
Modified: trunk/Source/WebCore/ChangeLog (276746 => 276747)
--- trunk/Source/WebCore/ChangeLog 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/ChangeLog 2021-04-29 01:53:30 UTC (rev 276747)
@@ -1,3 +1,38 @@
+2021-04-28 Chris Dumez <[email protected]>
+
+ Crash under WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long)
+ https://bugs.webkit.org/show_bug.cgi?id=225174
+ <rdar://77270701>
+
+ Unreviewed, revert r274767 as it introduced this crash.
+
+ * Modules/webaudio/AudioNodeInput.cpp:
+ (WebCore::AudioNodeInput::connect):
+ (WebCore::AudioNodeInput::disconnect):
+ (WebCore::AudioNodeInput::disable):
+ (WebCore::AudioNodeInput::enable):
+ (WebCore::AudioNodeInput::bus):
+ (WebCore::AudioNodeInput::sumAllConnections):
+ (WebCore::AudioNodeInput::pull):
+ * Modules/webaudio/AudioNodeInput.h:
+ * Modules/webaudio/AudioNodeOutput.cpp:
+ (WebCore::AudioNodeOutput::disable):
+ (WebCore::AudioNodeOutput::enable):
+ * Modules/webaudio/AudioParam.cpp:
+ (WebCore::AudioParam::calculateFinalValues):
+ * Modules/webaudio/AudioSummingJunction.cpp:
+ (WebCore::AudioSummingJunction::addOutput):
+ (WebCore::AudioSummingJunction::removeOutput):
+ (WebCore::AudioSummingJunction::updateRenderingState):
+ (WebCore::AudioSummingJunction::maximumNumberOfChannels const):
+ * Modules/webaudio/AudioSummingJunction.h:
+ (WebCore::AudioSummingJunction::renderingOutput):
+ (WebCore::AudioSummingJunction::renderingOutput const):
+ (WebCore::AudioSummingJunction::isConnected const):
+ (WebCore::AudioSummingJunction::numberOfConnections const):
+ * Modules/webaudio/WebKitAudioPannerNode.cpp:
+ (WebCore::WebKitAudioPannerNode::notifyAudioSourcesConnectedToNode):
+
2021-04-28 Wenson Hsieh <[email protected]>
[iOS] Coalesce adjacent selection geometries when rendering individual selection quads
Modified: trunk/Source/WebCore/Modules/webaudio/AudioNodeInput.cpp (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/AudioNodeInput.cpp 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/AudioNodeInput.cpp 2021-04-29 01:53:30 UTC (rev 276747)
@@ -54,7 +54,13 @@
if (!output || !node())
return;
- if (addOutput(*output))
+ auto addPotentiallyDisabledOutput = [this](AudioNodeOutput& output) {
+ if (output.isEnabled())
+ return addOutput(output);
+ return m_disabledOutputs.add(&output).isNewEntry;
+ };
+
+ if (addPotentiallyDisabledOutput(*output))
output->addInput(this);
}
@@ -72,24 +78,60 @@
output->removeInput(this); // Note: it's important to return immediately after this since the node may be deleted.
return;
}
+
+ // Otherwise, try to disconnect from disabled connections.
+ if (m_disabledOutputs.remove(output)) {
+ output->removeInput(this); // Note: it's important to return immediately after this since the node may be deleted.
+ return;
+ }
ASSERT_NOT_REACHED();
}
-void AudioNodeInput::outputEnabledStateChanged(AudioNodeOutput& output)
+void AudioNodeInput::disable(AudioNodeOutput* output)
{
ASSERT(context());
ASSERT(context()->isGraphOwner());
- AudioSummingJunction::outputEnabledStateChanged(output);
+ ASSERT(output && node());
+ if (!output || !node())
+ return;
+
+ {
+ // Heap allocations are forbidden on the audio thread for performance reasons so we need to
+ // explicitly allow the following allocation(s).
+ DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;
+ m_disabledOutputs.add(output);
+ bool wasRemoved = removeOutput(*output);
+ ASSERT_UNUSED(wasRemoved, wasRemoved);
+ }
+
// Propagate disabled state to outputs.
- ASSERT(node());
- if (!node())
+ node()->disableOutputsIfNecessary();
+}
+
+void AudioNodeInput::enable(AudioNodeOutput* output)
+{
+ ASSERT(context());
+ ASSERT(context()->isGraphOwner());
+
+ ASSERT(output && node());
+ if (!output || !node())
return;
- if (output.isEnabled())
- node()->enableOutputsIfNecessary();
- else
- node()->disableOutputsIfNecessary();
+
+ ASSERT(m_disabledOutputs.contains(output));
+
+ // Move output from disabled list to active list.
+ {
+ // Heap allocations are forbidden on the audio thread for performance reasons so we need to
+ // explicitly allow the following allocation(s).
+ DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;
+ addOutput(*output);
+ }
+ m_disabledOutputs.remove(output);
+
+ // Propagate enabled state to outputs.
+ node()->enableOutputsIfNecessary();
}
void AudioNodeInput::didUpdate()
@@ -135,7 +177,7 @@
// Handle single connection specially to allow for in-place processing.
if (numberOfRenderingConnections() == 1 && node()->channelCountMode() == ChannelCountMode::Max)
- return (*renderingOutputs().begin())->bus();
+ return renderingOutput(0)->bus();
// Multiple connections case or complex ChannelCountMode (or no connections).
return internalSummingBus();
@@ -165,7 +207,7 @@
auto interpretation = node()->channelInterpretation();
- for (auto* output : renderingOutputs()) {
+ for (auto& output : m_renderingOutputs) {
ASSERT(output);
// Render audio from this output.
@@ -181,17 +223,16 @@
ASSERT(context());
ASSERT(context()->isAudioThread());
- auto numberOfRenderingConnections = this->numberOfRenderingConnections();
// Handle single connection case.
- if (numberOfRenderingConnections == 1 && node()->channelCountMode() == ChannelCountMode::Max) {
+ if (numberOfRenderingConnections() == 1 && node()->channelCountMode() == ChannelCountMode::Max) {
// The output will optimize processing using inPlaceBus if it's able.
- AudioNodeOutput* output = *renderingOutputs().begin();
+ AudioNodeOutput* output = this->renderingOutput(0);
return output->pull(inPlaceBus, framesToProcess);
}
AudioBus* internalSummingBus = this->internalSummingBus();
- if (!numberOfRenderingConnections) {
+ if (!numberOfRenderingConnections()) {
// At least, generate silence if we're not connected to anything.
// FIXME: if we wanted to get fancy, we could propagate a 'silent hint' here to optimize the downstream graph processing.
internalSummingBus->zero();
Modified: trunk/Source/WebCore/Modules/webaudio/AudioNodeInput.h (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/AudioNodeInput.h 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/AudioNodeInput.h 2021-04-29 01:53:30 UTC (rev 276747)
@@ -55,8 +55,11 @@
void connect(AudioNodeOutput*);
void disconnect(AudioNodeOutput*);
+ // disable() will take the output out of the active connections list and set aside in a disabled list.
+ // enable() will put the output back into the active connections list.
// Must be called with the context's graph lock.
- void outputEnabledStateChanged(AudioNodeOutput&) final;
+ void enable(AudioNodeOutput*);
+ void disable(AudioNodeOutput*);
// pull() processes all of the AudioNodes connected to us.
// In the case of multiple connections it sums the result into an internal summing bus.
@@ -79,6 +82,11 @@
private:
AudioNode* m_node;
+ // m_disabledOutputs contains the AudioNodeOutputs which are disabled (will not be processed) by the audio graph rendering.
+ // But, from _javascript_'s perspective, these outputs are still connected to us.
+ // Generally, these represent disabled connections from "notes" which have finished playing but are not yet garbage collected.
+ HashSet<AudioNodeOutput*> m_disabledOutputs;
+
// Called from context's audio thread.
AudioBus* internalSummingBus();
void sumAllConnections(AudioBus* summingBus, size_t framesToProcess);
Modified: trunk/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp 2021-04-29 01:53:30 UTC (rev 276747)
@@ -228,12 +228,11 @@
{
ASSERT(context().isGraphOwner());
- if (!m_isEnabled)
- return;
-
- m_isEnabled = false;
- for (auto& input : m_inputs.keys())
- input->outputEnabledStateChanged(*this);
+ if (m_isEnabled) {
+ for (auto& input : m_inputs.keys())
+ input->disable(this);
+ m_isEnabled = false;
+ }
}
void AudioNodeOutput::enable()
@@ -240,12 +239,11 @@
{
ASSERT(context().isGraphOwner());
- if (m_isEnabled)
- return;
-
- m_isEnabled = true;
- for (auto& input : m_inputs.keys())
- input->outputEnabledStateChanged(*this);
+ if (!m_isEnabled) {
+ for (auto& input : m_inputs.keys())
+ input->enable(this);
+ m_isEnabled = true;
+ }
}
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webaudio/AudioParam.cpp (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/AudioParam.cpp 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/AudioParam.cpp 2021-04-29 01:53:30 UTC (rev 276747)
@@ -314,7 +314,7 @@
ASSERT(numberOfValues <= AudioUtilities::renderQuantumSize);
m_summingBus->setChannelMemory(0, values, sampleAccurate ? numberOfValues : 1);
- for (auto* output : renderingOutputs()) {
+ for (auto& output : m_renderingOutputs) {
ASSERT(output);
// Render audio from this output.
Modified: trunk/Source/WebCore/Modules/webaudio/AudioSummingJunction.cpp (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/AudioSummingJunction.cpp 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/AudioSummingJunction.cpp 2021-04-29 01:53:30 UTC (rev 276747)
@@ -62,13 +62,10 @@
if (!m_outputs.add(&output).isNewEntry)
return false;
- if (!output.isEnabled())
- return true;
-
- if (!m_pendingRenderingOutputs)
- m_pendingRenderingOutputs.emplace(m_outputs);
+ if (m_pendingRenderingOutputs.isEmpty())
+ m_pendingRenderingOutputs = copyToVector(m_outputs);
else
- m_pendingRenderingOutputs->append(output);
+ m_pendingRenderingOutputs.append(&output);
markRenderingStateAsDirty();
return true;
@@ -81,19 +78,11 @@
if (!m_outputs.remove(&output))
return false;
- if (!output.isEnabled())
- return true;
+ if (m_pendingRenderingOutputs.isEmpty())
+ m_pendingRenderingOutputs = copyToVector(m_outputs);
+ else
+ m_pendingRenderingOutputs.removeFirst(&output);
- if (!m_pendingRenderingOutputs) {
- // Heap allocations are forbidden on the audio thread for performance reasons so we need to
- // explicitly allow the following allocation(s).
- DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;
- m_pendingRenderingOutputs.emplace(m_outputs);
- } else {
- bool wasRemoved = m_pendingRenderingOutputs->remove(output);
- ASSERT_UNUSED(wasRemoved, wasRemoved);
- }
-
markRenderingStateAsDirty();
return true;
}
@@ -105,13 +94,9 @@
if (m_renderingStateNeedUpdating && canUpdateState()) {
// Copy from m_outputs to m_renderingOutputs.
- if (!m_pendingRenderingOutputs)
- m_renderingOutputs = { };
- else {
- m_renderingOutputs = *std::exchange(m_pendingRenderingOutputs, WTF::nullopt);
- for (auto* output : m_renderingOutputs)
- output->updateRenderingState();
- }
+ m_renderingOutputs = std::exchange(m_pendingRenderingOutputs, { });
+ for (auto& output : m_renderingOutputs)
+ output->updateRenderingState();
didUpdate();
@@ -123,9 +108,6 @@
{
unsigned maxChannels = 0;
for (auto* output : m_outputs) {
- if (!output->isEnabled())
- continue;
-
// Use output()->numberOfChannels() instead of output->bus()->numberOfChannels(),
// because the calling of AudioNodeOutput::bus() is not safe here.
maxChannels = std::max(maxChannels, output->numberOfChannels());
@@ -133,34 +115,6 @@
return maxChannels;
}
-void AudioSummingJunction::outputEnabledStateChanged(AudioNodeOutput& output)
-{
- ASSERT(context());
- ASSERT(context()->isGraphOwner());
- if (!m_pendingRenderingOutputs) {
- // Heap allocations are forbidden on the audio thread for performance reasons so we need to
- // explicitly allow the following allocation(s).
- DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;
- m_pendingRenderingOutputs.emplace(m_outputs);
- } else
- m_pendingRenderingOutputs->updatedEnabledState(output);
- markRenderingStateAsDirty();
-}
-
-inline AudioSummingJunction::RenderingOutputCollection::RenderingOutput::RenderingOutput(AudioNodeOutput* output)
- : output(output)
- , isEnabled(output->isEnabled())
-{ }
-
-void WebCore::AudioSummingJunction::RenderingOutputCollection::updatedEnabledState(AudioNodeOutput& output)
-{
- auto index = m_outputs.find(&output);
- ASSERT(index != notFound);
- if (index == notFound)
- return;
- m_outputs[index].isEnabled = output.isEnabled();
-}
-
} // namespace WebCore
#endif // ENABLE(WEB_AUDIO)
Modified: trunk/Source/WebCore/Modules/webaudio/AudioSummingJunction.h (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/AudioSummingJunction.h 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/AudioSummingJunction.h 2021-04-29 01:53:30 UTC (rev 276747)
@@ -25,7 +25,6 @@
#pragma once
#include "AudioBus.h"
-#include <iterator>
#include <wtf/HashSet.h>
#include <wtf/Vector.h>
#include <wtf/WeakPtr.h>
@@ -50,76 +49,11 @@
// This must be called when we own the context's graph lock in the audio thread at the very start or end of the render quantum.
void updateRenderingState();
- class RenderingOutputCollection {
- public:
- RenderingOutputCollection() = default;
- RenderingOutputCollection(const HashSet<AudioNodeOutput*>& outputs)
- : m_outputs(copyToVectorOf<RenderingOutput>(outputs))
- {
- }
-
- bool isEmpty() const { return begin() == end(); }
- size_t size() const { return std::distance(begin(), end()); }
-
- void append(AudioNodeOutput& output) { m_outputs.append(&output); }
- bool remove(AudioNodeOutput& output) { return m_outputs.removeFirst(&output); }
- void updatedEnabledState(AudioNodeOutput&);
-
- struct RenderingOutput {
- RenderingOutput(AudioNodeOutput*);
- bool operator==(const AudioNodeOutput* other) const { return output == other; }
- AudioNodeOutput* output;
- bool isEnabled;
- };
-
- class ConstIterator {
- public:
- using difference_type = size_t;
- using value_type = AudioNodeOutput*;
- using pointer = AudioNodeOutput**;
- using reference = AudioNodeOutput*&;
- using iterator_category = std::forward_iterator_tag;
-
- ConstIterator(const Vector<RenderingOutput>& outputs, Vector<RenderingOutput>::const_iterator position)
- : m_position(position)
- , m_end(outputs.end())
- {
- skipDisabledOutputs();
- }
-
- AudioNodeOutput* operator*() const { return m_position->output; }
- constexpr bool operator==(const ConstIterator& other) const { return m_position == other.m_position; }
- constexpr bool operator!=(const ConstIterator& other) const { return !(*this == other); }
- ConstIterator& operator++()
- {
- ASSERT(m_position != m_end);
- ++m_position;
- skipDisabledOutputs();
- return *this;
- }
-
- private:
- void skipDisabledOutputs()
- {
- while (m_position != m_end && !m_position->isEnabled)
- ++m_position;
- }
-
- Vector<RenderingOutput>::const_iterator m_position;
- Vector<RenderingOutput>::const_iterator m_end;
- };
-
- ConstIterator begin() const { return ConstIterator { m_outputs, m_outputs.begin() }; }
- ConstIterator end() const { return ConstIterator { m_outputs, m_outputs.end() }; }
-
- private:
- Vector<RenderingOutput> m_outputs;
- };
-
// Rendering code accesses its version of the current connections here.
- const RenderingOutputCollection& renderingOutputs() const { return m_renderingOutputs; }
unsigned numberOfRenderingConnections() const { return m_renderingOutputs.size(); }
- bool isConnected() const { return !m_renderingOutputs.isEmpty(); }
+ AudioNodeOutput* renderingOutput(unsigned i) { return m_renderingOutputs[i]; }
+ const AudioNodeOutput* renderingOutput(unsigned i) const { return m_renderingOutputs[i]; }
+ bool isConnected() const { return numberOfRenderingConnections() > 0; }
virtual bool canUpdateState() = 0;
virtual void didUpdate() = 0;
@@ -129,27 +63,29 @@
void markRenderingStateAsDirty();
- virtual void outputEnabledStateChanged(AudioNodeOutput&);
+protected:
+ WeakPtr<BaseAudioContext> m_context;
-protected:
+ // numberOfConnections() should never be called from the audio rendering thread.
+ // Instead numberOfRenderingConnections() and renderingOutput() should be used.
+ unsigned numberOfConnections() const { return m_outputs.size(); }
+
unsigned maximumNumberOfChannels() const;
-private:
- WeakPtr<BaseAudioContext> m_context;
-
// m_renderingOutputs is a copy of m_outputs which will never be modified during the graph rendering on the audio thread.
// This is the list which is used by the rendering code.
// Whenever m_outputs is modified, the context is told so it can later update m_renderingOutputs from m_outputs at a safe time.
// Most of the time, m_renderingOutputs is identical to m_outputs.
- RenderingOutputCollection m_renderingOutputs;
+ Vector<AudioNodeOutput*> m_renderingOutputs;
// m_renderingStateNeedUpdating keeps track if m_outputs is modified.
bool m_renderingStateNeedUpdating { false };
+private:
// m_outputs contains the AudioNodeOutputs representing current connections which are not disabled.
// The rendering code should never use this directly, but instead uses m_renderingOutputs.
HashSet<AudioNodeOutput*> m_outputs;
- Optional<RenderingOutputCollection> m_pendingRenderingOutputs;
+ Vector<AudioNodeOutput*> m_pendingRenderingOutputs;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.cpp (276746 => 276747)
--- trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.cpp 2021-04-29 01:36:32 UTC (rev 276746)
+++ trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.cpp 2021-04-29 01:53:30 UTC (rev 276747)
@@ -319,7 +319,8 @@
AudioNodeInput* input = node->input(i);
// For each input, go through all of its connections, looking for AudioBufferSourceNodes.
- for (auto* connectedOutput : input->renderingOutputs()) {
+ for (unsigned j = 0; j < input->numberOfRenderingConnections(); ++j) {
+ AudioNodeOutput* connectedOutput = input->renderingOutput(j);
AudioNode* connectedNode = connectedOutput->node();
if (visitedNodes.contains(connectedNode))
continue;
