Simplify the workaround for detecting systems with microphone and no audio, and drop unnecessary 1500 exceptions

VERIFIED FIXED in Firefox 42

Status

Hello (Loop)
Client
P2
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla42
Points:
2
Bug Flags:
firefox-backlog +
in-testsuite +

Firefox Tracking Flags

(firefox42 verified)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Now we're logging exceptions, we're seeing a lot of 1500 exceptions from the sdk (unable to publish). We could be getting these from multiple causes, but one cause I do know that's giving false alerts is if a user has only a microphone on desktop.

We currently attempt camera + microphone, and if that fails we fallback to just the microphone. This is because the sdk doesn't yet support the new mediaDevices API (Bug 1138851).

This bug simplifies our workarounds so that the api they do support is simulated and based on mediaDevices.

As this only affects in-browser users, we're fine using the new API to do this.
(Assignee)

Comment 1

3 years ago
Created attachment 8638535 [details] [diff] [review]
Simplify the no-camera work around for Loop that was put in place when we didn't have device enumeration - avoid unnecessary exceptions from the sdk.

See comment 0
Attachment #8638535 - Flags: review?(mdeboer)
Comment on attachment 8638535 [details] [diff] [review]
Simplify the no-camera work around for Loop that was put in place when we didn't have device enumeration - avoid unnecessary exceptions from the sdk.

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

Very nice simplification! With the change mentioned below, r=me.

::: browser/components/loop/content/shared/js/otSdkDriver.js
@@ +78,5 @@
> +          }
> +
> +          var result = [];
> +
> +          if (devices.some(hasAudio)) {

I'm pretty sure that the 'some' use here has very little use here over simple one-time-iteration with forEach - IOW run one loop, not two:

```js
var result = [];
devices.forEach(function(device) {
  if (device.kind === "audioinput") {
    result.push("audio");
  } else if (device.kind === "videoinput") {
    result.push("video");
  }
});
callback(result);
```
Attachment #8638535 - Flags: review?(mdeboer) → review+
(Assignee)

Comment 4

3 years ago
For verification, this is all on the desktop side, link-clicker isn't affected.

Verify that for both conversations via links, and direct calls via contacts:

- No media devices (mic/video) gives appropriate message
- Just a microphone allows a call to work
- Video + Microphone works normally

This also fixes bug 1158138, which I'll mark as a duplicate of this in a moment.
Flags: qe-verify+
Flags: firefox-backlog+
Priority: -- → P2
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1158138
QA Contact: bogdan.maris
https://hg.mozilla.org/mozilla-central/rev/521c01986d01
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
(In reply to Mark Banner (:standard8) (away until 24 Aug) from comment #4)
> Verify that for both conversations via links, and direct calls via contacts:
> 
> - No media devices (mic/video) gives appropriate message
> - Just a microphone allows a call to work
> - Video + Microphone works normally
> 
> This also fixes bug 1158138, which I'll mark as a duplicate of this in a
> moment.

Verified that all this scenarios are working as they should on Windows 7 64-bit and Ubuntu 14.04 32-bit using latest Aurora 42.0a2.
Status: RESOLVED → VERIFIED
status-firefox42: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.