Bug 638807 - Data race on nsBuiltinDecoder::mFrameBufferLength; r=chris.double

content/media/nsAudioAvailableEventManager.cpp

@@ -82,6 +82,7 @@ nsAudioAvailableEventManager::nsAudioAvailableEventManager(nsBuiltinDecoder* aDe
   mDecoder(aDecoder),
   mSignalBuffer(new float[mDecoder->GetFrameBufferLength()]),
   mSignalBufferLength(mDecoder->GetFrameBufferLength()),
+  mNewSignalBufferLength(mSignalBufferLength),
   mSignalBufferPosition(0),
   mMonitor("media.audioavailableeventmanager")
 {
@@ -119,7 +120,9 @@ void nsAudioAvailableEventManager::QueueWrittenAudioData(SoundDataValue* aAudioD
                                                          PRUint32 aAudioDataLength,
                                                          PRUint64 aEndTimeSampleOffset)
 {
-  PRUint32 currentBufferSize = mDecoder->GetFrameBufferLength();
+  MonitorAutoEnter mon(mMonitor);
+
+  PRUint32 currentBufferSize = mNewSignalBufferLength;
   if (currentBufferSize == 0) {
     NS_WARNING("Decoder framebuffer length not set.");
     return;
@@ -155,8 +158,6 @@ void nsAudioAvailableEventManager::QueueWrittenAudioData(SoundDataValue* aAudioD
     audioData += signalBufferTail;
     audioDataLength -= signalBufferTail;
 
-    MonitorAutoEnter mon(mMonitor);
-
     if (mPendingEvents.Length() > 0) {
       // Check last event timecode to make sure that all queued events
       // are in non-descending sequence.
@@ -236,3 +237,11 @@ void nsAudioAvailableEventManager::Drain(PRUint64 aEndTime)
 
   mSignalBufferPosition = 0;
 }
+
+void nsAudioAvailableEventManager::SetSignalBufferLength(PRUint32 aLength)
+{
+  MonitorAutoEnter mon(mMonitor);
+
+  mNewSignalBufferLength = aLength;
+}
+

content/media/nsAudioAvailableEventManager.h

@@ -76,6 +76,10 @@ public:
   // Called from the state machine thread.
   void Drain(PRUint64 aTime);
 
+  // Sets the size of the signal buffer.
+  // Called from the main and the state machine thread.
+  void SetSignalBufferLength(PRUint32 aLength);
+
 private:
   // The decoder associated with the event manager.  The event manager shares
   // the same lifetime as the decoder (the decoder holds a reference to the
@@ -91,6 +95,9 @@ private:
   // The current size of the signal buffer, may change due to DOM calls.
   PRUint32 mSignalBufferLength;
 
+  // The size of the new signal buffer, may change due to DOM calls.
+  PRUint32 mNewSignalBufferLength;
+
   // The position of the first available item in mSignalBuffer
   PRUint32 mSignalBufferPosition;
 
@@ -98,7 +105,7 @@ private:
   // between the state machine and audio threads.
   nsTArray< nsCOMPtr<nsIRunnable> > mPendingEvents;
 
-  // Monitor for shared access to mPendingEvents queue.
+  // Monitor for shared access to mPendingEvents queue or buffer length.
   Monitor mMonitor;
 };
 

content/media/nsBuiltinDecoder.cpp

@@ -220,6 +220,11 @@ nsresult nsBuiltinDecoder::Load(nsMediaStream* aStream,
     MonitorAutoEnter mon(mMonitor);
     mDecoderStateMachine->SetSeekable(mSeekable);
     mDecoderStateMachine->SetDuration(mDuration);
+    
+    if (mFrameBufferLength > 0) {
+      // The valid mFrameBufferLength value was specified earlier
+      mDecoderStateMachine->SetFrameBufferLength(mFrameBufferLength);
+    }
   }
 
   ChangeState(PLAY_STATE_LOADING);
@@ -227,6 +232,18 @@ nsresult nsBuiltinDecoder::Load(nsMediaStream* aStream,
   return StartStateMachineThread();
 }
 
+nsresult nsBuiltinDecoder::RequestFrameBufferLength(PRUint32 aLength)
+{
+  nsresult res = nsMediaDecoder::RequestFrameBufferLength(aLength);
+  NS_ENSURE_SUCCESS(res,res);
+
+  MonitorAutoEnter mon(mMonitor);
+  if (mDecoderStateMachine) {
+      mDecoderStateMachine->SetFrameBufferLength(aLength);
+  }
+  return res;
+}
+
 nsresult nsBuiltinDecoder::StartStateMachineThread()
 {
   NS_ASSERTION(mDecoderStateMachine,
@@ -328,16 +345,13 @@ void nsBuiltinDecoder::AudioAvailable(float* aFrameBuffer,
 }
 
 void nsBuiltinDecoder::MetadataLoaded(PRUint32 aChannels,
-                                      PRUint32 aRate,
+                                      PRUint32 aRate)
-                                      PRUint32 aFrameBufferLength)
 {
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
   if (mShuttingDown) {
     return;
   }
 
-  mFrameBufferLength = aFrameBufferLength;
-
   // Only inform the element of MetadataLoaded if not doing a load() in order
   // to fulfill a seek, otherwise we'll get multiple metadataloaded events.
   PRBool notifyElement = PR_TRUE;

content/media/nsBuiltinDecoder.h

@@ -303,6 +303,10 @@ public:
   // with the decode monitor held. Called on the state machine thread and
   // the main thread.
   virtual void StartBuffering() = 0;
+
+  // Sets the current size of the framebuffer used in MozAudioAvailable events.
+  // Called on the state machine thread and the main thread.
+  virtual void SetFrameBufferLength(PRUint32 aLength) = 0;
 };
 
 class nsBuiltinDecoder : public nsMediaDecoder
@@ -446,6 +450,10 @@ class nsBuiltinDecoder : public nsMediaDecoder
     return mDecoderStateMachine->NotifyDataArrived(aBuffer, aLength, aOffset);
   }
 
+  // Sets the length of the framebuffer used in MozAudioAvailable events.
+  // The new size must be between 512 and 16384.
+  virtual nsresult RequestFrameBufferLength(PRUint32 aLength);
+
  public:
   // Return the current state. Can be called on any thread. If called from
   // a non-main thread, the decoder monitor must be held.
@@ -491,8 +499,7 @@ class nsBuiltinDecoder : public nsMediaDecoder
   // Called when the metadata from the media file has been read.
   // Call on the main thread only.
   void MetadataLoaded(PRUint32 aChannels,
-                      PRUint32 aRate,
+                      PRUint32 aRate);
-                      PRUint32 aFrameBufferLength);
 
   // Called when the first frame has been loaded.
   // Call on the main thread only.

content/media/nsBuiltinDecoderStateMachine.cpp

@@ -154,23 +154,21 @@ private:
   nsCOMPtr<nsBuiltinDecoder> mDecoder;
 public:
   nsAudioMetadataEventRunner(nsBuiltinDecoder* aDecoder, PRUint32 aChannels,
-                             PRUint32 aRate, PRUint32 aFrameBufferLength) :
+                             PRUint32 aRate) :
     mDecoder(aDecoder),
     mChannels(aChannels),
-    mRate(aRate),
+    mRate(aRate)
-    mFrameBufferLength(aFrameBufferLength)
   {
   }
 
   NS_IMETHOD Run()
   {
-    mDecoder->MetadataLoaded(mChannels, mRate, mFrameBufferLength);
+    mDecoder->MetadataLoaded(mChannels, mRate);
     return NS_OK;
   }
 
   const PRUint32 mChannels;
   const PRUint32 mRate;
-  const PRUint32 mFrameBufferLength;
 };
 
 nsBuiltinDecoderStateMachine::nsBuiltinDecoderStateMachine(nsBuiltinDecoder* aDecoder,
@@ -1031,6 +1029,14 @@ PRInt64 nsBuiltinDecoderStateMachine::GetUndecodedData() const
   return 0;
 }
 
+void nsBuiltinDecoderStateMachine::SetFrameBufferLength(PRUint32 aLength)
+{
+  NS_ASSERTION(aLength >= 512 && aLength <= 16384,
+               "The length must be between 512 and 16384");
+  mDecoder->GetMonitor().AssertCurrentThreadIn();
+  mEventManager.SetSignalBufferLength(aLength);
+}
+
 nsresult nsBuiltinDecoderStateMachine::Run()
 {
   NS_ASSERTION(IsCurrentThread(mDecoder->mStateMachineThread),
@@ -1088,15 +1094,17 @@ nsresult nsBuiltinDecoderStateMachine::Run()
         // setting the default framebuffer size for audioavailable events.  Also,
         // if there is audio, let the MozAudioAvailable event manager know about
         // the metadata.
-        PRUint32 frameBufferLength = mInfo.mAudioChannels * FRAMEBUFFER_LENGTH_PER_CHANNEL;
-        nsCOMPtr<nsIRunnable> metadataLoadedEvent =
-          new nsAudioMetadataEventRunner(mDecoder, mInfo.mAudioChannels,
-                                         mInfo.mAudioRate, frameBufferLength);
-        NS_DispatchToMainThread(metadataLoadedEvent, NS_DISPATCH_NORMAL);
         if (HasAudio()) {
           mEventManager.Init(mInfo.mAudioChannels, mInfo.mAudioRate);
+          // Set the buffer length at the decoder level to be able, to be able
+          // to retrive the value via media element method. The RequestFrameBufferLength
+          // will call the nsBuiltinDecoderStateMachine::SetFrameBufferLength().
+          PRUint32 frameBufferLength = mInfo.mAudioChannels * FRAMEBUFFER_LENGTH_PER_CHANNEL;
           mDecoder->RequestFrameBufferLength(frameBufferLength);
         }
+        nsCOMPtr<nsIRunnable> metadataLoadedEvent =
+          new nsAudioMetadataEventRunner(mDecoder, mInfo.mAudioChannels, mInfo.mAudioRate);
+        NS_DispatchToMainThread(metadataLoadedEvent, NS_DISPATCH_NORMAL);
 
         if (mState == DECODER_STATE_DECODING_METADATA) {
           LOG(PR_LOG_DEBUG, ("%p Changed state from DECODING_METADATA to DECODING", mDecoder));

content/media/nsBuiltinDecoderStateMachine.h

@@ -247,6 +247,10 @@ public:
     return mEndTime;
   }
 
+  // Sets the current frame buffer length for the MozAudioAvailable event.
+  // Accessed on the main and state machine threads.
+  virtual void SetFrameBufferLength(PRUint32 aLength);
+
 protected:
 
   // Returns PR_TRUE if we'v got less than aAudioMs ms of decoded and playable