1807193 - Firefox cannot handle simulcast m-sections being negotiated inactive

Status: RESOLVED FIXED

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

Description

[Kagami Rosylight [:saschanaz] (they/them)]

1. Open https://meet.jit.si/AsleepEndorsementsRecordEver on a stable channel Firefox 108.
2. Either allow or block the mic access, and tap "Join meeting" button.
3. Open the same page on nightly and do the same thing

Expected: Both should work same
Actual: Nightly fails and reloads with the message "Unfortunately, something went wrong.
We're trying to fix this. Reconnecting in 3 sec..."

Mozregression says it's bug 1676855.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1676855

:bwc, since you are the author of the regressor, bug 1676855, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 2

[Byron Campen [:bwc]]

(In reply to Kagami :saschanaz [ooo until 2022-01-06] from comment #0)

1. Open https://meet.jit.si/AsleepEndorsementsRecordEver on a stable channel Firefox 108.
2. Either allow or block the mic access, and tap "Join meeting" button.
3. Open the same page on nightly and do the same thing

Expected: Both should work same
Actual: Nightly fails and reloads with the message "Unfortunately, something went wrong.
We're trying to fix this. Reconnecting in 3 sec..."

Mozregression says it's bug 1676855.

I'm confused; you report that this happens on stable 108, but bug 1676855 is only present on 110 right now.

Edit: I see, you're just saying this happens when 108 is on the other side of the call. Trying to reproduce.

Comment 3

[Byron Campen [:bwc]]

I cannot reproduce. What platform are you on?

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Comment #0 is about Nightly failure. Stable Firefox is used only to have another client for Nightly to connect. In other words, step 3 is the core part of this.

And I'm on Windows 11.

Comment 5

[Dan Baker[:dbaker]]

We seem to be able to reproduce this error or at least something similar but in order to do this we need to have video muted on the Nightly build.
Are you testing this using the same device where the camera is not available to the Nightly build?

• If yes does it work if you use two devices?

Do you see this issue if you do the below:

• Only have Nightly browser open (close 108 if open).
• Join the meeting from Nightly allowing the camera.
• Launch Firefox (108) and join the meeting.

Comment 6

[Byron Campen [:bwc]]

In reproducing this, we ended up hitting this error:

https://searchfox.org/mozilla-central/rev/abf6758ed833c203f84703aa2e3e3d317571b1e9/third_party/rust/webrtc-sdp/src/lib.rs#717

This may have been invalid in earlier draft versions, but this is valid according to current spec. Right now, it seems that we cannot handle a remote simulcast m-section being marked sendonly/inactive (or a local simulcast m-section being marked recvonly/inactive). At a minimum, we'll need to:

Remove this code in webrtc-sdp
Remove this code in m-c
Remove these tests, or perhaps change them to expect no failure
Probably write a wpt that tries to renegotiate simulcast with direction attributes other than sendrecv.

Comment 7

[Byron Campen [:bwc]]

Nico, have we decided to stop making modifications in-tree to webrtc-sdp? I can file a bug on webrtc-sdp if that is the case.

Comment 8

[Byron Campen [:bwc]]

This is probably the kind of fix we'd like to uplift, if possible.

Comment 9

[Byron Campen [:bwc]]

Issue filed and pull request submitted for webrtc-sdp:

https://github.com/mozilla/webrtc-sdp/issues/267
https://github.com/mozilla/webrtc-sdp/pull/268

Comment 10

[Byron Campen [:bwc]]

I've manually verified that the changes described in comment 6 result in jitsi functioning as expected. I'll get some patches up once I write a wpt.

Comment 11

[Byron Campen [:bwc]]

Attachment: Bug 1807193: Test cases for bug. r?ng

Comment 12

[Byron Campen [:bwc]]

Attachment: Bug 1807193: Remove direction validation for simulcast in sipcc code. r?ng

Depends on D165990

Comment 13

[Byron Campen [:bwc]]

Ok, patches are up for tests and the sipcc modification. We still need to fix webrtc-sdp and update our copy.

Comment 14

[Byron Campen [:bwc]]

I should note that the wpt show that the only place we run into trouble is setting a local simulcast transceiver to recvonly; setting it to inactive does not seem to cause a problem, which is a bit strange, and we also do not seem to have difficulty with the simulcast receiver (ie; the other end) using sendonly or inactive in an answer. So, maybe this bug is not quite as easy to trigger as I thought.

Comment 15

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/jobs?repo=try&revision=7824ba94b72140abadfd01d28b50688f7a03fc8d
https://treeherder.mozilla.org/jobs?repo=try&revision=967a14c3032cac85e35b14f318e52df03c8a68cf
https://treeherder.mozilla.org/jobs?repo=try&revision=ea366c7e6d887d2128d12a32de96bf3337b79eb5

Comment 16

[Byron Campen [:bwc]]

Attachment: Bug 1807193: Update our copy of webrtc-sdp to 0.3.10. r?ng

This is primarily to pick up https://github.com/mozilla/webrtc-sdp/pull/268

Depends on D165991

Comment 17

[Byron Campen [:bwc]]

Try looks fine.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a583cdaaf33
Test cases for bug. r=ng
https://hg.mozilla.org/integration/autoland/rev/d8c897d21cae
Remove direction validation for simulcast in sipcc code. r=ng
https://hg.mozilla.org/integration/autoland/rev/911b6bb03056
Update our copy of webrtc-sdp to 0.3.10. r=ng,supply-chain-reviewers

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/3a583cdaaf33
https://hg.mozilla.org/mozilla-central/rev/d8c897d21cae
https://hg.mozilla.org/mozilla-central/rev/911b6bb03056

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37775 for changes under testing/web-platform/tests

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 22

[Ryan VanderMeulen [:RyanVM]]

(In reply to Byron Campen [:bwc] from comment #8)

This is probably the kind of fix we'd like to uplift, if possible.

It's pretty late in the cycle for uplifts as we're building the RC for Fx109 on Monday. If you feel strongly that we should ship this fix ASAP, please nominate for uplift when you get a chance. If nothing else, I'm leaving ESR set to affected for possible consideration down the road.

Comment 23

[Byron Campen [:bwc]]

Since this is harder to trigger than I thought it was (see comment 14), I think this can ride the trains.

Comment 26

[nemo]

FYI. This bug just happened to me, again, in nightly firefox today in a jitsi. It was a bit of a shock.
Has there been a regression?

That is, the bug I reported in https://bugzilla.mozilla.org/show_bug.cgi?id=1808024 happened where as soon as a 3rd person joined I lost audio and I saw the following on stderr.

[ERROR rsdparsa_capi] Error parsing SDP in rust: Line error: Parsing error: format number in media line is out of range in line(10): m=audio 3480 UDP/TLS/RTP/SAVPF 113 109 104 102 9 103 111 18 0 8 97 101 13 120

Comment 27

[Byron Campen [:bwc]]

Looks like a bug in webrtc-sdp. There seems to be a pull request that would fix this here:

https://github.com/mozilla/webrtc-sdp/pull/266/commits/af5e3fee9ed28edf04d3d104f8cf93b9548e6551

However, this code needs more fixup than that:

https://searchfox.org/mozilla-central/rev/961a9e56a0b5fa96ceef22c61c5e75fb6ba53395/third_party/rust/webrtc-sdp/src/media_type.rs#384-388

That code is way too strict. It should be more in line with this code, if it does any validation at all:

https://searchfox.org/mozilla-central/rev/961a9e56a0b5fa96ceef22c61c5e75fb6ba53395/dom/media/webrtc/jsep/JsepSessionImpl.cpp#46-51

Comment 28

[Byron Campen [:bwc]]

Looks like there's a more thorough PR here, although I'd personally just enumerate the forbidden types, and not bother enumerating the allowed ones:

https://github.com/mozilla/webrtc-sdp/pull/249/commits/1598a8d1a33cd270ae1724733c988138ede17df7

Comment 29

[Michael Froman [:mjf]]

Nico has been working with webrtc-sdp lately, I'll let him comment.

Comment 30

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

(In reply to Michael Froman [:mjf] from comment #29)

Nico has been working with webrtc-sdp lately, I'll let him comment.

I have a fix which matches the behavior in JsepSessionImpl.cpp. It will land in Bug 1875184.

Comment 31

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

Bug 1875184 has landed.

Nemo, if you have a moment, could you use the latest Nightly and check to see if the issue still reproduces?

Comment 32

[nemo]

It took a while to check this one since the computer I was dogfooding nightly on in work jitsi meetings was not one I usually used.
But, I had no issues with nightly in Jitsi all last Friday, and I'm pretty sure that included a couple of meetings with at least 3 participants.
So going to say this has probably been resolved.