Bug 833578 - Start AudioSegment playing after first write rather than waiting for AudioStream's buffer to fill. r=roc

content/html/content/src/nsHTMLAudioElement.cpp

@@ -176,14 +176,11 @@ nsHTMLAudioElement::MozWriteAudio(const JS::Value& aData, JSContext* aCx, uint32
   // AudioDataValue is 'float', but it's not worth it for this deprecated API.
   nsAutoArrayPtr<AudioDataValue> audioData(new AudioDataValue[writeLen * mChannels]);
   ConvertAudioSamples(frames, audioData.get(), writeLen * mChannels);
-  if (!mAudioStream->IsStarted()) {
-    mAudioStream->Start();
-  }
   nsresult rv = mAudioStream->Write(audioData.get(), writeLen);
-
   if (NS_FAILED(rv)) {
     return rv;
   }
+  mAudioStream->Start();
 
   // Return the actual amount written.
   *aRetVal = writeLen * mChannels;

content/media/AudioSegment.cpp

@@ -112,6 +112,7 @@ AudioSegment::WriteTo(AudioStream* aOutput)
     }
     aOutput->Write(buf.Elements(), int32_t(c.mDuration));
   }
+  aOutput->Start();
 }
 
 }

content/media/AudioStream.cpp

@@ -59,8 +59,7 @@ class NativeAudioStream : public AudioStream
   uint32_t Available();
   void SetVolume(double aVolume);
   void Drain();
-  nsresult Start();
-  bool IsStarted();
+  void Start();
   void Pause();
   void Resume();
   int64_t GetPosition();
@@ -465,17 +464,10 @@ void NativeAudioStream::Drain()
   }
 }
 
-nsresult NativeAudioStream::Start()
+void NativeAudioStream::Start()
 {
   // Since sydneyaudio is a push API, the playback is started when enough frames
   // have been written. Hence, Start() is a noop.
-  return NS_OK;
-}
-
-bool NativeAudioStream::IsStarted()
-{
-  // See the comment for the |Start()| method.
-  return true;
 }
 
 void NativeAudioStream::Pause()
@@ -620,8 +612,7 @@ class BufferedAudioStream : public AudioStream
   uint32_t Available();
   void SetVolume(double aVolume);
   void Drain();
-  nsresult Start();
-  bool IsStarted();
+  void Start();
   void Pause();
   void Resume();
   int64_t GetPosition();
@@ -657,6 +648,8 @@ private:
   // Caller must own the monitor.
   int64_t GetPositionInFramesUnlocked();
 
+  void StartUnlocked();
+
   // The monitor is held to protect all access to member variables.  Write()
   // waits while mBuffer is full; DataCallback() notifies as it consumes
   // data from mBuffer.  Drain() waits while mState is DRAINING;
@@ -821,9 +814,11 @@ BufferedAudioStream::Write(const AudioDataValue* aBuf, uint32_t aFrames)
     if (bytesToCopy > 0) {
       // If we are not playing, but our buffer is full, start playing to make
       // room for soon-to-be-decoded data.
-      if (!IsStarted()) {
-        MonitorAutoUnlock mon(mMonitor);
-        Start();
+      if (mState != STARTED) {
+        StartUnlocked();
+        if (mState != STARTED) {
+          return NS_ERROR_FAILURE;
+        }
       }
       mon.Wait();
     }
@@ -861,6 +856,7 @@ BufferedAudioStream::Drain()
 {
   MonitorAutoLock mon(mMonitor);
   if (mState != STARTED) {
+    NS_ASSERTION(mBuffer.Available() == 0, "Draining with unplayed audio");
     return;
   }
   mState = DRAINING;
@@ -869,24 +865,30 @@ BufferedAudioStream::Drain()
   }
 }
 
-nsresult
+void
 BufferedAudioStream::Start()
 {
-  if (!mCubebStream) {
-    return NS_ERROR_FAILURE;
-  }
-  if (mState != STARTED) {
-    int r = cubeb_stream_start(mCubebStream);
-    mState = r == CUBEB_OK ? STARTED : ERRORED;
-    return mState == STARTED ? NS_OK : NS_ERROR_FAILURE;
-  }
-  return NS_OK;
+  MonitorAutoLock mon(mMonitor);
+  StartUnlocked();
 }
 
-bool
-BufferedAudioStream::IsStarted()
+void
+BufferedAudioStream::StartUnlocked()
 {
-  return mState == STARTED ? true : false;
+  mMonitor.AssertCurrentThreadOwns();
+  if (!mCubebStream || mState != INITIALIZED) {
+    return;
+  }
+  if (mState != STARTED) {
+    int r;
+    {
+      MonitorAutoUnlock mon(mMonitor);
+      r = cubeb_stream_start(mCubebStream);
+    }
+    if (mState != ERRORED) {
+      mState = r == CUBEB_OK ? STARTED : ERRORED;
+    }
+  }
 }
 
 void

content/media/AudioStream.h

@@ -133,10 +133,7 @@ public:
   virtual void Drain() = 0;
 
   // Start the stream.
-  virtual nsresult Start() = 0;
-
-  // Check if the stream is started.
-  virtual bool IsStarted() = 0;
+  virtual void Start() = 0;
 
   // Return the number of frames written so far in the stream. This allow the
   // caller to check if it is safe to start the stream, if needed.

content/media/MediaDecoderStateMachine.cpp

@@ -961,8 +961,7 @@ static void
 StartAudioStreamPlaybackIfNeeded(AudioStream* aStream)
 {
   // We want to have enough data in the buffer to start the stream.
-  if (!aStream->IsStarted() &&
-      static_cast<double>(aStream->GetWritten()) / aStream->GetRate() >=
+  if (static_cast<double>(aStream->GetWritten()) / aStream->GetRate() >=
       static_cast<double>(AUDIOSTREAM_MIN_WRITE_BEFORE_START_USECS) / USECS_PER_S) {
     aStream->Start();
   }
@@ -1128,9 +1127,7 @@ void MediaDecoderStateMachine::AudioLoop()
     {
       // If the media was too short to trigger the start of the audio stream,
       // start it now.
-      if (!mAudioStream->IsStarted()) {
-        mAudioStream->Start();
-      }
+      mAudioStream->Start();
       // Last frame pushed to audio hardware, wait for the audio to finish,
       // before the audio thread terminates.
       bool seeking = false;