997365 - end support for backwards-compatible facingMode constraint on mobile

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Bug 907352 added code that supports backwards compatible constraints for mobile. The intent was to keep this in for one release to aid transition. This bug is filed as a reminder to remove it.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

We can keep this around longer if need be (e.g. for Loop back-end). FYI:

> BACKWARDS-COMPATIBLE OLD FORMAT       | NEW FORMAT
> --------------------------------------+----------------------
> { optional: [{ facingMode:"user" }] } | { facingMode:"user" }
> { mandatory: { facingMode:"user" } }  | { facingMode:"user", require:"facingMode" }
> 

Again, the old format works only on B2G and Android.

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: end support for hold-out mandatory/optional: facingMode constraint on mobile

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: end support for hold-out mandatory/optional: facingMode constraint on mobile (2)

Splinter helped me see I forgot to remove a file entirely.

Comment 4

[Martin Thomson [:mt:]]

Comment on attachment 8556720 [details] [diff] [review]
end support for hold-out mandatory/optional: facingMode constraint on mobile (2)

Review of attachment 8556720 [details] [diff] [review]:
-----------------------------------------------------------------

You need to start using reviewboard.  The UX sucks (apart from the bit where you push to create a review, that's nice), but it's still better than splinter...  In this case, I think that I'm going to trust you that the transplant of the common tests from constraints.js was correct.

::: dom/media/tests/mochitest/test_getUserMedia_constraints.html
@@ -25,4 @@
>  */
> -var desktop_tests = [
> -  { message: "legacy facingMode ignored (desktop)",
> -    constraints: { video: { mandatory: { facingMode:'left' } }, fake: true },

This is still probably a useful test to retain.  Even if 'mandatory' is just another 'somethingunknown', it still holds special meaning by virtue of having been something we respected at one time.

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: constraints.interdiff (NOT FOR LANDING)

(In reply to Martin Thomson [:mt] from comment #4)
> You need to start using reviewboard.  The UX sucks (apart from the bit where
> you push to create a review, that's nice), but it's still better than
> splinter...  In this case, I think that I'm going to trust you that the
> transplant of the common tests from constraints.js was correct.

Trust and verify. :-) Here's a diff against constraints.js showing tests unharmed. Still comfy on the fence about reviewboard. It seems to be getting better though.

> This is still probably a useful test to retain.  Even if 'mandatory' is just
> another 'somethingunknown', it still holds special meaning by virtue of
> having been something we respected at one time.

I almost left it in there, so, sure.

Comment 6

[Martin Thomson [:mt:]]

Note, I'm going to bitrot your patch when I disable the _mobile test to address bug 1063290.

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: end support for hold-out mandatory/optional: facingMode constraint on mobile (3) r=smaug, mt

Incorporate feedback. Carrying forward r=smaug, mt.

Comment 8

[Carsten Book [:Tomcat]]

jib: can you post a try link here thanks!

Comment 9

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=65688f856328

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae5b86a2529d

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ae5b86a2529d

Comment 12

[Eric Shepherd [:sheppy]]

Docs need to reflect this change in the appropriate places.

Comment 13

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Note that this bug is over a year old, and the information is outdated. As of 38, Firefox follows the spec and no longer uses the require keyword mentioned in comment 1.

BACKWARDS-COMPATIBLE OLD FORMAT        | SPEC (38+)
---------------------------------------+----------------------
{ optional: [{ facingMode: "user" }] } | { facingMode: "user" }
                                       | { facingMode: { ideal: "user" } }
{ mandatory: { facingMode: "user" } }  | { facingMode: { exact: "user" } }

See also http://stackoverflow.com/questions/28282385/webrtc-firefox-constraints/28911694#28911694