Bug 1213517 - Clamp competing ideal values before considering them to avoid outliers distorting result. draft
authorJan-Ivar Bruaroey <jib@mozilla.com>
Thu, 07 Jul 2016 10:45:34 -0400
changeset 388783 f58083bd964e5b88883f0cf175bff9f9ae68ece9
parent 388782 e184aff156d1ed338e0b0a9eedb336f732fb0bd0
child 388784 5f9c07e37783f9ffb427c91cf334998cb1e5c2bb
push id23232
push userjbruaroey@mozilla.com
push dateSun, 17 Jul 2016 21:00:46 +0000
bugs1213517
milestone50.0a1
Bug 1213517 - Clamp competing ideal values before considering them to avoid outliers distorting result. MozReview-Commit-ID: 3RNgKfDpJxL
dom/media/webrtc/MediaTrackConstraints.cpp
dom/media/webrtc/MediaTrackConstraints.h
--- a/dom/media/webrtc/MediaTrackConstraints.cpp
+++ b/dom/media/webrtc/MediaTrackConstraints.cpp
@@ -42,25 +42,26 @@ NormalizedConstraintSet::Range<bool>::Me
   Intersect(aOther);
 
   // To avoid "unsafe use of type 'bool'", we keep counter in mMergeDenominator
   uint32_t counter = mMergeDenominator >> 16;
   uint32_t denominator = mMergeDenominator & 0xffff;
 
   if (aOther.mIdeal.isSome()) {
     if (mIdeal.isNothing()) {
-      mIdeal.emplace(aOther.mIdeal.value());
-      counter = aOther.mIdeal.value();
+      mIdeal.emplace(aOther.Get(false));
+      counter = aOther.Get(false);
       denominator = 1;
     } else {
       if (!denominator) {
-        counter = mIdeal.value();
+        counter = Get(false);
+        denominator = 1;
       }
-      counter += aOther.mIdeal.value();
-      denominator = std::max(2U, denominator + 1);
+      counter += aOther.Get(false);
+      denominator++;
     }
   }
   mMergeDenominator = ((counter & 0xffff) << 16) + (denominator & 0xffff);
   return true;
 }
 
 template<>
 void
--- a/dom/media/webrtc/MediaTrackConstraints.h
+++ b/dom/media/webrtc/MediaTrackConstraints.h
@@ -90,22 +90,28 @@ public:
     }
     bool Merge(const Range& aOther) {
       if (!Intersects(aOther)) {
         return false;
       }
       Intersect(aOther);
 
       if (aOther.mIdeal.isSome()) {
+        // Ideal values, as stored, may be outside their min max range, so use
+        // clamped values in averaging, to avoid extreme outliers skewing results.
         if (mIdeal.isNothing()) {
-          mIdeal.emplace(aOther.mIdeal.value());
+          mIdeal.emplace(aOther.Get(0));
           mMergeDenominator = 1;
         } else {
-          *mIdeal += aOther.mIdeal.value();
-          mMergeDenominator = std::max(2U, mMergeDenominator + 1);
+          if (!mMergeDenominator) {
+            *mIdeal = Get(0);
+            mMergeDenominator = 1;
+          }
+          *mIdeal += aOther.Get(0);
+          mMergeDenominator++;
         }
       }
       return true;
     }
     void FinalizeMerge() override
     {
       if (mMergeDenominator) {
         *mIdeal /= mMergeDenominator;