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

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P2)

Description

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

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/

Comment 1

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

[Tracking Requested - why for this release]: Major bug in new 59 feature. May affect Google Meet.

Comment 2

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

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

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1439001 - AudioLevel RTP header ext. send/recv sense reversed in xceiver

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

Comment 4

[Nils Ohlmeier [: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?

Comment 5

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1439001 - P2 - AudioLevel RTP header ext. send/recv mochitest

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

Comment 6

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8951800 [details]
Bug 1439001 - AudioLevel RTP header ext. send/recv sense reversed in xceiver

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

Comment 7

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8951800 [details]
Bug 1439001 - AudioLevel RTP header ext. send/recv sense reversed in xceiver

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4bee139e29a1
AudioLevel RTP header ext. send/recv sense reversed in xceiver r=drno

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/4bee139e29a1

Comment 10

[(PTO June 19-July 7) Ryan VanderMeulen]

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.

Comment 11

[Nico Grunbaum [:ng, @chew:mozilla.org]]

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.

Comment 12

[Nico Grunbaum [:ng, @chew:mozilla.org]]

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

Comment 13

[(PTO June 19-July 7) Ryan VanderMeulen]

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.

Comment 14

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/7e8036db9669

Comment 15

[Petruta Horea [:phorea], Desktop Test Engineering]

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.