Changeset 283322 in webkit

Timestamp:

Sep 30, 2021, 12:23:43 PM (5 years ago)

Author:

Chris Dumez

Message:

Web Audio panner node quality deteriorates over time
​https://bugs.webkit.org/show_bug.cgi?id=230950
<rdar://problem/83675934>

Reviewed by Eric Carlson.

When profiling the test case, I noticed what we were spending most of the CPU time
under AudioParamTimeline::valuesForFrameRangeImpl() / isEventCurrent() called from
PannerNode::process(). The reason for that is that the number of events in the
AudioParamTimeline would keep growing unboundedly and it would make the
valuesForFrameRangeImpl() implementation more and more expensive over time, since
it has to iterate over all events.

To address the issue, valuesForFrameRangeImpl() now keeps track of the events
that it had to skip because they were in the past. Then, at the end of the loop,
it removes the outdated events so it won't have to iterate over them the next
time around. This behavior is similar to what Blink does.

• Modules/webaudio/AudioParamTimeline.cpp:

(WebCore::AudioParamTimeline::removeOldEvents):
(WebCore::AudioParamTimeline::valuesForFrameRangeImpl):

• Modules/webaudio/AudioParamTimeline.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/webaudio/AudioParamTimeline.cpp (modified) (4 diffs)
• Modules/webaudio/AudioParamTimeline.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-30  Chris Dumez  <cdumez@apple.com>
+
+        Web Audio panner node quality deteriorates over time
+        https://bugs.webkit.org/show_bug.cgi?id=230950
+        <rdar://problem/83675934>
+
+        Reviewed by Eric Carlson.
+
+        When profiling the test case, I noticed what we were spending most of the CPU time
+        under AudioParamTimeline::valuesForFrameRangeImpl() / isEventCurrent() called from
+        PannerNode::process(). The reason for that is that the number of events in the
+        AudioParamTimeline would keep growing unboundedly and it would make the
+        valuesForFrameRangeImpl() implementation more and more expensive over time, since
+        it has to iterate over all events.
+
+        To address the issue, valuesForFrameRangeImpl() now keeps track of the events
+        that it had to skip because they were in the past. Then, at the end of the loop,
+        it removes the outdated events so it won't have to iterate over them the next
+        time around. This behavior is similar to what Blink does.
+
+        * Modules/webaudio/AudioParamTimeline.cpp:
+        (WebCore::AudioParamTimeline::removeOldEvents):
+        (WebCore::AudioParamTimeline::valuesForFrameRangeImpl):
+        * Modules/webaudio/AudioParamTimeline.h:
+
 2021-09-30  Ziran Sun  <zsun@igalia.com>
 

trunk/Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp

@@ -325 +325 @@
 }
 
+void AudioParamTimeline::removeOldEvents(size_t eventCount)
+{
+    ASSERT(eventCount <= m_events.size());
+    if (m_events.isEmpty())
+        return;
+
+    // Always leave at least one event in the list.
+    m_events.remove(0, std::min(eventCount, m_events.size() - 1));
+}
+
 std::optional<float> AudioParamTimeline::valueForContextTime(BaseAudioContext& context, float defaultValue, float minValue, float maxValue)
 {
@@ -398 +408 @@
 
     float value = defaultValue;
+    size_t numberOfSkippedEvents = 0;
 
     // Go through each event and render the value buffer where the times overlap,
@@ -409 +420 @@
 
         // Wait until we get a more recent event.
-        if (!isEventCurrent(*event, nextEvent, currentFrame, sampleRate))
+        if (!isEventCurrent(*event, nextEvent, currentFrame, sampleRate)) {
+            ++numberOfSkippedEvents;
             continue;
+        }
 
         auto nextEventType = nextEvent ? static_cast<ParamEvent::Type>(nextEvent->type()) : ParamEvent::LastType /* unknown */;
@@ -482 +495 @@
         }
     }
+
+    // Drop outdated events that we skipped so we don't have to go through them again in the future.
+    if (numberOfSkippedEvents > 0)
+        removeOldEvents(numberOfSkippedEvents);
 
     // If there's any time left after processing the last event then just propagate the last value

trunk/Source/WebCore/Modules/webaudio/AudioParamTimeline.h

@@ -191 +191 @@
 
     void removeCancelledEvents(size_t firstEventToRemove) WTF_REQUIRES_LOCK(m_eventsLock);
+    void removeOldEvents(size_t eventCount) WTF_REQUIRES_LOCK(m_eventsLock);
     ExceptionOr<void> insertEvent(ParamEvent&&) WTF_REQUIRES_LOCK(m_eventsLock);
     float valuesForFrameRangeImpl(size_t startFrame, size_t endFrame, float defaultValue, float* values, unsigned numberOfValues, double sampleRate, double controlRate) WTF_REQUIRES_LOCK(m_eventsLock);