Closed Bug 1439001 Opened 2 years ago Closed 2 years ago

receiver.getSynchronizationSources()[0].audioLevel only present in two-way calls

Categories

(Core :: WebRTC, defect, P2, major)

59 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 + verified
firefox60 + verified

People

(Reporter: jib, Assigned: ng)

References

Details

Attachments

(1 file, 1 obsolete file)

It doesn't work at all in a one-way call (on the receiving end where you would expect it to work).

STRs:
 1. Open One-way call https://jsfiddle.net/jib1/w2p24L1b/ and Allow cam+mic

Expected result:

GetSSRC
{"audioLevel":0.0002818382931264455,"source":3557623822,"timestamp":1518817025102}

Actual result:

GetSSRC
{"source":3557623822,"timestamp":1518817025102}

Workaround: Two-way call https://jsfiddle.net/jib1/0yb7t7uh/
[Tracking Requested - why for this release]: Major bug in new 59 feature. May affect Google Meet.
Our mochitests [1] pass because they test this feature only for two-way calls. We should add a test for one-way as well.

[1] https://searchfox.org/mozilla-central/rev/cac28623a15ace458a8f4526e107a71db1519daf/dom/media/tests/mochitest/test_peerConnection_audioSynchronizationSources.html#38
Attachment #8951800 - Flags: review?(mfroman)
Attachment #8951800 - Flags: review?(mfroman) → review?(drno)
Comment on attachment 8951800 [details]
Bug 1439001 - AudioLevel RTP header ext. send/recv sense reversed in xceiver

https://reviewboard.mozilla.org/r/221076/#review226978

LGTM

Are you planing on adding a one-way mochitest in a new bug?
Attachment #8951800 - Flags: review?(drno) → review+
Attachment #8952520 - Flags: review?(mfroman)
Blocks: 1439736
Attachment #8952520 - Attachment is obsolete: true
Attachment #8952520 - Flags: review?(mfroman)
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/4bee139e29a1
AudioLevel RTP header ext. send/recv sense reversed in xceiver r=drno
https://hg.mozilla.org/mozilla-central/rev/4bee139e29a1
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
A test for this would indeed be nice :). That said, let's get QE to verify this for now and get it nominated for Beta uplift.
Flags: qe-verify+
Flags: needinfo?(na-g)
Flags: in-testsuite?
Ryan, a mochitest for this is landing with bug 1437936. It is in the autoland queue now. My plan was to request uplift after that lands on central. If it would be better to request uplift now, I will.

I am retaining the ni? as a reminder.
Flags: needinfo?(na-g) → needinfo?(ryanvm)
Flags: needinfo?(na-g)
Comment on attachment 8951800 [details]
Bug 1439001 - AudioLevel RTP header ext. send/recv sense reversed in xceiver

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1439001
[User impact if declined]: getSynchronizationSources would effectively only work in two way peer connections. This could impact webrtc services such as Meet.
[Is this code covered by automated tests?]: Yes, Bug 1439736 is a mochitest for this.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Patch is of trivial size; it should have very little interaction with other code; it does not exercise new code paths
[String changes made/needed]: none
Flags: needinfo?(na-g)
Attachment #8951800 - Flags: approval-mozilla-beta?
Comment on attachment 8951800 [details]
Bug 1439001 - AudioLevel RTP header ext. send/recv sense reversed in xceiver

WebRTC compatibility fix, taking for 59b13. Let's make sure that the new mochitest gets uplifted as well.
Flags: needinfo?(ryanvm)
Attachment #8951800 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed using Firefox 59.0 RC build and latest Nightly 60.0a1 2018-03-06 under Win 10 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.13.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.