Bring RTCRtpParameters/setParameters/getParameters up-to-spec
Categories
(Core :: WebRTC: Signaling, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox110 | --- | fixed |
People
(Reporter: bwc, Assigned: bwc)
References
(Depends on 1 open bug, Blocks 7 open bugs)
Details
(Keywords: dev-doc-complete, site-compat, Whiteboard: [jitsi-meet], [wptsync upstream])
Crash Data
Attachments
(18 files, 2 obsolete files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1401592: Test that sRD with a single rid results in [[SendEncodings]] containing that rid. r?jib
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
5.37 KB,
text/plain
|
chutten
:
data-review+
|
Details |
Updated•7 years ago
|
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
So, the spec says that in setParameters, we check whether parameters.encodings has been reordered. But what, exactly, does it mean to reorder a sequence of mutable dictionaries that don't have some unique id in them? Are we supposed to treat codecPayloadType as the unique id?
Comment 2•5 years ago
|
||
Resolved offline. This changed a couple of times in the spec:
- from re-ordering
encodings
to settingcodecPayloadType
in w3c/webrtc-pc#1592, - then from that to
setCodecPreferences
in w3c/webrtc-pc/pull#2037.
Updated•5 years ago
|
This here is required for Jitsi Meet to work properly.
As such, as you had some whiteboard for [jitsi-meet], please assign that to this.
More information by a Jitsi dev, here: https://github.com/jitsi/jitsi-meet/issues/4758#issuecomment-672148872
Updated•4 years ago
|
Will this feature be available in Firefox 80? Thank you
Comment 5•4 years ago
|
||
(In reply to Óvári from comment #4)
Will this feature be available in Firefox 80? Thank you
Definitely not in Firefox 80. I believe that Jitsi is looking for the RTCRtpParameters.active field specifically. Perhaps that could be spun out into a separate bug that could be fixed outside of the larger work to bring things up to spec here. I'd have to defer to Nils and Byron on how practical that would be.
Assignee | ||
Comment 7•2 years ago
|
||
Assignee | ||
Comment 8•2 years ago
|
||
Depends on D156828
Assignee | ||
Comment 9•2 years ago
|
||
The spec has settled on not allowing these constraints to be negotiated, so all
that is left is the negotiation of the set of rids.
Depends on D156829
Assignee | ||
Comment 10•2 years ago
|
||
JsepTrack used to be in charge of this, but because JSEP doesn't negotiate
encoding constraints, it did not make sense to delegate this to the JSEP code.
Depends on D156830
Assignee | ||
Comment 11•2 years ago
|
||
Includes a configured max rid length, since it is not very reasonable to assume
that rids of length 255 are supported by the RTP engine, regardless of what
the IETF specs say.
Depends on D156831
Assignee | ||
Comment 12•2 years ago
|
||
There is not really any reason to support disabling this feature.
Depends on D156832
Assignee | ||
Comment 13•2 years ago
|
||
Depends on D156833
Assignee | ||
Comment 14•2 years ago
|
||
Also adds some helper code based on the helper code in wpt.
Depends on D156834
Assignee | ||
Comment 15•2 years ago
|
||
This still needs spec work, but this is a proof of concept that works.
Here we adopt the strategy used for sRD(offer) racing against addTrack; when
the race is detected, the sRD is restarted.
Depends on D156835
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 16•2 years ago
|
||
Depends on D156836
Assignee | ||
Comment 17•2 years ago
|
||
Depends on D157650
Assignee | ||
Comment 18•2 years ago
|
||
Depends on D157651
Assignee | ||
Comment 19•2 years ago
|
||
Depends on D157653
Assignee | ||
Comment 20•2 years ago
|
||
Depends on D157654
Assignee | ||
Comment 21•2 years ago
|
||
Depends on D157655
Assignee | ||
Comment 22•2 years ago
|
||
Depends on D157656
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 25•2 years ago
|
||
Assignee | ||
Comment 26•2 years ago
|
||
Assignee | ||
Comment 27•2 years ago
|
||
Depends on D157657
Assignee | ||
Comment 28•2 years ago
|
||
Assignee | ||
Comment 29•2 years ago
|
||
Comment 30•2 years ago
|
||
Comment on attachment 9302711 [details]
data-review-1401592.md
PRELIMINARY NOTES:
Since this data collection is Glean in Firefox Desktop, you can use mach data-review
to generate a data review request template for you.
DATA COLLECTION REVIEW RESPONSE:
Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?
Yes.
Is there a control mechanism that allows the user to turn the data collection on and off?
Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.
If the request is for permanent data collection, is there someone who will monitor the data over time?
No. This collection will expire in Firefox 116.
Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?
Category 1, Technical.
Is the data collection request for default-on or default-off?
Default on for all channels.
Does the instrumentation include the addition of any new identifiers?
No.
Is the data collection covered by the existing Firefox privacy notice?
Yes.
Does the data collection use a third-party collection tool?
No.
Result: datareview+
Assignee | ||
Comment 31•2 years ago
|
||
Assignee | ||
Comment 32•2 years ago
|
||
Assignee | ||
Comment 33•2 years ago
|
||
Comment 34•2 years ago
|
||
Comment 36•2 years ago
|
||
Backed out for causing failures at test_peerConnection_scaleResolution_oldSetParameters.html.
Backout link: https://hg.mozilla.org/integration/autoland/rev/d62d1882c5affd1c699ae5147f97af3cc5d40e88
Failure log: https://treeherder.mozilla.org/logviewer?job_id=399526333&repo=autoland&lineNumber=5348
Assignee | ||
Comment 37•2 years ago
|
||
Is this only failing on Android 8.0 Pixel2 AArch64 WebRender debug?
Comment 38•2 years ago
|
||
Yes, its failing only on Android 8.0 Pixel2 AArch64 WebRender debug.
Assignee | ||
Comment 39•2 years ago
|
||
Ok, let me cancel the try push I just kicked off, and narrow it a bit.
Comment 40•2 years ago
|
||
Comment 41•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bdaba2c1cf13
https://hg.mozilla.org/mozilla-central/rev/ca3f32aeb043
https://hg.mozilla.org/mozilla-central/rev/157fce0b0691
https://hg.mozilla.org/mozilla-central/rev/e2341923b50f
https://hg.mozilla.org/mozilla-central/rev/6dab2f3adac1
https://hg.mozilla.org/mozilla-central/rev/4760dbd9857c
https://hg.mozilla.org/mozilla-central/rev/5c37fb3a7a77
https://hg.mozilla.org/mozilla-central/rev/a7f2ea3ca266
https://hg.mozilla.org/mozilla-central/rev/7ab95603e050
https://hg.mozilla.org/mozilla-central/rev/42543938d60d
https://hg.mozilla.org/mozilla-central/rev/f27124a55a11
https://hg.mozilla.org/mozilla-central/rev/15e2c79da249
https://hg.mozilla.org/mozilla-central/rev/a6e4123b8836
https://hg.mozilla.org/mozilla-central/rev/bd4500a78ca4
https://hg.mozilla.org/mozilla-central/rev/9c621a081558
https://hg.mozilla.org/mozilla-central/rev/a5ceeaf7e43b
https://hg.mozilla.org/mozilla-central/rev/92f418bbba4a
Comment 44•2 years ago
|
||
Copying crash signatures from duplicate bugs.
Comment 45•2 years ago
|
||
Tracking issue for MDN work https://github.com/mdn/content/issues/23677
Comment 47•2 years ago
•
|
||
I'm trying to update the docs and browser compatibility information for RTCRtpSender.getParameters()
, RTCRtpSender.setParameters()
in light of these changes.
The methods return an object with: codecs
, headerExtensions
, rtcp
, from RTCRtpParameters
, and encodings
, transactionId
, degradationPreference
from RTCRtpSendParameters
- The
RTCRtpParameters
properties seem to match the spec but be empty in FF110. Do they "work/get populated"? Or are they just objects to conform to the spec? There is a note in the change somewhere that seems to indicate the later. - In the current compatibility data it says for encodings "Firefox uses
RTCRtpParameters.encodings
instead. From an end user point of view does it matter? - Note that compatibility data will just record an object being returned - the dictionaries are no longer quoted if at all possible. - The compatibility data is supposed to show compatibility breaks compared to the spec. For
RTCRtpSender.getParameters()
I can see that now we return an object that matches the spec. In FF109 and earlier the object that returned was not particularly amenable to inspection - do we know what was "wrong" with the getParameters() and the returned object w.r.t. the spec for a developer? For example, it sounds likecodecs
,headerExtensions
,rtcp
are present but not used by firefox, and that was true prior to FF110 - so has any thing changed for developers? - Similarly, for the MDN release note I can say "Brings RTCRtpParameters/setParameters/getParameters up-to-spec", but that begs the question "what were the main things that were wrong before". On scan some of the things that happened:
- RTCRtpEncodingParameters.active support implemented (in a different issue, but part of this). Support sendEncodings in addTransceiver
- RTCRtpEncodingParameters - added default values for
.active
and.priority
. Removed.degradationPreference
- There also seems to be some kind of warnings if you try and set parameters but haven't got them first.
- What would you consider important to let developers know?
Updated•2 years ago
|
Comment 48•2 years ago
•
|
||
EDITED: - confirmed that the there is only one peer associated with each sender. Would appreciate confirmation of how this works still. My guess:
- Encodings that are available are set when you call
RTCPeerConnection.addTransceiver()
- When you call
RTCRtpSender.getParameters()
you'll get this list of encodings. The one that is in use for sending data is the one with theactive
flag set - right? Given the default for the active flag is true does that mean if you have multiples you will have to explicitly set the ones you don't want to use as false or will this happen for you when you set an encoding as active? - It looks like you can get a list of codecs in the parameters too. However as these are set using
transceiver.setCodecPreferences
from a point of view of set/getParameters these would be read-only right? - In other words, if you're getting and setting parameters, the interesting one in this discussion is the encodings, since you can use those to change how the data is sent. The codecs are more "for information" at this point..
Is that ^^^ in any way correct?
Updated•2 years ago
|
Assignee | ||
Comment 49•1 years ago
|
||
(In reply to Hamish Willee from comment #47)
I'm trying to update the docs and browser compatibility information for
RTCRtpSender.getParameters()
,RTCRtpSender.setParameters()
in light of these changes.The methods return an object with:
codecs
,headerExtensions
,rtcp
, fromRTCRtpParameters
, andencodings
,transactionId
,degradationPreference
fromRTCRtpSendParameters
- The
RTCRtpParameters
properties seem to match the spec but be empty in FF110. Do they "work/get populated"? Or are they just objects to conform to the spec? There is a note in the change somewhere that seems to indicate the later.- In the current compatibility data it says for encodings "Firefox uses
RTCRtpParameters.encodings
instead. From an end user point of view does it matter? - Note that compatibility data will just record an object being returned - the dictionaries are no longer quoted if at all possible.- The compatibility data is supposed to show compatibility breaks compared to the spec. For
RTCRtpSender.getParameters()
I can see that now we return an object that matches the spec. In FF109 and earlier the object that returned was not particularly amenable to inspection - do we know what was "wrong" with the getParameters() and the returned object w.r.t. the spec for a developer? For example, it sounds likecodecs
,headerExtensions
,rtcp
are present but not used by firefox, and that was true prior to FF110 - so has any thing changed for developers?- Similarly, for the MDN release note I can say "Brings RTCRtpParameters/setParameters/getParameters up-to-spec", but that begs the question "what were the main things that were wrong before". On scan some of the things that happened:
- RTCRtpEncodingParameters.active support implemented (in a different issue, but part of this). Support sendEncodings in addTransceiver
- RTCRtpEncodingParameters - added default values for
.active
and.priority
. Removed.degradationPreference
- There also seems to be some kind of warnings if you try and set parameters but haven't got them first.
- What would you consider important to let developers know?
The main change is implementation of the transactionId mechanic. Previously, Firefox was expecting JS to build the RTCRtpSendParameters from scratch, and call setParameters with it. Now, the spec requires JS to obtain a pre-built RTCRtpSendParameters using getParameters(), make (allowed) alterations, and then call setParameters() with the altered version, without relinquishing the event loop. Making this usable required implementing addTransceiver({sendEncodings: [...]}, since that is now the only way for JS to specify how many encodings to use (since JS can no longer build the parameters object from scratch).
Regarding .active, the idea is that you'll typically be sending on more than one encoding at a time (at different quality levels, so a conferencing server can transmit low quality to some participants, and high quality to others). JS can use .active to pause transmission of a particular encoding (just as a contrived example, maybe you're in a long conference but have never spoken, so transmitting the high quality video is not necessary, but we want to be able to enable it if it becomes useful later).
The codecs stuff is read only, and we don't really support it right now.
Updated•8 months ago
|
Description
•