Unreviewed, rolling out r179618.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Feb 2015 22:42:49 +0000 (22:42 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Feb 2015 22:42:49 +0000 (22:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141263

Off-by-one error causing flaky behavior in webaudio
/audiobuffersource-negative-playbackrate.html (Requested by
jernoble_ on #webkit).

Reverted changeset:

"[WebAudio] AudioBufferSourceNodes should accurately play
backwards if given a negative playbackRate."
https://bugs.webkit.org/show_bug.cgi?id=140955
http://trac.webkit.org/changeset/179618

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179638 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-expected.wav [deleted file]
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated-expected.wav [deleted file]
LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated.html [deleted file]
LayoutTests/webaudio/audiobuffersource-negative-playbackrate.html [deleted file]
LayoutTests/webaudio/resources/note-grain-on-testing.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp

index b780960..fb750ae 100644 (file)
@@ -1,3 +1,19 @@
+2015-02-04  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r179618.
+        https://bugs.webkit.org/show_bug.cgi?id=141263
+
+        Off-by-one error causing flaky behavior in webaudio
+        /audiobuffersource-negative-playbackrate.html (Requested by
+        jernoble_ on #webkit).
+
+        Reverted changeset:
+
+        "[WebAudio] AudioBufferSourceNodes should accurately play
+        backwards if given a negative playbackRate."
+        https://bugs.webkit.org/show_bug.cgi?id=140955
+        http://trac.webkit.org/changeset/179618
+
 2015-02-03  David Hyatt  <hyatt@apple.com>
 
         Tables don't repaginate properly when the pagination height changes or the pagination offset changes.
diff --git a/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-expected.wav b/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-expected.wav
deleted file mode 100644 (file)
index c0d7999..0000000
Binary files a/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-expected.wav and /dev/null differ
diff --git a/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated-expected.wav b/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated-expected.wav
deleted file mode 100644 (file)
index 15baab7..0000000
Binary files a/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated-expected.wav and /dev/null differ
diff --git a/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated.html b/LayoutTests/webaudio/audiobuffersource-negative-playbackrate-interpolated.html
deleted file mode 100644 (file)
index 823ba27..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-    <title>audiobuffersource-negative-playbackrate-interpolated</title>
-    <script type="text/javascript" src="resources/audio-testing.js"></script>
-    <script type="text/javascript" src="resources/buffer-loader.js"></script>
-
-    <script>
-    var sampleRate = 44100.0;
-    var lengthInSeconds = 2;
-
-    var context = 0;
-    var bufferLoader = 0;
-
-    function go() {
-        if (!window.testRunner)
-            return;
-
-        context = new webkitOfflineAudioContext(2, sampleRate * lengthInSeconds, sampleRate);
-
-        bufferLoader = new BufferLoader(context, ["resources/hyper-reality/br-jam-loop.wav"], finishedLoading);
-        bufferLoader.load();
-        testRunner.waitUntilDone();
-    }
-
-    function finishedLoading(bufferList) {
-        var bufferSource = context.createBufferSource();
-        bufferSource.buffer = bufferList[0];
-
-        bufferSource.connect(context.destination);
-        bufferSource.playbackRate.value = -0.75;
-        bufferSource.start(0);
-
-        context.oncomplete = finishAudioTest;
-        context.startRendering();
-    }
-
-    </script>
-</head>
-<body onload="go()">
-</body>
-</html>
diff --git a/LayoutTests/webaudio/audiobuffersource-negative-playbackrate.html b/LayoutTests/webaudio/audiobuffersource-negative-playbackrate.html
deleted file mode 100644 (file)
index dbd9e9a..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-    <title>audiobuffersource-negative-playbackrate</title>
-    <script type="text/javascript" src="resources/audio-testing.js"></script>
-    <script type="text/javascript" src="resources/buffer-loader.js"></script>
-
-    <script>
-    var sampleRate = 44100.0;
-    var lengthInSeconds = 2;
-
-    var context = 0;
-    var bufferLoader = 0;
-
-    function go() {
-        if (!window.testRunner)
-            return;
-
-        context = new webkitOfflineAudioContext(2, sampleRate * lengthInSeconds, sampleRate);
-
-        bufferLoader = new BufferLoader(context, ["resources/hyper-reality/br-jam-loop.wav"], finishedLoading);
-        bufferLoader.load();
-        testRunner.waitUntilDone();
-    }
-
-    function finishedLoading(bufferList) {
-        var bufferSource = context.createBufferSource();
-        bufferSource.buffer = bufferList[0];
-
-        bufferSource.connect(context.destination);
-        bufferSource.playbackRate.value = -1;
-        bufferSource.start(0);
-
-        context.oncomplete = finishAudioTest;
-        context.startRendering();
-    }
-
-    </script>
-</head>
-<body onload="go()">
-</body>
-</html>
index e1d1443..8f93c9e 100644 (file)
@@ -1,13 +1,21 @@
 var sampleRate = 44100.0;
 
+// HRTF extra frames.  This is a magic constant currently in
+// AudioBufferSourceNode::process that always extends the
+// duration by this number of samples.  See bug 77224
+// (https://bugs.webkit.org/show_bug.cgi?id=77224).
+var extraFramesHRTF = 512;
+
 // How many grains to play.
 var numberOfTests = 100;
 
 // Duration of each grain to be played
 var duration = 0.01;
 
-// Time step between the start of each grain.
-var timeStep = duration + .005;
+// Time step between the start of each grain.  We need to add a little
+// bit of silence so we can detect grain boundaries and also account
+// for the extra frames for HRTF.
+var timeStep = duration + .005 + extraFramesHRTF / sampleRate;
 
 // Time step between the start for each grain.
 var grainOffsetStep = 0.001;
@@ -25,7 +33,7 @@ function createSignalBuffer(context, f) {
     // Make sure the buffer has enough data for all of the possible
     // grain offsets and durations.  Need to include the extra frames
     // for HRTF.  The additional 1 is for any round-off errors.
-    var signalLength = Math.floor(1 + sampleRate * (numberOfTests * grainOffsetStep + duration));
+    var signalLength = Math.floor(1 + extraFramesHRTF + sampleRate * (numberOfTests * grainOffsetStep + duration));
 
     var buffer = context.createBuffer(2, signalLength, sampleRate);
     var data = buffer.getChannelData(0);
@@ -127,7 +135,7 @@ function verifyStartAndEndFrames(startEndFrames) {
         var expectedStart = timeToSampleFrame(k * timeStep, sampleRate);
         // The end point is the duration, plus the extra frames
         // for HRTF.
-        var expectedEnd = expectedStart + grainLengthInSampleFrames(k * grainOffsetStep, duration, sampleRate);
+        var expectedEnd = extraFramesHRTF + expectedStart + grainLengthInSampleFrames(k * grainOffsetStep, duration, sampleRate);
 
         if (startFrames[k] != expectedStart) {
             testFailed("Pulse " + k + " started at " + startFrames[k] + " but expected at " + expectedStart);
index 0beb032..9e839a0 100644 (file)
@@ -1,3 +1,19 @@
+2015-02-04  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r179618.
+        https://bugs.webkit.org/show_bug.cgi?id=141263
+
+        Off-by-one error causing flaky behavior in webaudio
+        /audiobuffersource-negative-playbackrate.html (Requested by
+        jernoble_ on #webkit).
+
+        Reverted changeset:
+
+        "[WebAudio] AudioBufferSourceNodes should accurately play
+        backwards if given a negative playbackRate."
+        https://bugs.webkit.org/show_bug.cgi?id=140955
+        http://trac.webkit.org/changeset/179618
+
 2015-02-03  David Hyatt  <hyatt@apple.com>
 
         Tables don't repaginate properly when the pagination height changes or the pagination offset changes.
index 9a5f9c2..13bcacd 100644 (file)
@@ -70,7 +70,7 @@ AudioBufferSourceNode::AudioBufferSourceNode(AudioContext* context, float sample
     setNodeType(NodeTypeAudioBufferSource);
 
     m_gain = AudioParam::create(context, "gain", 1.0, 0.0, 1.0);
-    m_playbackRate = AudioParam::create(context, "playbackRate", 1.0, -MaxRate, MaxRate);
+    m_playbackRate = AudioParam::create(context, "playbackRate", 1.0, 0.0, MaxRate);
 
     // Default to mono.  A call to setBuffer() will set the number of output channels to that of the buffer.
     addOutput(std::make_unique<AudioNodeOutput>(this, 1));
@@ -200,54 +200,47 @@ bool AudioBufferSourceNode::renderFromBuffer(AudioBus* bus, unsigned destination
 
     size_t bufferLength = buffer()->length();
     double bufferSampleRate = buffer()->sampleRate();
-    double pitchRate = totalPitchRate();
-    bool reverse = pitchRate < 0;
 
     // Avoid converting from time to sample-frames twice by computing
     // the grain end time first before computing the sample frame.
-    unsigned maxFrame;
+    unsigned endFrame = m_isGrain ? AudioUtilities::timeToSampleFrame(m_grainOffset + m_grainDuration, bufferSampleRate) : bufferLength;
+    
+    // This is a HACK to allow for HRTF tail-time - avoids glitch at end.
+    // FIXME: implement tailTime for each AudioNode for a more general solution to this problem.
+    // https://bugs.webkit.org/show_bug.cgi?id=77224
     if (m_isGrain)
-        maxFrame = AudioUtilities::timeToSampleFrame(m_grainOffset + m_grainDuration, bufferSampleRate);
-    else
-        maxFrame = bufferLength;
+        endFrame += 512;
 
     // Do some sanity checking.
-    if (maxFrame > bufferLength)
-        maxFrame = bufferLength;
-    if (reverse && m_virtualReadIndex <= 0)
-        m_virtualReadIndex = maxFrame;
-    else if (!reverse && m_virtualReadIndex >= maxFrame)
+    if (endFrame > bufferLength)
+        endFrame = bufferLength;
+    if (m_virtualReadIndex >= endFrame)
         m_virtualReadIndex = 0; // reset to start
 
     // If the .loop attribute is true, then values of m_loopStart == 0 && m_loopEnd == 0 implies
     // that we should use the entire buffer as the loop, otherwise use the loop values in m_loopStart and m_loopEnd.
-    double virtualMaxFrame = maxFrame;
-    double virtualMinFrame = 0;
-    double virtualDeltaFrames = maxFrame;
+    double virtualEndFrame = endFrame;
+    double virtualDeltaFrames = endFrame;
 
     if (loop() && (m_loopStart || m_loopEnd) && m_loopStart >= 0 && m_loopEnd > 0 && m_loopStart < m_loopEnd) {
         // Convert from seconds to sample-frames.
-        double loopMinFrame = m_loopStart * buffer()->sampleRate();
-        double loopMaxFrame = m_loopEnd * buffer()->sampleRate();
+        double loopStartFrame = m_loopStart * buffer()->sampleRate();
+        double loopEndFrame = m_loopEnd * buffer()->sampleRate();
 
-        virtualMaxFrame = std::min(loopMaxFrame, virtualMaxFrame);
-        virtualMinFrame = std::max(loopMinFrame, virtualMinFrame);
-        virtualDeltaFrames = virtualMaxFrame - virtualMinFrame;
+        virtualEndFrame = std::min(loopEndFrame, virtualEndFrame);
+        virtualDeltaFrames = virtualEndFrame - loopStartFrame;
     }
 
 
+    double pitchRate = totalPitchRate();
+
     // Sanity check that our playback rate isn't larger than the loop size.
-    if (fabs(pitchRate) >= virtualDeltaFrames)
+    if (pitchRate >= virtualDeltaFrames)
         return false;
 
     // Get local copy.
     double virtualReadIndex = m_virtualReadIndex;
 
-    bool needsInterpolation = virtualReadIndex != floor(virtualReadIndex)
-        || virtualDeltaFrames != floor(virtualDeltaFrames)
-        || virtualMaxFrame != floor(virtualMaxFrame)
-        || virtualMinFrame != floor(virtualMinFrame);
-
     // Render loop - reading from the source buffer to the destination using linear interpolation.
     int framesToProcess = numberOfFrames;
 
@@ -256,12 +249,14 @@ bool AudioBufferSourceNode::renderFromBuffer(AudioBus* bus, unsigned destination
 
     // Optimize for the very common case of playing back with pitchRate == 1.
     // We can avoid the linear interpolation.
-    if (pitchRate == 1 && !needsInterpolation) {
+    if (pitchRate == 1 && virtualReadIndex == floor(virtualReadIndex)
+        && virtualDeltaFrames == floor(virtualDeltaFrames)
+        && virtualEndFrame == floor(virtualEndFrame)) {
         unsigned readIndex = static_cast<unsigned>(virtualReadIndex);
         unsigned deltaFrames = static_cast<unsigned>(virtualDeltaFrames);
-        maxFrame = static_cast<unsigned>(virtualMaxFrame);
+        endFrame = static_cast<unsigned>(virtualEndFrame);
         while (framesToProcess > 0) {
-            int framesToEnd = maxFrame - readIndex;
+            int framesToEnd = endFrame - readIndex;
             int framesThisTime = std::min(framesToProcess, framesToEnd);
             framesThisTime = std::max(0, framesThisTime);
 
@@ -273,85 +268,13 @@ bool AudioBufferSourceNode::renderFromBuffer(AudioBus* bus, unsigned destination
             framesToProcess -= framesThisTime;
 
             // Wrap-around.
-            if (readIndex >= maxFrame) {
+            if (readIndex >= endFrame) {
                 readIndex -= deltaFrames;
                 if (renderSilenceAndFinishIfNotLooping(bus, writeIndex, framesToProcess))
                     break;
             }
         }
         virtualReadIndex = readIndex;
-    } else if (pitchRate == -1 && !needsInterpolation) {
-        unsigned readIndex = static_cast<unsigned>(virtualReadIndex);
-        unsigned deltaFrames = static_cast<unsigned>(virtualDeltaFrames);
-        unsigned minFrame = static_cast<unsigned>(virtualMinFrame);
-        while (framesToProcess > 0) {
-            unsigned framesToEnd = readIndex - minFrame;
-            unsigned framesThisTime = std::min<unsigned>(framesToProcess, framesToEnd);
-            framesThisTime = std::max<unsigned>(0, framesThisTime);
-
-            while (framesThisTime--) {
-                for (unsigned i = 0; i < numberOfChannels; ++i) {
-                    float* destination = destinationChannels[i];
-                    const float* source = sourceChannels[i];
-
-                    destination[writeIndex] = source[readIndex];
-                }
-
-                ++writeIndex;
-                --readIndex;
-                --framesToProcess;
-            }
-
-            // Wrap-around.
-            if (!readIndex) {
-                readIndex = deltaFrames;
-                if (renderSilenceAndFinishIfNotLooping(bus, writeIndex, framesToProcess))
-                    break;
-            }
-        }
-        virtualReadIndex = readIndex;
-    } else if (!pitchRate) {
-        unsigned readIndex = static_cast<unsigned>(virtualReadIndex);
-
-        for (unsigned i = 0; i < numberOfChannels; ++i)
-            std::fill_n(destinationChannels[i], framesToProcess, sourceChannels[i][readIndex]);
-    } else if (reverse) {
-        while (framesToProcess--) {
-            unsigned readIndex = static_cast<unsigned>(virtualReadIndex);
-            double interpolationFactor = virtualReadIndex - readIndex;
-
-            unsigned readIndex2;
-            if (!readIndex) {
-                if (loop())
-                    readIndex2 = static_cast<unsigned>(virtualReadIndex + virtualDeltaFrames + 1);
-                else
-                    readIndex2 = readIndex;
-            } else
-                readIndex2 = readIndex - 1;
-
-            // Linear interpolation.
-            for (unsigned i = 0; i < numberOfChannels; ++i) {
-                float* destination = destinationChannels[i];
-                const float* source = sourceChannels[i];
-
-                double sample1 = source[readIndex];
-                double sample2 = source[readIndex2];
-                double sample = (1.0 - interpolationFactor) * sample1 + interpolationFactor * sample2;
-
-                destination[writeIndex] = narrowPrecisionToFloat(sample);
-            }
-
-            writeIndex++;
-
-            virtualReadIndex += pitchRate;
-
-            // Wrap-around, retaining sub-sample position since virtualReadIndex is floating-point.
-            if (virtualReadIndex <= virtualMinFrame) {
-                virtualReadIndex += virtualDeltaFrames;
-                if (renderSilenceAndFinishIfNotLooping(bus, writeIndex, framesToProcess))
-                    break;
-            }
-        }
     } else {
         while (framesToProcess--) {
             unsigned readIndex = static_cast<unsigned>(virtualReadIndex);
@@ -388,7 +311,7 @@ bool AudioBufferSourceNode::renderFromBuffer(AudioBus* bus, unsigned destination
             virtualReadIndex += pitchRate;
 
             // Wrap-around, retaining sub-sample position since virtualReadIndex is floating-point.
-            if (virtualReadIndex >= virtualMaxFrame) {
+            if (virtualReadIndex >= virtualEndFrame) {
                 virtualReadIndex -= virtualDeltaFrames;
                 if (renderSilenceAndFinishIfNotLooping(bus, writeIndex, framesToProcess))
                     break;
@@ -459,7 +382,7 @@ void AudioBufferSourceNode::start(double when, ExceptionCode& ec)
 
 void AudioBufferSourceNode::start(double when, double grainOffset, ExceptionCode& ec)
 {
-    startPlaying(Partial, when, grainOffset, buffer() ? buffer()->duration() - grainOffset : 0, ec);
+    startPlaying(Partial, when, grainOffset, 0, ec);
 }
 
 void AudioBufferSourceNode::start(double when, double grainOffset, double grainDuration, ExceptionCode& ec)
@@ -508,7 +431,7 @@ void AudioBufferSourceNode::startPlaying(BufferPlaybackMode playbackMode, double
         m_grainDuration = std::min(maxDuration, grainDuration);
     } else {
         m_grainOffset = 0.0;
-        m_grainDuration = buffer()->duration();
+        m_grainDuration = DefaultGrainDuration;
     }
 
     m_startTime = when;
@@ -517,8 +440,7 @@ void AudioBufferSourceNode::startPlaying(BufferPlaybackMode playbackMode, double
     // at a sub-sample position since it will degrade the quality.
     // When aligned to the sample-frame the playback will be identical to the PCM data stored in the buffer.
     // Since playbackRate == 1 is very common, it's worth considering quality.
-    double grainStart = totalPitchRate() < 0 ? m_grainOffset + m_grainDuration : m_grainOffset;
-    m_virtualReadIndex = AudioUtilities::timeToSampleFrame(grainStart, buffer()->sampleRate());
+    m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset, buffer()->sampleRate());
     
     m_playbackState = SCHEDULED_STATE;
 }
@@ -549,7 +471,11 @@ double AudioBufferSourceNode::totalPitchRate()
 
     double totalRate = dopplerRate * sampleRateFactor * basePitchRate;
 
-    totalRate = std::max(-MaxRate, std::min(MaxRate, totalRate));
+    // Sanity check the total rate.  It's very important that the resampler not get any bad rate values.
+    totalRate = std::max(0.0, totalRate);
+    if (!totalRate)
+        totalRate = 1; // zero rate is considered illegal
+    totalRate = std::min(MaxRate, totalRate);
     
     bool isTotalRateValid = !std::isnan(totalRate) && !std::isinf(totalRate);
     ASSERT(isTotalRateValid);