# HG changeset patch
# User Andreas Pehrson <pehrsons@mozilla.com>
# Date 1523977751 -7200
# Node ID a3c07c7bfce3ca859fafdf7fc194c23ba1f67fc9
# Parent  d1874cd6122a62c04e4a4f800f616e338a7bb97f
Bug 1454625 - Don't update settings from constraints if already updated from frame. r=padenot

MozReview-Commit-ID: KgiPUnXCWoM

diff --git a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -36,16 +36,17 @@ MediaEngineRemoteVideoSource::MediaEngin
     bool aScary)
   : mCaptureIndex(aIndex)
   , mMediaSource(aMediaSource)
   , mCapEngine(aCapEngine)
   , mScary(aScary)
   , mMutex("MediaEngineRemoteVideoSource::mMutex")
   , mRescalingBufferPool(/* zero_initialize */ false,
                          /* max_number_of_buffers */ 1)
+  , mSettingsUpdatedByFrame(MakeAndAddRef<media::Refcountable<AtomicBool>>())
   , mSettings(MakeAndAddRef<media::Refcountable<MediaTrackSettings>>())
 {
   MOZ_ASSERT(aMediaSource != MediaSourceEnum::Other);
   mSettings->mWidth.Construct(0);
   mSettings->mHeight.Construct(0);
   mSettings->mFrameRate.Construct(0);
   Init();
 }
@@ -294,44 +295,51 @@ MediaEngineRemoteVideoSource::Start(cons
   MOZ_ASSERT(mStream);
   MOZ_ASSERT(IsTrackIDExplicit(mTrackID));
 
   {
     MutexAutoLock lock(mMutex);
     mState = kStarted;
   }
 
+  mSettingsUpdatedByFrame->mValue = false;
+
   if (camera::GetChildAndCall(&camera::CamerasChild::StartCapture,
                               mCapEngine, mCaptureIndex, mCapability, this)) {
     LOG(("StartCapture failed"));
     MutexAutoLock lock(mMutex);
     mState = kStopped;
     return NS_ERROR_FAILURE;
   }
 
   NS_DispatchToMainThread(NS_NewRunnableFunction(
       "MediaEngineRemoteVideoSource::SetLastCapability",
-      [settings = mSettings, source = mMediaSource, cap = mCapability]() mutable {
+      [settings = mSettings,
+       updated = mSettingsUpdatedByFrame,
+       source = mMediaSource,
+       cap = mCapability]() mutable {
     switch (source) {
       case dom::MediaSourceEnum::Screen:
       case dom::MediaSourceEnum::Window:
       case dom::MediaSourceEnum::Application:
         // Undo the hack where ideal and max constraints are crammed together
         // in mCapability for consumption by low-level code. We don't actually
         // know the real resolution yet, so report min(ideal, max) for now.
         // TODO: This can be removed in bug 1453269.
         cap.width = std::min(cap.width >> 16, cap.width & 0xffff);
         cap.height = std::min(cap.height >> 16, cap.height & 0xffff);
         break;
       default:
         break;
     }
 
-    settings->mWidth.Value() = cap.width;
-    settings->mHeight.Value() = cap.height;
+    if (!updated->mValue) {
+      settings->mWidth.Value() = cap.width;
+      settings->mHeight.Value() = cap.height;
+    }
     settings->mFrameRate.Value() = cap.maxFPS;
   }));
 
   return NS_OK;
 }
 
 nsresult
 MediaEngineRemoteVideoSource::Stop(const RefPtr<const AllocationHandle>& aHandle)
@@ -602,19 +610,23 @@ MediaEngineRemoteVideoSource::DeliverFra
             frame_num++, aProps.width(), aProps.height(), dst_width, dst_height,
             aProps.rotation(), aProps.timeStamp(), aProps.ntpTimeMs(),
             aProps.renderTimeMs()));
 #endif
 
   if (mImageSize.width != dst_width || mImageSize.height != dst_height) {
     NS_DispatchToMainThread(NS_NewRunnableFunction(
         "MediaEngineRemoteVideoSource::FrameSizeChange",
-        [settings = mSettings, dst_width, dst_height]() mutable {
+        [settings = mSettings,
+         updated = mSettingsUpdatedByFrame,
+         dst_width,
+         dst_height]() mutable {
       settings->mWidth.Value() = dst_width;
       settings->mHeight.Value() = dst_height;
+      updated->mValue = true;
     }));
   }
 
   {
     MutexAutoLock lock(mMutex);
     // implicitly releases last image
     mImage = image.forget();
     mImageSize = mImage->GetSize();
diff --git a/dom/media/webrtc/MediaEngineRemoteVideoSource.h b/dom/media/webrtc/MediaEngineRemoteVideoSource.h
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.h
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.h
@@ -222,16 +222,25 @@ private:
   // incoming images. Cameras IPC thread only.
   webrtc::I420BufferPool mRescalingBufferPool;
 
   // The intrinsic size of the latest captured image, so we can feed black
   // images of the same size while stopped.
   // Set under mMutex on the Cameras IPC thread. Accessed under one of the two.
   gfx::IntSize mImageSize = gfx::IntSize(0, 0);
 
+  struct AtomicBool {
+    Atomic<bool> mValue;
+  };
+
+  // True when resolution settings have been updated from a real frame's
+  // resolution. Threadsafe.
+  // TODO: This can be removed in bug 1453269.
+  const RefPtr<media::Refcountable<AtomicBool>> mSettingsUpdatedByFrame;
+
   // The current settings of this source.
   // Note that these may be different from the settings of the underlying device
   // since we scale frames to avoid fingerprinting.
   // Members are main thread only.
   const RefPtr<media::Refcountable<dom::MediaTrackSettings>> mSettings;
 
   // The capability currently chosen by constraints of the user of this source.
   // Set under mMutex on the owning thread. Accessed under one of the two.