Diff
Modified: trunk/Source/WebCore/ChangeLog (274649 => 274650)
--- trunk/Source/WebCore/ChangeLog 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/ChangeLog 2021-03-18 17:51:00 UTC (rev 274650)
@@ -1,3 +1,36 @@
+2021-03-18 Chris Dumez <[email protected]>
+
+ Avoid heap allocation under PannerNode::pullInputs()
+ https://bugs.webkit.org/show_bug.cgi?id=223445
+
+ Reviewed by Darin Adler.
+
+ PannerNode::pullInputs() was doing a heap allocation on the audio thread when
+ constructing a HashSet. It turns out that PannerNode::pullInputs() and
+ PannerNode::notifyAudioSourcesConnectedToNode() were not needed at all in
+ the modern PannerNode. They were only used to set the PannerNode on
+ AudioBufferSourceNodes so they could use its dopplerRate. However, the
+ dopplerRate was only a thing on the legacy WebKitAudioPannerNode. For the
+ modern PannerNode, there is no dopplerRate so we always end up using 1.0.
+
+ To address the issue, I thus completely removed PannerNode's pullInputs &
+ notifyAudioSourcesConnectedToNode(). I also updated AudioBufferSourceNode
+ to make it clear it uses a WebKitAudioPannerNode. Finally, I got rid of
+ the PannerNodeBase base class too since it is no longer needed.
+
+ * Modules/webaudio/AudioBufferSourceNode.cpp:
+ (WebCore::AudioBufferSourceNode::totalPitchRate):
+ (WebCore::AudioBufferSourceNode::setPannerNode):
+ (WebCore::AudioBufferSourceNode::clearPannerNode):
+ (WebCore::AudioBufferSourceNode::finish):
+ * Modules/webaudio/AudioBufferSourceNode.h:
+ * Modules/webaudio/PannerNode.cpp:
+ (WebCore::PannerNode::PannerNode):
+ * Modules/webaudio/PannerNode.h:
+ * Modules/webaudio/WebKitAudioPannerNode.cpp:
+ (WebCore::WebKitAudioPannerNode::WebKitAudioPannerNode):
+ * Modules/webaudio/WebKitAudioPannerNode.h:
+
2021-03-18 Truitt Savell <[email protected]>
Unreviewed, reverting r274644.
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp (274649 => 274650)
--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp 2021-03-18 17:51:00 UTC (rev 274650)
@@ -35,9 +35,9 @@
#include "AudioParam.h"
#include "AudioUtilities.h"
#include "FloatConversion.h"
-#include "PannerNode.h"
#include "ScriptExecutionContext.h"
#include "WebKitAudioBufferSourceNode.h"
+#include "WebKitAudioPannerNode.h"
#include <wtf/IsoMallocInlines.h>
namespace WebCore {
@@ -538,8 +538,8 @@
double AudioBufferSourceNode::totalPitchRate()
{
double dopplerRate = 1.0;
- if (m_pannerNode)
- dopplerRate = m_pannerNode->dopplerRate();
+ if (m_legacyPannerNode)
+ dopplerRate = m_legacyPannerNode->dopplerRate();
// Incorporate buffer's sample-rate versus AudioContext's sample-rate.
// Normally it's not an issue because buffers are loaded at the AudioContext's sample-rate, but we can handle it in any case.
@@ -567,21 +567,21 @@
return !isPlayingOrScheduled() || hasFinished() || !m_buffer;
}
-void AudioBufferSourceNode::setPannerNode(PannerNodeBase* pannerNode)
+void AudioBufferSourceNode::setPannerNode(WebKitAudioPannerNode* pannerNode)
{
- if (m_pannerNode != pannerNode && !hasFinished())
- m_pannerNode = pannerNode;
+ if (m_legacyPannerNode != pannerNode && !hasFinished())
+ m_legacyPannerNode = pannerNode;
}
void AudioBufferSourceNode::clearPannerNode()
{
- m_pannerNode = nullptr;
+ m_legacyPannerNode = nullptr;
}
void AudioBufferSourceNode::finish()
{
clearPannerNode();
- ASSERT(!m_pannerNode);
+ ASSERT(!m_legacyPannerNode);
AudioScheduledSourceNode::finish();
}
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h (274649 => 274650)
--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h 2021-03-18 17:51:00 UTC (rev 274650)
@@ -33,7 +33,7 @@
namespace WebCore {
class AudioBuffer;
-class PannerNodeBase;
+class WebKitAudioPannerNode;
struct AudioBufferSourceOptions;
// AudioBufferSourceNode is an AudioNode representing an audio source from an in-memory audio asset represented by an AudioBuffer.
@@ -77,7 +77,7 @@
AudioParam& playbackRate() { return m_playbackRate.get(); }
// If a panner node is set, then we can incorporate doppler shift into the playback pitch rate.
- void setPannerNode(PannerNodeBase*);
+ void setPannerNode(WebKitAudioPannerNode*);
void clearPannerNode();
// If we are no longer playing, propogate silence ahead to downstream nodes.
@@ -142,7 +142,7 @@
// We optionally keep track of a panner node which has a doppler shift that is incorporated into
// the pitch rate.
- AudioConnectionRefPtr<PannerNodeBase> m_pannerNode;
+ AudioConnectionRefPtr<WebKitAudioPannerNode> m_legacyPannerNode;
// This synchronizes process() with setBuffer() which can cause dynamic channel count changes.
mutable Lock m_processLock;
Modified: trunk/Source/WebCore/Modules/webaudio/PannerNode.cpp (274649 => 274650)
--- trunk/Source/WebCore/Modules/webaudio/PannerNode.cpp 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/Modules/webaudio/PannerNode.cpp 2021-03-18 17:51:00 UTC (rev 274650)
@@ -50,11 +50,6 @@
x = 0.0;
}
-PannerNodeBase::PannerNodeBase(BaseAudioContext& context)
- : AudioNode(context, NodeTypePanner)
-{
-}
-
ExceptionOr<Ref<PannerNode>> PannerNode::create(BaseAudioContext& context, const PannerOptions& options)
{
auto panner = adoptRef(*new PannerNode(context, options));
@@ -83,7 +78,7 @@
}
PannerNode::PannerNode(BaseAudioContext& context, const PannerOptions& options)
- : PannerNodeBase(context)
+ : AudioNode(context, NodeTypePanner)
, m_panningModel(options.panningModel)
, m_positionX(AudioParam::create(context, "positionX"_s, options.positionX, -FLT_MAX, FLT_MAX, AutomationRate::ARate))
, m_positionY(AudioParam::create(context, "positionY"_s, options.positionY, -FLT_MAX, FLT_MAX, AutomationRate::ARate))
@@ -109,21 +104,6 @@
uninitialize();
}
-void PannerNode::pullInputs(size_t framesToProcess)
-{
- // We override pullInputs(), so we can detect new AudioSourceNodes which have connected to us when new connections are made.
- // These AudioSourceNodes need to be made aware of our existence in order to handle doppler shift pitch changes.
- if (m_connectionCount != context().connectionCount()) {
- m_connectionCount = context().connectionCount();
-
- // Recursively go through all nodes connected to us.
- HashSet<AudioNode*> visitedNodes;
- notifyAudioSourcesConnectedToNode(this, visitedNodes);
- }
-
- AudioNode::pullInputs(framesToProcess);
-}
-
void PannerNode::process(size_t framesToProcess)
{
AudioBus* destination = output(0)->bus();
@@ -533,11 +513,6 @@
calculateAzimuthElevation(outAzimuth, outElevation, position(), listener().position(), listener().orientation(), listener().upVector());
}
-float PannerNode::dopplerRate()
-{
- return 1.0f;
-}
-
bool PannerNode::requiresTailProcessing() const
{
// If there's no internal panner method set up yet, assume we require tail
@@ -564,35 +539,6 @@
return calculateDistanceConeGain(position(), orientation(), listener().position());
}
-void PannerNode::notifyAudioSourcesConnectedToNode(AudioNode* node, HashSet<AudioNode*>& visitedNodes)
-{
- ASSERT(node);
- if (!node)
- return;
-
- // First check if this node is an AudioBufferSourceNode. If so, let it know about us so that doppler shift pitch can be taken into account.
- if (node->nodeType() == NodeTypeAudioBufferSource) {
- AudioBufferSourceNode* bufferSourceNode = reinterpret_cast<AudioBufferSourceNode*>(node);
- bufferSourceNode->setPannerNode(this);
- } else {
- // Go through all inputs to this node.
- for (unsigned i = 0; i < node->numberOfInputs(); ++i) {
- AudioNodeInput* input = node->input(i);
-
- // For each input, go through all of its connections, looking for AudioBufferSourceNodes.
- for (unsigned j = 0; j < input->numberOfRenderingConnections(); ++j) {
- AudioNodeOutput* connectedOutput = input->renderingOutput(j);
- AudioNode* connectedNode = connectedOutput->node();
- if (visitedNodes.contains(connectedNode))
- continue;
-
- visitedNodes.add(connectedNode);
- notifyAudioSourcesConnectedToNode(connectedNode, visitedNodes);
- }
- }
- }
-}
-
} // namespace WebCore
#endif // ENABLE(WEB_AUDIO)
Modified: trunk/Source/WebCore/Modules/webaudio/PannerNode.h (274649 => 274650)
--- trunk/Source/WebCore/Modules/webaudio/PannerNode.h 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/Modules/webaudio/PannerNode.h 2021-03-18 17:51:00 UTC (rev 274650)
@@ -44,16 +44,6 @@
class HRTFDatabaseLoader;
class BaseAudioContext;
-class PannerNodeBase : public AudioNode {
-public:
- virtual ~PannerNodeBase() = default;
-
- virtual float dopplerRate() = 0;
-
-protected:
- PannerNodeBase(BaseAudioContext&);
-};
-
// PannerNode is an AudioNode with one input and one output.
// It positions a sound in 3D space, with the exact effect dependent on the panning model.
// It has a position and an orientation in 3D space which is relative to the position and orientation of the context's AudioListener.
@@ -61,7 +51,7 @@
// A cone effect will attenuate the gain as the orientation moves away from the listener.
// All of these effects follow the OpenAL specification very closely.
-class PannerNode final : public PannerNodeBase {
+class PannerNode final : public AudioNode {
WTF_MAKE_ISO_ALLOCATED(PannerNode);
public:
static ExceptionOr<Ref<PannerNode>> create(BaseAudioContext&, const PannerOptions& = { });
@@ -71,7 +61,6 @@
// AudioNode
void process(size_t framesToProcess) override;
void processOnlyAudioParams(size_t framesToProcess) final;
- void pullInputs(size_t framesToProcess) override;
void initialize() override;
void uninitialize() override;
@@ -123,7 +112,6 @@
ExceptionOr<void> setChannelCountMode(ChannelCountMode) final;
void azimuthElevation(double* outAzimuth, double* outElevation);
- float dopplerRate() final;
double tailTime() const override { return m_panner ? m_panner->tailTime() : 0; }
double latencyTime() const override { return m_panner ? m_panner->latencyTime() : 0; }
@@ -139,10 +127,6 @@
bool requiresTailProcessing() const final;
- // Notifies any AudioBufferSourceNodes connected to us either directly or indirectly about our existence.
- // This is in order to handle the pitch change necessary for the doppler shift.
- void notifyAudioSourcesConnectedToNode(AudioNode*, HashSet<AudioNode*>& visitedNodes);
-
void processSampleAccurateValues(AudioBus* destination, const AudioBus* source, size_t framesToProcess);
bool hasSampleAccurateValues() const;
bool shouldUseARate() const;
@@ -165,8 +149,6 @@
// HRTF Database loader
RefPtr<HRTFDatabaseLoader> m_hrtfDatabaseLoader;
- unsigned m_connectionCount { 0 };
-
// Synchronize process() with setting of the panning model, source's location
// information, listener, distance parameters and sound cones.
mutable Lock m_processLock;
Modified: trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.cpp (274649 => 274650)
--- trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.cpp 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.cpp 2021-03-18 17:51:00 UTC (rev 274650)
@@ -49,7 +49,7 @@
}
WebKitAudioPannerNode::WebKitAudioPannerNode(WebKitAudioContext& context)
- : PannerNodeBase(context)
+ : AudioNode(context, NodeTypePanner)
{
initializeDefaultNodeOptions(2, ChannelCountMode::ClampedMax, ChannelInterpretation::Speakers);
Modified: trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.h (274649 => 274650)
--- trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.h 2021-03-18 17:25:44 UTC (rev 274649)
+++ trunk/Source/WebCore/Modules/webaudio/WebKitAudioPannerNode.h 2021-03-18 17:51:00 UTC (rev 274650)
@@ -32,7 +32,6 @@
#include "Distance.h"
#include "FloatPoint3D.h"
#include "Panner.h"
-#include "PannerNode.h"
#include "WebKitAudioContext.h"
#include "WebKitAudioListener.h"
#include <memory>
@@ -50,7 +49,7 @@
// A cone effect will attenuate the gain as the orientation moves away from the listener.
// All of these effects follow the OpenAL specification very closely.
-class WebKitAudioPannerNode final : public PannerNodeBase {
+class WebKitAudioPannerNode final : public AudioNode {
WTF_MAKE_ISO_ALLOCATED(WebKitAudioPannerNode);
public:
static Ref<WebKitAudioPannerNode> create(WebKitAudioContext& context)
@@ -112,7 +111,7 @@
void setConeOuterGain(double angle) { m_coneEffect.setOuterGain(angle); }
void getAzimuthElevation(double* outAzimuth, double* outElevation);
- float dopplerRate() final;
+ float dopplerRate();
double tailTime() const override { return m_panner ? m_panner->tailTime() : 0; }
double latencyTime() const override { return m_panner ? m_panner->latencyTime() : 0; }
