Bug 638807 - Data race on nsBuiltinDecoder::mFrameBufferLength; r=chris.double
authorYury <async.processingjs@yahoo.com>
Mon, 11 Apr 2011 17:15:45 -0400
changeset 67873 71fa806ffd267a33d923992984bec9f2682211fc
parent 67872 1a53dc7c5c20a7415eec653652002c041392c2ef
child 67874 3577f08df0c106cb7297f77d9f04c3df9733a18b
push idunknown
push userunknown
push dateunknown
reviewerschris
bugs638807
milestone2.2a1pre
Bug 638807 - Data race on nsBuiltinDecoder::mFrameBufferLength; r=chris.double
content/media/nsAudioAvailableEventManager.cpp
content/media/nsAudioAvailableEventManager.h
content/media/nsBuiltinDecoder.cpp
content/media/nsBuiltinDecoder.h
content/media/nsBuiltinDecoderStateMachine.cpp
content/media/nsBuiltinDecoderStateMachine.h
--- a/content/media/nsAudioAvailableEventManager.cpp
+++ b/content/media/nsAudioAvailableEventManager.cpp
@@ -77,16 +77,17 @@ public:
   const float mTime;
 };
 
 
 nsAudioAvailableEventManager::nsAudioAvailableEventManager(nsBuiltinDecoder* aDecoder) :
   mDecoder(aDecoder),
   mSignalBuffer(new float[mDecoder->GetFrameBufferLength()]),
   mSignalBufferLength(mDecoder->GetFrameBufferLength()),
+  mNewSignalBufferLength(mSignalBufferLength),
   mSignalBufferPosition(0),
   mMonitor("media.audioavailableeventmanager")
 {
   MOZ_COUNT_CTOR(nsAudioAvailableEventManager);
 }
 
 nsAudioAvailableEventManager::~nsAudioAvailableEventManager()
 {
@@ -114,17 +115,19 @@ void nsAudioAvailableEventManager::Dispa
     NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
   }
 }
 
 void nsAudioAvailableEventManager::QueueWrittenAudioData(SoundDataValue* aAudioData,
                                                          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;
   }
 
   if (!mSignalBuffer ||
       (mSignalBufferPosition == 0 && mSignalBufferLength != currentBufferSize)) {
     if (!mSignalBuffer || (mSignalBufferLength < currentBufferSize)) {
@@ -150,18 +153,16 @@ void nsAudioAvailableEventManager::Queue
     PRUint32 i;
     float *signalBuffer = mSignalBuffer.get() + mSignalBufferPosition;
     for (i = 0; i < signalBufferTail; ++i) {
       signalBuffer[i] = MOZ_CONVERT_SOUND_SAMPLE(audioData[i]);
     }
     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.
       nsAudioAvailableEventRunner* lastPendingEvent =
         (nsAudioAvailableEventRunner*)mPendingEvents[mPendingEvents.Length() - 1].get();
       if (lastPendingEvent->mTime > time) {
         // Clear the queue to start a fresh sequence.
         mPendingEvents.Clear();
@@ -231,8 +232,16 @@ void nsAudioAvailableEventManager::Drain
                (mSignalBufferPosition / mSamplesPerSecond);
   nsCOMPtr<nsIRunnable> lastEvent =
     new nsAudioAvailableEventRunner(mDecoder, mSignalBuffer.forget(),
                                     mSignalBufferLength, time);
   NS_DispatchToMainThread(lastEvent, NS_DISPATCH_NORMAL);
 
   mSignalBufferPosition = 0;
 }
+
+void nsAudioAvailableEventManager::SetSignalBufferLength(PRUint32 aLength)
+{
+  MonitorAutoEnter mon(mMonitor);
+
+  mNewSignalBufferLength = aLength;
+}
+
--- a/content/media/nsAudioAvailableEventManager.h
+++ b/content/media/nsAudioAvailableEventManager.h
@@ -71,35 +71,42 @@ public:
   // machine and audio threads.
   void Clear();
 
   // Fires one last event for any extra samples that didn't fit in a whole
   // framebuffer. This is meant to be called only once when the audio finishes.
   // 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
   // manager).
   nsBuiltinDecoder* mDecoder;
 
   // The number of samples per second.
   float mSamplesPerSecond;
 
   // A buffer for audio data to be dispatched in DOM events.
   nsAutoArrayPtr<float> mSignalBuffer;
 
   // 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;
 
   // The MozAudioAvailable events to be dispatched.  This queue is shared
   // 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;
 };
 
 #endif
--- a/content/media/nsBuiltinDecoder.cpp
+++ b/content/media/nsBuiltinDecoder.cpp
@@ -215,23 +215,40 @@ nsresult nsBuiltinDecoder::Load(nsMediaS
   if (NS_FAILED(mDecoderStateMachine->Init(cloneDonor ?
                                            cloneDonor->mDecoderStateMachine : nsnull))) {
     return NS_ERROR_FAILURE;
   }
   {
     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);
 
   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,
                "Must have state machine to start state machine thread");
   if (mStateMachineThread) {
     return NS_OK;
   }
   nsresult rv = NS_NewThread(getter_AddRefs(mStateMachineThread));
@@ -323,26 +340,23 @@ void nsBuiltinDecoder::AudioAvailable(fl
   if (!mElement->MayHaveAudioAvailableEventListener()) {
     return;
   }
 
   mElement->NotifyAudioAvailable(frameBuffer.forget(), aFrameBufferLength, aTime);
 }
 
 void nsBuiltinDecoder::MetadataLoaded(PRUint32 aChannels,
-                                      PRUint32 aRate,
-                                      PRUint32 aFrameBufferLength)
+                                      PRUint32 aRate)
 {
   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;
   {
     MonitorAutoEnter mon(mMonitor);
     mDuration = mDecoderStateMachine ? mDecoderStateMachine->GetDuration() : -1;
     // Duration has changed so we should recompute playback rate
     UpdatePlaybackRate();
--- a/content/media/nsBuiltinDecoder.h
+++ b/content/media/nsBuiltinDecoder.h
@@ -298,16 +298,20 @@ public:
 
   virtual void NotifyDataArrived(const char* aBuffer, PRUint32 aLength, PRUint32 aOffset) = 0;
 
   // Causes the state machine to switch to buffering state, and to
   // immediately stop playback and buffer downloaded data. Must be called
   // 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
 {
   // ISupports
   NS_DECL_ISUPPORTS
 
   // nsIObserver
@@ -441,16 +445,20 @@ class nsBuiltinDecoder : public nsMediaD
     }
     return NS_ERROR_FAILURE;
   }
 
   virtual void NotifyDataArrived(const char* aBuffer, PRUint32 aLength, PRUint32 aOffset) {
     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.
   PlayState GetState() {
     return mPlayState;
   }
 
   // Stop updating the bytes downloaded for progress notifications. Called
@@ -486,18 +494,17 @@ class nsBuiltinDecoder : public nsMediaD
   // Change to a new play state. This updates the mState variable and
   // notifies any thread blocking on this object's monitor of the
   // change. Call on the main thread only.
   void ChangeState(PlayState aState);
 
   // Called when the metadata from the media file has been read.
   // Call on the main thread only.
   void MetadataLoaded(PRUint32 aChannels,
-                      PRUint32 aRate,
-                      PRUint32 aFrameBufferLength);
+                      PRUint32 aRate);
 
   // Called when the first frame has been loaded.
   // Call on the main thread only.
   void FirstFrameLoaded();
 
   // Called when the video has completed playing.
   // Call on the main thread only.
   void PlaybackEnded();
--- a/content/media/nsBuiltinDecoderStateMachine.cpp
+++ b/content/media/nsBuiltinDecoderStateMachine.cpp
@@ -149,33 +149,31 @@ static PRInt64 DurationToMs(TimeDuration
 }
 
 class nsAudioMetadataEventRunner : public nsRunnable
 {
 private:
   nsCOMPtr<nsBuiltinDecoder> mDecoder;
 public:
   nsAudioMetadataEventRunner(nsBuiltinDecoder* aDecoder, PRUint32 aChannels,
-                             PRUint32 aRate, PRUint32 aFrameBufferLength) :
+                             PRUint32 aRate) :
     mDecoder(aDecoder),
     mChannels(aChannels),
-    mRate(aRate),
-    mFrameBufferLength(aFrameBufferLength)
+    mRate(aRate)
   {
   }
 
   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,
                                                            nsBuiltinDecoderReader* aReader) :
   mDecoder(aDecoder),
   mState(DECODER_STATE_DECODING_METADATA),
   mAudioMonitor("media.audiostream"),
   mCbCrSize(0),
