web-compat problems with setParameters({encodings: []})
Categories
(Core :: WebRTC: Signaling, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr102 | --- | unaffected |
firefox110 | --- | fixed |
firefox111 | --- | fixed |
People
(Reporter: bwc, Assigned: bwc)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-release+
|
Details | Review |
Some sites are calling setParameters with an empty encodings array (for audio, at least), and other browsers are (incorrectly) allowing it. We should let this slide in setParameters compat mode. We'll also want to add some telemetry for this in a follow-up.
Assignee | ||
Comment 1•2 years ago
|
||
FWIW, we have telemetry for this failure that has not pinged once, so maybe not a big problem? It is weird that we did not get a ping from this:
https://mediasoup.discourse.group/t/no-mic-on-mediasoup-2-and-latest-firefox/4933
Updated•2 years ago
|
Updated•2 years ago
|
Comment 2•2 years ago
|
||
other browsers are (incorrectly) allowing it
Other browsers and Firefox release, so marking this a regression.
Comment 3•2 years ago
•
|
||
For other vendors pointed here:
Even audio transceivers have a single encoding to allow for modifications like params.encodings[0].maxBitrate = bps
, so changing the length from 1 to 0 is InvalidModificationError
according to spec:
- "Let N be the number of RTCRtpEncodingParameters stored in sender.[[SendEncodings]]. ... If any of the following conditions are met, return a promise rejected with a newly created InvalidModificationError: 1. encodings.length is different from N. "
Assignee | ||
Comment 4•2 years ago
|
||
Also add some wpt for this (invalid) case.
Assignee | ||
Comment 5•2 years ago
|
||
Comment 6•2 years ago
|
||
So this not throwing in Chrome is an artifact of https://crbug.com/1372603
But the real issue is we broke web compat with Firefox release
In Firefox release 109 https://jsfiddle.net/jib1/1mspnv42/9/ emits (before and after the affected setParameters call):
sender.getParameters() = {}
sender.getParameters() = {
"encodings": []
}
...whereas in 110 it's InvalidModificationError: Cannot set an empty encodings array
My guess would be this isn't affecting a lot of services.
Assignee | ||
Comment 7•2 years ago
|
||
Assignee | ||
Comment 8•2 years ago
|
||
Try looks fine.
Assignee | ||
Comment 11•2 years ago
|
||
Comment on attachment 9317123 [details]
Bug 1816160: Warn (and no-op) on setParameters with empty encodings if the compat mode is enabled. r?jib
Beta/Release Uplift Approval Request
- User impact if declined: Potential breakage in web teleconferencing.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This is a very small change that lets an uncommon error slide.
- String changes made/needed: None.
- Is Android affected?: Yes
Comment 12•2 years ago
|
||
bugherder |
Comment 14•2 years ago
|
||
Comment on attachment 9317123 [details]
Bug 1816160: Warn (and no-op) on setParameters with empty encodings if the compat mode is enabled. r?jib
Fx110 is already in RC, switching to release uplift request
Comment 15•2 years ago
|
||
This bug has the keyword regression
, so its type should be defect.
Updated•2 years ago
|
Comment 16•2 years ago
|
||
Comment on attachment 9317123 [details]
Bug 1816160: Warn (and no-op) on setParameters with empty encodings if the compat mode is enabled. r?jib
Low risk, baked on beta with no regression reported, improves webcompat, uplift approved for 110.0.1, thanks.
Comment 17•2 years ago
|
||
bugherder uplift |
Description
•