Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Separate Send/ReceiveCodecs and NegotiatedCodecs #2972

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Separate Send/ReceiveCodecs and NegotiatedCodecs #2972

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
70aa381
Separate Send/ReceiveCodecs and NegotiatedCodecs
alvestrand May 8, 2024
50bc218
Fix markup.
alvestrand May 8, 2024
be077ac
Fix html bug
alvestrand May 13, 2024
2da8426
Improve error communicate in amemendment checker
dontcallmedom May 13, 2024
805ca44
Improve error communicate in amemendment checker
dontcallmedom May 13, 2024
8991050
Attempt at amendments.json
alvestrand May 21, 2024
10bc207
Fix syntax error
alvestrand May 22, 2024
62f77eb
Reference test
alvestrand Jun 20, 2024
8dacfdc
Fix JSON in amendments.json
dontcallmedom Jun 21, 2024
ea35721
Simplify data in amendments
dontcallmedom Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions amendments.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,21 @@
"type": "correction",
"status": "candidate",
"id": 26
},
{
"description": "Separate Send/ReceiveCodecs and NegotiatedCodecs.",
"pr": 2972,
"difftype": "modify",
alvestrand marked this conversation as resolved.
Show resolved Hide resolved
"tests": [
"webrtc/RTCRtpSender-getParameters.html",
"webrtc/RTCRtpReceiver-getParameters.html",
dontcallmedom marked this conversation as resolved.
Show resolved Hide resolved
],
"testUpdates": [
"web-platform-tests/wpt#46840"
],
"type": "correction",
"status": "candidate",
"id": 47
}
],
"create-answer-restrictions": [
Expand Down Expand Up @@ -841,6 +856,14 @@
"type": "addition",
"status": "candidate",
"id": 41
},
{
"description": "Separate Send/ReceiveCodecs and NegotiatedCodecs.",
"pr": 2972,
"difftype": "modify",
"type": "correction",
"status": "candidate",
"id": 47
}
],
"rtcrtpreceiver-laststablestatereceivecodecs": [
Expand Down
8 changes: 6 additions & 2 deletions tools/check-rec-amendment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,22 @@ module.exports = async ({github, context, core}) => {
}
const amendments = require(process.env.GITHUB_WORKSPACE + '/amendments.json');
const prAmendmentSection = Object.values(amendments).find(list => list.find(a => Array.isArray(a.pr) ? a.pr.includes(context.issue.number) : a.pr === context.issue.number ));
const prAmendment = prAmendmentSection.find(a => Array.isArray(a.pr) ? a.pr.includes(context.issue.number) : a.pr === context.issue.number );
if (!prAmendment) {
if (!prAmendmentSection) {
core.setFailed(`Pull request ${context.issue.number} not labeled as editorial and not referenced in amendments.json`);
process.exit(2);
}
const prAmendment = prAmendmentSection.find(a => Array.isArray(a.pr) ? a.pr.includes(context.issue.number) : a.pr === context.issue.number );
if (!prAmendment.testUpdates || !prAmendment.testUpdates.length === 0) {
core.setFailed(`Pull request ${context.issue.number} declares an amendment but does not document its test status in testUpdates`);
process.exit(2);
}
const validTestUpdates = ["already-tested", "not-testable"];
if (typeof prAmendment.testUpdates === "string" && !validTestUpdates.includes(prAmendment.testUpdates)) {
core.setFailed(`Pull request ${context.issue.number} declares an invalid test status in its amendment testUpdates field`);
process.exit(2);
}
if (Array.isArray(prAmendment.testUpdates) && !prAmendment.testUpdates.every(t => t.match(/^web-platform-tests\/wpt#[0-9]+$/))) {
core.setFailed(`Pull request ${context.issue.number} declares test updates but not using the expected format to point to web-platform-tests PRs: "web-platform-tests/wpt#NNN"`);
process.exit(2);
}
};
65 changes: 38 additions & 27 deletions webrtc.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1864,9 +1864,9 @@ <h4>
<li>
<p>
Set
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[LastStableStateReceiveCodecs]]}}
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[LastStableStateNegotiatedCodecs]]}}
to
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[ReceiveCodecs]]}}.
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[NegotiatedCodecs]]}}.
</p>
</li>
</ol>
Expand Down Expand Up @@ -2187,24 +2187,6 @@ <h4>
<code>false</code>.
</p>
</li>
<li>
<p>
For each of the codecs that <var>description</var> negotiates for receiving, execute the following steps:
<ol>
<li>Locate the matching codec description in <var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[ReceiveCodecs]]}}.</li>
<li>If the matching codec description is not found, abort these steps.</li>
<li>Set the "enabled" flag in the matching codec description to "true".</li>
</ol>

</p>
<p class='note'>
If the <var>direction</var> is
{{RTCRtpTransceiverDirection/"sendonly"}} or
{{RTCRtpTransceiverDirection/"inactive"}},
the receiver is not prepared to receive
anything, and the list will be empty.
</p>
</li>
<li>
<p>
If <var>description</var> is of type
Expand Down Expand Up @@ -2244,13 +2226,37 @@ <h4>
</li>
<li>
<p>
For each of the codecs that <var>description</var> negotiates for sending, execute the following steps:
Clear the <var>transceiver</var>.{{RTCRtpTransceiver/[[Sender]]}}.{{RTCRtpSender/[[NegotiatedCodecs]]}} slot.
<br>
Then, for each of the codecs that <var>description</var> negotiates for sending, execute the following steps:
<ol>
<li>Locate the matching codec description in <var>transceiver</var>.{{RTCRtpTransceiver/[[Sender]]}}.{{RTCRtpSender/[[SendCodecs]]}}.</li>
<li>If the matching codec description is not found, abort these steps.</li>
Comment on lines +2231 to 2234
Copy link
Member

