Bug 1299515 - Fix MediaEngineDefault. r?padenot draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Tue, 28 Nov 2017 11:22:21 +0100
changeset 705123 acee1bc87dfce46d2c82b049c2c5d2df1496be3b
parent 705122 723352ff415cb25a6571c5b53e1dc6f5d8dc9bdd
child 705124 6fcc0bae37cd11e074e0f3258f2327f314d781a3
push id91361
push userbmo:apehrson@mozilla.com
push dateWed, 29 Nov 2017 13:44:11 +0000
reviewerspadenot
bugs1299515
milestone59.0a1
Bug 1299515 - Fix MediaEngineDefault. r?padenot There was nothing in MediaEngineDefaultAudio handling the case when the stream was blocked (and NotifyPull asked for a negative delta). Then I also found that MediaEngineDefaultVideo was lacking some locking and wouldn't forward the image size when stopped (null image). This patch fixes all these issues. MozReview-Commit-ID: E2VF0wwOArq
dom/media/webrtc/MediaEngineDefault.cpp
--- a/dom/media/webrtc/MediaEngineDefault.cpp
+++ b/dom/media/webrtc/MediaEngineDefault.cpp
@@ -124,16 +124,17 @@ MediaEngineDefaultVideoSource::Allocate(
   *aOutHandle = nullptr;
   return NS_OK;
 }
 
 nsresult
 MediaEngineDefaultVideoSource::Deallocate(AllocationHandle* aHandle)
 {
   MOZ_ASSERT(!aHandle);
+  MonitorAutoLock lock(mMonitor);
   if (mState != kStopped && mState != kAllocated) {
     return NS_ERROR_FAILURE;
   }
   mState = kReleased;
   mImage = nullptr;
   return NS_OK;
 }
 
@@ -211,16 +212,17 @@ MediaEngineDefaultVideoSource::Start(Sou
 
   return NS_OK;
 }
 
 nsresult
 MediaEngineDefaultVideoSource::Stop(SourceMediaStream* aStream,
                                     TrackID aID)
 {
+  MonitorAutoLock lock(mMonitor);
   if (mState != kStarted) {
     return NS_ERROR_FAILURE;
   }
   if (!mTimer) {
     return NS_ERROR_FAILURE;
   }
 
   mTimer->Cancel();
@@ -311,32 +313,37 @@ MediaEngineDefaultVideoSource::NotifyPul
                                           SourceMediaStream *aSource,
                                           TrackID aID,
                                           StreamTime aDesiredTime,
                                           const PrincipalHandle& aPrincipalHandle)
 {
   // AppendFrame takes ownership of `segment`
   VideoSegment segment;
   MonitorAutoLock lock(mMonitor);
-  if (mState != kStarted) {
+  if (mState != kStarted && mState != kStopped) {
     return;
   }
 
   // Note: we're not giving up mImage here
   RefPtr<layers::Image> image = mImage;
   StreamTime delta = aDesiredTime - aSource->GetEndOfAppendedData(aID);
+  if (delta <= 0) {
+    // Nothing to do
+    return;
+  }
 
-  if (delta > 0) {
-    // nullptr images are allowed
-    IntSize size(image ? mOpts.mWidth : 0, image ? mOpts.mHeight : 0);
-    segment.AppendFrame(image.forget(), delta, size, aPrincipalHandle);
-    // This can fail if either a) we haven't added the track yet, or b)
-    // we've removed or finished the track.
-    aSource->AppendToTrack(aID, &segment);
-  }
+  MOZ_ASSERT(mOpts.mWidth > 0);
+  MOZ_ASSERT(mOpts.mHeight > 0);
+  IntSize size(mOpts.mWidth, mOpts.mHeight);
+
+  // nullptr images are allowed
+  segment.AppendFrame(image.forget(), delta, size, aPrincipalHandle);
+  // This can fail if either a) we haven't added the track yet, or b)
+  // we've removed or finished the track.
+  aSource->AppendToTrack(aID, &segment);
 }
 
 /**
  * Default audio source.
  */
 
 NS_IMPL_ISUPPORTS0(MediaEngineDefaultAudioSource)
 
@@ -485,16 +492,21 @@ MediaEngineDefaultAudioSource::NotifyPul
                                           TrackID aID,
                                           StreamTime aDesiredTime,
                                           const PrincipalHandle& aPrincipalHandle)
 {
   AudioSegment segment;
   // avoid accumulating rounding errors
   TrackTicks desired = aSource->TimeToTicksRoundUp(AUDIO_RATE, aDesiredTime);
   TrackTicks delta = desired - mLastNotify;
+  if (delta <= 0) {
+    // Nothing to do
+    return;
+  }
+
   mLastNotify += delta;
   AppendToSegment(segment, delta, aPrincipalHandle);
   aSource->AppendToTrack(aID, &segment);
 }
 
 void
 MediaEngineDefault::EnumerateVideoDevices(dom::MediaSourceEnum aMediaSource,
                                           nsTArray<RefPtr<MediaEngineVideoSource> >* aVSources) {