1439529 - No video stream on webRTC - Xiaomi & Huawei devices

Status: VERIFIED FIXED

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

Description

[Oana Horvath [:ohorvath]]

Attachment: Screenshot_2018-02-20-10-19-44.png

Devices:
Huawei MediaPad M2 (Android 5.1.1)
Huawei Honor 8 (Android 7.0)
Huawei P9 Lite (Android 6.0)
Huawei Honor 5X (Android 5.1.1)
Xiaomi Mi Pad 2 (Android 5.1)

Build: Nightly 60.0a1 (2018-02-20) & Beta 59.0b11;

Steps to reproduce:
1. Go to talky.io and enter a room.
2. Share camera and mic.
3. Check the video streaming.

Expected result:
Video and audio streaming work.

Actual result:
Video streaming doesn't work. "No video source" error is displayed.
There's an extra microphone permission request displayed on these devices compared to others (see screenshot).

Notes:
Doesn't reproduce on other devices I've checked:
LG G4 (Android 5.1)
Samsung Galaxy Tab 3 (Android 7.0)
LG Nexus 5 (Android 6.0.1)

Ran a bisection on mozregression and found the following cause: 
2018-02-20T13:48:27: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=879aa8e4a7abc87eb2053d07666e92a38afbb674&full=1
2018-02-20T13:48:28: DEBUG : Found commit message:
Bug 1433552 - Allow zeroes to CamerasParent's FeasibilityDistance functions. r=jib

Getting zeroes here is rare, but the numbers come from a platform API so no
guarantees are given for them. This patch makes it as permissive as possible.

MozReview-Commit-ID: 2bjPRzhk1L7

Comment 1

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

Looks like a regression from bug 1433552 I guess. Andreas, can you have a look?

Comment 2

[Andreas Pehrson [:pehrsons]]

I assume "No video source" means we reject gUM with a NotFoundError.

That's weird, especially coming from that bug, but I'll try to dig deeper into where that error could be coming from.

Comment 3

[Andreas Pehrson [:pehrsons]]

Oana, can you confirm whether there is a difference in selecting "Back facing camera" or "Front facing camera" in the prompt?

I can reproduce with front facing but not back facing on a Samsung Galaxy S8.

Comment 4

[Oana Horvath [:ohorvath]]

There is a difference indeed, with the back camera I noticed that it worked 2/3 times, but it's still crashing eventually. With the front camera, it reproduces 3/3 times.

Comment 5

[Andreas Pehrson [:pehrsons]]

Oh a crash, that sounds like something else. Do you have the crash report for that one?

Comment 6

[Oana Horvath [:ohorvath]]

I'm sorry, I made a confusion and thought about Bug 1440255 when I rushed to comment. No crash in this case :)
Checked on more devices and this is what I've found:
The issue happens on both front & back facing camera on Huawei & Xiaomi devices.
On Samsung Galaxy Note 4 (Android 5.0.1) it only reproduces with the back-facing camera.
On Samsung Galaxy S8 (Android 7.0) thou, it is as you said: only reproduces with the front-facing camera.

Comment 7

[Andreas Pehrson [:pehrsons]]

Great, thanks, then I have a device to reproduce with!

Comment 8

[Andreas Pehrson [:pehrsons]]

I have identified one accidental logic change in bug 1433552. When we request 640x480 this would cause us to select the closest higher resolution instead, which for my galaxy s8 is 720x480@30. It's unclear why, but this fails to start.

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1439529 - Same resolution is also feasible.

Review commit: https://reviewboard.mozilla.org/r/223090/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223090/

Comment 10

[Andreas Pehrson [:pehrsons]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1eb816ee5c65d767329a13fb896c2fd75c8a75fa

Comment 11

[Andreas Pehrson [:pehrsons]]

I also need to point out that with this fix we'll now get a stream of resolution 480x480 on Android on [1]. I also see this on 59 beta 9, so before the regression range in comment 0 came into effect (beta 10). On 58 we get a stream at 480x640 as I'd expect. I would think this is a regression from bug 1388219 because we don't handle a camera rotated 90 or 270 degrees like is common in a phone but not on desktop.

Oana, could you help confirm the regression range of this? In either case I reckon we need a new bug for this issue. Thanks!


[1] https://jsfiddle.net/d8dw0sw9/8/

Comment 12

[Oana Horvath [:ohorvath]]

(In reply to Andreas Pehrson [:pehrsons] from comment #11)
> I also need to point out that with this fix we'll now get a stream of
> resolution 480x480 on Android on [1]. I also see this on 59 beta 9, so
> before the regression range in comment 0 came into effect (beta 10). On 58
> we get a stream at 480x640 as I'd expect. I would think this is a regression
> from bug 1388219 because we don't handle a camera rotated 90 or 270 degrees
> like is common in a phone but not on desktop.
> 
> Oana, could you help confirm the regression range of this? In either case I
> reckon we need a new bug for this issue. Thanks!
> 
> 
> [1] https://jsfiddle.net/d8dw0sw9/8/

Logged Bug 1441145 for this.

Comment 13

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

Comment on attachment 8953944 [details]
Bug 1439529 - Same resolution is also feasible.

https://reviewboard.mozilla.org/r/223090/#review229160

Lgtm.

::: dom/media/systemservices/CamerasParent.cpp:63
(Diff revision 1)
> -  if (candidate > requested) {
> +  if (candidate >= requested) {
>      // This is a good case, the candidate is higher than the requested
>      // resolution which makes it feasible.

Just fix the comment to match.

Comment 14

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8953944 [details]
Bug 1439529 - Same resolution is also feasible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223090/diff/1-2/

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b419a3421162
Same resolution is also feasible. r=jib

Comment 16

[Andreea Pavel [:apavel]]

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

Comment 17

[Oana Horvath [:ohorvath]]

Verified as fixed on Nightly 60 (2018-02-28).
Devices:
Huawei Honor 8 (Android 7.0)
Huawei Honor 5X (Android 5.1.1)
Samsung Galaxy S8 (Android 7.0)

Comment 18

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8953944 [details]
Bug 1439529 - Same resolution is also feasible.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1433552
[User impact if declined]: android users might not be able to use the camera
[Is this code covered by automated tests?]: yes, but not in a way that fails. This particular failure has only been observed for certain android devices.
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: yes, see comment 0
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial change
[String changes made/needed]: none

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8953944 [details]
Bug 1439529 - Same resolution is also feasible.

Fix for a regression from an earlier 59 beta. 
Let's get this into beta 14.

Comment 20

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4742bec11514

Comment 22

[Oana Horvath [:ohorvath]]

Verified as fixed on Beta 59b15.
Devices:
HTC 10 (Android 8.0)
Huawei P9 Lite (Android 6.0)
Samsung Galaxy S8 (Android 7.0)
Huawei Honor 5X (Android 5.1.1)
Xiaomi Mi Pad 2 (Android 5.1)