Closed Bug 1807193 Opened 1 year ago Closed 1 year ago

Firefox cannot handle simulcast m-sections being negotiated inactive

Categories

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

Product:
Core
Component:
WebRTC: Signaling
Platform:
Desktop
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
110 Branch
Tracking Flags:
Tracking Status
firefox-esr102 --- wontfix
firefox108 --- wontfix
firefox109 --- wontfix
firefox110 --- fixed

People

(Reporter: saschanaz, Assigned: bwc)

References

Details

Attachments

(3 files)

Bug 1807193: Test cases for bug. r?ng
48 bytes, text/x-phabricator-request
Details | Review
Bug 1807193: Remove direction validation for simulcast in sipcc code. r?ng
48 bytes, text/x-phabricator-request
Details | Review
Bug 1807193: Update our copy of webrtc-sdp to 0.3.10. r?ng
48 bytes, text/x-phabricator-request
Details | Review
  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.

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.

status-firefox108: --- → unaffected
status-firefox109: --- → unaffected
status-firefox110: --- → affected
status-firefox-esr102: --- → unaffected
Flags: needinfo?(docfaraday)

(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.

Flags: needinfo?(docfaraday)

I cannot reproduce. What platform are you on?

Flags: needinfo?(krosylight)

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.

Flags: needinfo?(krosylight) → needinfo?(docfaraday)
OS: Unspecified → Windows 11
Hardware: Unspecified → Desktop

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.
Flags: needinfo?(krosylight)

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.

Flags: needinfo?(docfaraday)

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.

Flags: needinfo?(na-g)
Assignee: nobody → docfaraday
Severity: -- → S2
Priority: -- → P2

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

status-firefox108: unaffected → affected
status-firefox109: unaffected → affected
status-firefox-esr102: unaffected → affected
Summary: meet.jit.si connection fails: "Unfortunately, something went wrong." → Firefox cannot handle simulcast m-sections being negotiated inactive
OS: Windows 11 → All
Keywords: regression
No longer regressed by: 1676855

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

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.

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

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.

Try looks fine.

Pushed by bcampen@mozilla.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37775 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot

(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.

status-firefox108: affected → wontfix
status-firefox109: affected → fix-optional
Flags: needinfo?(docfaraday)
Flags: in-testsuite+

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

Flags: needinfo?(docfaraday)
Flags: needinfo?(krosylight)
Duplicate of this bug: 1808024
Duplicate of this bug: 1747234
See Also: → 1834821

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

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

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

Flags: needinfo?(mfroman)

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

Flags: needinfo?(mfroman)

(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.

Flags: needinfo?(na-g)

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?

Flags: needinfo?(bugs)

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.

Flags: needinfo?(bugs)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: