Fix issue with failing to call AudioBufferSource.onended in some cases

Source/modules/webaudio/AudioBufferSourceNode.cpp

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 29 matching lines @@
 
 namespace blink {
 
 const double DefaultGrainDuration = 0.020; // 20ms
 
 // Arbitrary upper limit on playback rate.
 // Higher than expected rates can be useful when playing back oversampled buffers
 // to minimize linear interpolation aliasing.
 const double MaxRate = 1024;
 
+// Number of extra frames to use when determining if a source node can be stopped.  This should be
+// at least one rendering quantum, but we add one more quantum for good measure.  This doesn't need
+// to be extra precise, just more than one rendering quantum.  See |handleStoppableSourceNode()|.
+// FIXME: Expose the rendering quantum somehow instead of hardwiring a value here.
+const int kExtraStopFrames = 256;
+
 AudioBufferSourceHandler::AudioBufferSourceHandler(AudioNode& node, float sampleRate, AudioParamHandler& playbackRate)
     : AudioScheduledSourceHandler(NodeTypeAudioBufferSource, node, sampleRate)
     , m_buffer(nullptr)
     , m_playbackRate(playbackRate)
     , m_isLooping(false)
     , m_loopStart(0)
     , m_loopEnd(0)
     , m_virtualReadIndex(0)
     , m_isGrain(false)
     , m_grainOffset(0.0)
@@ skipping 422 matching lines @@
 double AudioBufferSourceHandler::computePlaybackRate()
 {
     double dopplerRate = 1.0;
     if (m_pannerNode)
         dopplerRate = m_pannerNode->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.
     double sampleRateFactor = 1.0;
-    if (buffer())
-        sampleRateFactor = buffer()->sampleRate() / sampleRate();
+    if (buffer()) {
+        // Use doubles to compute this to full accuracy.
+        sampleRateFactor = buffer()->sampleRate() / static_cast<double>(sampleRate());
+    }
 
     // Use finalValue() to incorporate changes of AudioParamTimeline and
     // AudioSummingJunction from m_playbackRate AudioParam.
     double basePlaybackRate = m_playbackRate->finalValue();
 
     double finalPlaybackRate = dopplerRate * sampleRateFactor * basePlaybackRate;
 
     // Sanity check the total rate.  It's very important that the resampler not
     // get any bad rate values.
     finalPlaybackRate = clampTo(finalPlaybackRate, 0.0, MaxRate);
@@ skipping 30 matching lines @@
         m_pannerNode->breakConnection();
         m_pannerNode.clear();
     }
 }
 
 void AudioBufferSourceHandler::handleStoppableSourceNode()
 {
     // If the source node is not looping, and we have a buffer, we can determine when the
     // source would stop playing.
     if (!loop() && buffer() && isPlayingOrScheduled()) {
-        double stopTime = m_startTime + buffer()->duration();
+        // See crbug.com/478301. If a source node is started via start(), the source won't start at
+        // that time but one quantum (128 frames) later.  But we compute the stop time based on the
+        // start time and the duration, so we end up stopping one quantum early.  Thus, add a little
+        // extra time; we just need to stop the source sometime after it should have stopped if it
+        // hadn't already.
+        double extraStopTime = kExtraStopFrames / static_cast<double>(context()->sampleRate());
+        double stopTime = m_startTime + buffer()->duration() + extraStopTime;

[Ken Russell (switch to Gerrit), 2015/04/22 19:01:50]

This seems like a classic rounding error. Does the sound play in its entirety,
or are the first quantum's frames dropped when it starts playing? If the latter,
should the if-test below be changed rather than adding on this extraStopTime?

[Raymond Toy, 2015/04/22 20:19:45]

Without this fix, it's the last few frames for the last quantum that don't play
because we prematurely stopped it.

It's not clear but the purpose of this function is to handle the case where you
have a source node with a buffer, and you start it. This creates a reference. 
However, if it's never connected to the destination, it never gets pulled and
thus stays around forever.  But if we know when the source would end playing
(because we know the duration and it's not looping) and the context time has
passed the end time, we can stop the node, allowing it to be collected.  So we
don't have to be super-precise about when to stop it; we just want it to stop
eventually.

For the typical case where the node is connected, normal processing will stop
the node, so this code shouldn't be called.  The bug is that we didn't correctly
account for the fact that the m_startTime may actually be one quantum behind the
actual play start time so the endTime computation was off, prematurely stopping
the source without an event.

(I should probably add a comment on what handleStoppableSourceNode is really
meant to do.)

+
         if (context()->currentTime() > stopTime) {
             // The context time has passed the time when the source nodes should have stopped
             // playing. Stop the node now and deref it. (But don't run the onEnded event because the
             // source never actually played.)
             finishWithoutOnEnded();
         }
     }
 }
 
 void AudioBufferSourceHandler::finish()
@@ skipping 88 matching lines @@
 }
 
 void AudioBufferSourceNode::start(double when, double grainOffset, double grainDuration, ExceptionState& exceptionState)
 {
     audioBufferSourceHandler().start(when, grainOffset, grainDuration, exceptionState);
 }
 
 } // namespace blink
 
 #endif // ENABLE(WEB_AUDIO)