Bug 1213517 - Take highest ideal value from competing width, height and frameRate. draft
authorJan-Ivar Bruaroey <jib@mozilla.com>
Thu, 07 Jul 2016 08:36:05 -0400
changeset 388784 5f9c07e37783f9ffb427c91cf334998cb1e5c2bb
parent 388783 f58083bd964e5b88883f0cf175bff9f9ae68ece9
child 388887 376d9213aaa70e796a307d1297ac996d92e1d347
child 388959 c8d91ac38d463a2ce235eba382b86ead02ce3ea5
push id23232
push userjbruaroey@mozilla.com
push dateSun, 17 Jul 2016 21:00:46 +0000
bugs1213517
milestone50.0a1
Bug 1213517 - Take highest ideal value from competing width, height and frameRate. MozReview-Commit-ID: JkR2rDDeFz1
dom/media/webrtc/MediaTrackConstraints.cpp
dom/media/webrtc/MediaTrackConstraints.h
--- a/dom/media/webrtc/MediaTrackConstraints.cpp
+++ b/dom/media/webrtc/MediaTrackConstraints.cpp
@@ -280,27 +280,29 @@ NormalizedConstraints::NormalizedConstra
   if (aOther.mAdvanced.WasPassed()) {
     for (auto& entry : aOther.mAdvanced.Value()) {
       mAdvanced.AppendElement(NormalizedConstraintSet(entry, true));
     }
   }
 }
 
 // Merge constructor. Create net constraints out of merging a set of others.
+// This is only used to resolve competing constraints from concurrent requests,
+// something the spec doesn't cover.
 
 NormalizedConstraints::NormalizedConstraints(
     const nsTArray<const NormalizedConstraints*>& aOthers)
   : NormalizedConstraintSet(*aOthers[0])
   , mBadConstraint(nullptr)
 {
   // Create a list of member pointers.
   nsTArray<MemberPtrType> list;
   NormalizedConstraints dummy(MediaTrackConstraints(), &list);
 
-  // Do intersection of all required constraints, and average of ideals.
+  // Do intersection of all required constraints, and average of ideals,
 
   for (uint32_t i = 1; i < aOthers.Length(); i++) {
     auto& other = *aOthers[i];
 
     for (auto& memberPtr : list) {
       auto& member = this->*memberPtr;
       auto& otherMember = other.*memberPtr;
 
@@ -312,16 +314,42 @@ NormalizedConstraints::NormalizedConstra
 
     for (auto& entry : other.mAdvanced) {
       mAdvanced.AppendElement(entry);
     }
   }
   for (auto& memberPtr : list) {
     (this->*memberPtr).FinalizeMerge();
   }
+
+  // ...except for resolution and frame rate where we take the highest ideal.
+  // This is a bit of a hack based on the perception that people would be more
+  // surprised if they were to get lower resolution than they ideally requested.
+  //
+  // The spec gives browsers leeway here, saying they "SHOULD use the one with
+  // the smallest fitness distance", and also does not directly address the
+  // problem of competing constraints at all. There is no real web interop issue
+  // here since this is more about interop with other tabs on the same browser.
+  //
+  // We should revisit this logic once we support downscaling of resolutions and
+  // decimating of frame rates, per track.
+
+  for (auto& other : aOthers) {
+    mWidth.TakeHighestIdeal(other->mWidth);
+    mHeight.TakeHighestIdeal(other->mHeight);
+
+    // Consider implicit 30 fps default in comparison of competing constraints.
+    // Avoids 160x90x10 and 640x480 becoming 1024x768x10 (fitness distance flaw)
+    // This pretty much locks in 30 fps or higher, except for single-tab use.
+    auto frameRate = other->mFrameRate;
+    if (frameRate.mIdeal.isNothing()) {
+      frameRate.mIdeal.emplace(30);
+    }
+    mFrameRate.TakeHighestIdeal(frameRate);
+  }
 }
 
 FlattenedConstraints::FlattenedConstraints(const NormalizedConstraints& aOther)
 : NormalizedConstraintSet(aOther)
 {
   for (auto& set : aOther.mAdvanced) {
     // Must only apply compatible i.e. inherently non-overconstraining sets
     // This rule is pretty much why this code is centralized here.
--- a/dom/media/webrtc/MediaTrackConstraints.h
+++ b/dom/media/webrtc/MediaTrackConstraints.h
@@ -113,16 +113,25 @@ public:
     }
     void FinalizeMerge() override
     {
       if (mMergeDenominator) {
         *mIdeal /= mMergeDenominator;
         mMergeDenominator = 0;
       }
     }
+    void TakeHighestIdeal(const Range& aOther) {
+      if (aOther.mIdeal.isSome()) {
+        if (mIdeal.isNothing()) {
+          mIdeal.emplace(aOther.Get(0));
+        } else {
+          *mIdeal = std::max(Get(0), aOther.Get(0));
+        }
+      }
+    }
   private:
     bool Merge(const BaseRange& aOther) override {
       return Merge(static_cast<const Range&>(aOther));
     }
 
     uint32_t mMergeDenominator;
   };