@@ -1026,16 +1024,24 @@ PRInt64 nsBuiltinDecoderStateMachine::Ge
 
     if (start <= currentTime && end >= currentTime) {
       return static_cast<PRInt64>((end - currentTime) * 1000);
     }
   }
   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),
                "Should be on state machine thread.");
   nsMediaStream* stream = mDecoder->GetCurrentStream();
   NS_ENSURE_TRUE(stream, NS_ERROR_NULL_POINTER);
 
   while (PR_TRUE) {
@@ -1083,25 +1089,27 @@ nsresult nsBuiltinDecoderStateMachine::R
 
         if (mState == DECODER_STATE_SHUTDOWN)
           continue;
 
         // Inform the element that we've loaded the metadata and the first frame,
         // 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));
           StartDecoding();
         }
 
         // Start playback.
         if (mDecoder->GetState() == nsBuiltinDecoder::PLAY_STATE_PLAYING) {
--- a/content/media/nsBuiltinDecoderStateMachine.h
+++ b/content/media/nsBuiltinDecoderStateMachine.h
@@ -242,16 +242,20 @@ public:
     mReader->NotifyDataArrived(aBuffer, aLength, aOffset);
   }
 
   PRInt64 GetEndMediaTime() const {
     mDecoder->GetMonitor().AssertCurrentThreadIn();
     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
   // data. The decoder monitor must be held.
   PRBool HasLowDecodedData(PRInt64 aAudioMs) const;
 
   // Returns PR_TRUE if we're running low on data which is not yet decoded.
   // The decoder monitor must be held.