@jan-ivar jan-ivar Sep 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which steps are aborted here? One of the nested for-loops or sLD/sRD without error? Either seems wrong.

Not failing assumes negotiated codecs is a subset of this client's [[SendCodecs]]. The 1.0 spec didn't have this. It said:

image image

...which, AFAIK meant sender.getParameters().codecs might include remote codecs not found locally, but found in the remote answer, which JSEP seems to allow: "Any currently available media formats that are not present in the current remote description MUST be added after all existing formats."

Why is this new limitation needed?

It's also unclear what constitutes a "matching codec description".

<li>Set the "enabled" flag in the matching codec description to "true".</li>
<li>Add the codec description to the sender's {{RTCRtpSender/[[NegotiatedCodecs]]}} internal slot.</li>
jan-ivar marked this conversation as resolved.
Show resolved Hide resolved
</ol>
</li>
<li>
<p>
Clear the <var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[NegotiatedCodecs]]}} slot.
<br>
Then, for each of the codecs that <var>description</var> negotiates for receiving, execute the following steps:
<ol>
<li>Locate the matching codec description in <var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[ReceiveCodecs]]}}.</li>
<li>If the matching codec description is not found, abort these steps.</li>
<li>Set the "enabled" flag in the matching codec description to "true".</li>
<li>Add the codec description to the receiver's {{RTCRtpReceiver/[[NegotiatedCodecs]]}} internal slot.</li>
</ol>

</p>
<p class='note'>
If the <var>direction</var> is
{{RTCRtpTransceiverDirection/"sendonly"}} or
{{RTCRtpTransceiverDirection/"inactive"}},
the receiver is not prepared to receive
anything, and the list will be empty.
</p>
</li>
<li>
Set <var>transceiver</var>.{{RTCRtpTransceiver/[[Sender]]}}.{{RTCRtpSender/[[LastReturnedParameters]]}}
to <code>null</code>.
Expand Down Expand Up @@ -2693,9 +2699,9 @@ <h4>
<li>
<p>
Set
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[ReceiveCodecs]]}}
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[NegotiatedCodecs]]}}
to
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[LastStableStateReceiveCodecs]]}}.
<var>transceiver</var>.{{RTCRtpTransceiver/[[Receiver]]}}.{{RTCRtpReceiver/[[LastStableStateNegotiatedCodecs]]}}.
</p>
</li>
<li>
Expand Down Expand Up @@ -8800,6 +8806,9 @@ <h3>
[=RTCRtpSender/list of implemented send codecs=], with the "enabled" flag
set in an implementation defined manner.
</p>
<p>
Let <var>sender</var> have a <dfn data-dfn-for="RTCRtpSender">[[\NegotiatedCodecs]]</dfn> internal slot, consisting of {{RTCRtpCodecParameters}}, and initialized to an empty list.
jan-ivar marked this conversation as resolved.
Show resolved Hide resolved
</p>
</li>
<li class="no-test-needed">
<p>
Expand Down Expand Up @@ -9188,8 +9197,7 @@ <h2>
<li data-tests=
"RTCRtpParameters-codecs.html,protocol/video-codecs.https.html">
{{RTCRtpParameters/codecs}} is set to the codecs from the
{{RTCRtpSender/[[SendCodecs]]}} internal slot where the
"enabled" flag is true.
{{RTCRtpSender/[[NegotiatedCodecs]]}} internal slot.
</li>
<li data-tests="RTCRtpParameters-encodings.html">
{{RTCRtpParameters/rtcp}}.{{RTCRtcpParameters/cname}} is
Expand Down Expand Up @@ -10225,11 +10233,14 @@ <h3>
<a>list of implemented receive codecs</a> for <var>kind</var>, and with the "enabled" flag
set in an implementation defined manner.
</p>
<p>
Let <var>receiver</var> have a <dfn data-dfn-for="RTCRtpReceiver">[[\NegotiatedCodecs]]</dfn> internal slot, consisting of {{RTCRtpCodecParameters}}, and initialized to an empty list.
</p>
</li>
<li id="rtcrtpreceiver-laststablestatereceivecodecs">
<p>
Let <var>receiver</var> have a
<dfn data-dfn-for="RTCRtpReceiver">[[\LastStableStateReceiveCodecs]]</dfn> internal slot and
<dfn data-dfn-for="RTCRtpReceiver">[[\LastStableStateNegotiatedCodecs]]</dfn> internal slot and
initialize it to an empty list.
</p>
</li>
Expand Down Expand Up @@ -10507,7 +10518,7 @@ <h2>
<p>
{{RTCRtpParameters/codecs}} is set to the value of the
"enabled" codecs from the
{{RTCRtpReceiver/[[ReceiveCodecs]]}} internal slot.
{{RTCRtpReceiver/[[NegotiatedCodecs]]}} internal slot.
</p>
<div class="note">
Both the local and remote description may affect this
Expand Down
Loading