1623987 - Front/back camera flipping is broken in Fenix

Status: VERIFIED FIXED

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

Description

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

Camera tracks are missing label, and the facingMode constraint is broken in Fenix.

STR 1:

1. Open https://fiddle.jshell.net/jib1/LbtxeLvw/show (to work around bug 1616729)
2. Click Start Camera! button and allow camera
3. Click EnumerateDevices! button

Expected result (like on Fennec):

...
videoinput: Camera 0, Facing back, Orientation 270
videoinput: Camera 1, Facing front, Orientation 270

(or similar)

Actual result:

No videoinput labels at all. This breaks our facingMode implementation:

STR 2:

1. Open https://fiddle.jshell.net/jib1/fsexmu8v/ and click Front or Back button and allow.

Expected result (like on Fennec):

• Front or back camera video shown

Actual result:

• OverconstrainedError: Constraints could not be satisfied.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

This is media code, so moving over there.

Dan could this be fallout from the WebRTC update?

Comment 2

[Dan Minor [:dminor]]

This ended up being a little trickier than I expected. The camera name format we use to determine facing mode is what the Camera1 API provides but not what we get from the Camera2 API. The emulators only support Camera1, so things were still working on the emulators. I ended up prepending the facing mode to the beginning of the camera name and then removing it again for camera look up.

It is strange that upstream does not provide a field for facing mode in VideoCaptureCapability. Even if it were added upstream, we'd need to rework our IPC code to get access to that field, so fixing things that way seems like a larger project. Maybe we should file a follow up bug for that.

Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d72bb62420bf70581416089234b948ffcd4c8796

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1623987 - Fix front/back camera flipping on Android; r=ng!

We plumb through front/back facing in the device name. When using the Camera1
API this is already present. With Camera2, it is not. Since we later use the
camera name when opening the camera, this prepends the front/back facing
information to the name, so it can easily be removed when we need to open
the camera.

Ideally, upstream would provide a field for this in VideoCaptureCapability and
none of this would be necessary.

Comment 4

[Dan Minor [:dminor]]

I manually tested this using the fiddles above on the emulator and on my phone.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b034f792c135
Fix front/back camera flipping on Android; r=ng

Comment 6

[Noemi Erli[:noemi_erli]]

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

Comment 7

[Dan Minor [:dminor]]

Comment on attachment 9141067 [details]
Bug 1623987 - Fix front/back camera flipping on Android; r=ng!

Beta/Release Uplift Approval Request

• User impact if declined: Sites which rely upon the front/back constraint to select the camera will not work properly.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This should be low risk, it manipulates the camera name to restore the previous behaviour, so the amount of code changed is small.
• String changes made/needed: None

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9141067 [details]
Bug 1623987 - Fix front/back camera flipping on Android; r=ng!

Per Riot discussion with Dan, we're going to let this ride 77 given the impact of the bug and where we are in the 76 cycle.

Comment 9

[Oana Horvath [:ohorvath - out of office]]

Verified as fixed on Fenix Nightly 4/23, GV 77. The labels appear as expected in the bug description.