Bug 1213517 - Take highest ideal value from competing width, height and frameRate.
MozReview-Commit-ID: JkR2rDDeFz1
--- 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;
};