Bug 828828 - Use monitor around all accesses to stream array. r=derf, a=akeybl
authorRandell Jesup <rjesup@jesup.org>
Thu, 10 Jan 2013 11:52:53 -0500
changeset 127111 2a6bf87c333e851ec485f298a5cbf28ae90c84b9
parent 127110 a3ed3feed8f43a76e62e021aba1ea35fbabc5c30
child 127112 3e7b5800b2d56bd644cc6265d09ea8a944817066
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersderf, akeybl
bugs828828
milestone20.0a2
Bug 828828 - Use monitor around all accesses to stream array. r=derf, a=akeybl
content/media/webrtc/MediaEngineWebRTC.h
content/media/webrtc/MediaEngineWebRTCAudio.cpp
--- a/content/media/webrtc/MediaEngineWebRTC.h
+++ b/content/media/webrtc/MediaEngineWebRTC.h
@@ -134,16 +134,20 @@ private:
   webrtc::CaptureCapability mCapability; // Doesn't work on OS X.
 
   int mCaptureIndex;
   bool mCapabilityChosen;
   int mWidth, mHeight;
   TrackID mTrackID;
   TrackTicks mLastEndTime;
 
+  // mMonitor protects mImage access/changes, and transitions of mState
+  // from kStarted to kStopped (which are combined with EndTrack() and
+  // image changes).  Note that mSources is not accessed from other threads
+  // for video and is not protected.
   mozilla::ReentrantMonitor mMonitor; // Monitor for processing WebRTC frames.
   nsTArray<SourceMediaStream *> mSources; // When this goes empty, we shut down HW
 
   int mFps; // Track rate (30 fps by default)
   int mMinFps; // Min rate we want to accept
   bool mInitDone;
   bool mInSnapshotMode;
   nsString* mSnapshotPath;
@@ -210,16 +214,19 @@ private:
   void Init();
   void Shutdown();
 
   webrtc::VoiceEngine* mVoiceEngine;
   webrtc::VoEBase* mVoEBase;
   webrtc::VoEExternalMedia* mVoERender;
   webrtc::VoENetwork*  mVoENetwork;
 
+  // mMonitor protects mSources[] access/changes, and transitions of mState
+  // from kStarted to kStopped (which are combined with EndTrack()).
+  // mSources[] is accessed from webrtc threads.
   mozilla::ReentrantMonitor mMonitor;
   nsTArray<SourceMediaStream *> mSources; // When this goes empty, we shut down HW
 
   int mCapIndex;
   int mChannel;
   TrackID mTrackID;
   bool mInitDone;
 
--- a/content/media/webrtc/MediaEngineWebRTCAudio.cpp
+++ b/content/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ -84,17 +84,20 @@ MediaEngineWebRTCAudioSource::Deallocate
 
 nsresult
 MediaEngineWebRTCAudioSource::Start(SourceMediaStream* aStream, TrackID aID)
 {
   if (!mInitDone || !aStream) {
     return NS_ERROR_FAILURE;
   }
 
-  mSources.AppendElement(aStream);
+  {
+    ReentrantMonitorAutoEnter enter(mMonitor);
+    mSources.AppendElement(aStream);
+  }
 
   AudioSegment* segment = new AudioSegment();
   segment->Init(CHANNELS);
   aStream->AddTrack(aID, SAMPLE_FREQUENCY, 0, segment);
   aStream->AdvanceKnownTracksTime(STREAM_TIME_MAX);
   LOG(("Initial audio"));
   mTrackID = aID;
 
@@ -114,32 +117,33 @@ MediaEngineWebRTCAudioSource::Start(Sour
   mVoERender->RegisterExternalMediaProcessing(mChannel, webrtc::kRecordingPerChannel, *this);
 
   return NS_OK;
 }
 
 nsresult
 MediaEngineWebRTCAudioSource::Stop(SourceMediaStream *aSource, TrackID aID)
 {
-  if (!mSources.RemoveElement(aSource)) {
-    // Already stopped - this is allowed
-    return NS_OK;
-  }
-  if (!mSources.IsEmpty()) {
-    return NS_OK;
-  }
-  if (mState != kStarted) {
-    return NS_ERROR_FAILURE;
-  }
-  if (!mVoEBase) {
-    return NS_ERROR_FAILURE;
-  }
-
   {
     ReentrantMonitorAutoEnter enter(mMonitor);
+
+    if (!mSources.RemoveElement(aSource)) {
+      // Already stopped - this is allowed
+      return NS_OK;
+    }
+    if (!mSources.IsEmpty()) {
+      return NS_OK;
+    }
+    if (mState != kStarted) {
+      return NS_ERROR_FAILURE;
+    }
+    if (!mVoEBase) {
+      return NS_ERROR_FAILURE;
+    }
+
     mState = kStopped;
     aSource->EndTrack(aID);
   }
 
   mVoERender->DeRegisterExternalMediaProcessing(mChannel, webrtc::kRecordingPerChannel);
 
   if (mVoEBase->StopSend(mChannel)) {
     return NS_ERROR_FAILURE;