Bug 1454625 - Don't update settings from constraints if already updated from frame. r=padenot
authorAndreas Pehrson <pehrsons@mozilla.com>
Tue, 17 Apr 2018 17:09:11 +0200
changeset 414308 a3c07c7bfce3ca859fafdf7fc194c23ba1f67fc9
parent 414307 197abd266c8b2b6ba61ebbe9fefa6e72a8b5e273
child 414309 67cef646951c79d7eef1f3d3d3ad713ef6be5f24
push id33865
push userbtara@mozilla.com
push dateWed, 18 Apr 2018 22:35:14 +0000
treeherdermozilla-central@e4c4c8159924 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1454625
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1454625 - Don't update settings from constraints if already updated from frame. r=padenot MozReview-Commit-ID: KgiPUnXCWoM
dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
dom/media/webrtc/MediaEngineRemoteVideoSource.h
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -37,16 +37,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();
 }
@@ -295,44 +296,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)
@@ -619,19 +627,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();
--- 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.