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

Allow level-id to be different in codec match #3023

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

Allow level-id to be different in codec match #3023

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c757b5f
Allow level-id to be different in codec match
henbos Nov 21, 2024
2473be4
grammar
henbos Nov 21, 2024
68f15e1
Update amendments
henbos Nov 21, 2024
d137450
Not testable?
henbos Nov 21, 2024
0aea1b3
Trial and error
henbos Nov 21, 2024
7197ba0
..
henbos Nov 21, 2024
5fb5525
Order and default
henbos Nov 21, 2024
a81bd7e
Rewording and grammar fix
henbos Nov 21, 2024
46142bd
nit
henbos Nov 21, 2024
3636bc6
Significant rework of PR
henbos Nov 22, 2024
adc8a46
...
henbos Nov 22, 2024
38b106c
Merge branch 'main' into 241121_LevelId
henbos Nov 28, 2024
0cc5068
Update wording
henbos Nov 28, 2024
d0e97ad
Update webrtc.html
henbos Nov 28, 2024
6e82ed5
Merge branch 'main' into 241121_LevelId
henbos Dec 2, 2024
dc885bc
Update filter steps
henbos Dec 2, 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
11 changes: 11 additions & 0 deletions amendments.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,5 +1242,16 @@
"web-platform-tests/wpt#48361"
]
}
],
"codec-match-without-level-id": [
{
"description": "Two codecs are considered the same even if level-id is not",
"type": "correction",
"status": "candidate",
"difftype": "modify",
"id": 52,
"pr": 3023,
"testUpdates": "not-testable"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remote SDP is testable.

}
]
}
1 change: 1 addition & 0 deletions base-rec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11174,6 +11174,7 @@ <h4 id="methods-8">
</ol>
</div>
<div id="codecmatch-algorithm"><!-- added as anchor to new algo in revised spec --></div>
<div id="codec-match-without-level-id"><!-- amendment to the codecmatch-algorithm --></div>
<div class="note" role="note" id="issue-container-generatedID-47"><div role="heading" class="note-title marker" id="h-note-47" aria-level="5"><span>Note</span></div><p class="">
If set, the offerer's codec preferences will decide the order
of the codecs in the offer. If the answerer does not have any
Expand Down
76 changes: 63 additions & 13 deletions webrtc.html
View file
Open in desktop
henbos marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2818,9 +2818,9 @@ <h4>
<li>
<p>For each <var>encoding</var> in
<var>transceiver</var>.{{RTCRtpTransceiver/[[Sender]]}}.{{RTCRtpSender/[[SendEncodings]]}},
if <var>encoding</var>.{{RTCRtpEncodingParameters/codec}} does not
[= codec dictionary match | match =] any entry in <var>codecs</var>,
[=map/remove=] <var>encoding</var>[{{RTCRtpEncodingParameters/codec}}].</p>
if <var>encoding</var>.{{RTCRtpEncodingParameters/codec}} does not match any entry in
<var>codecs</var>, using the [= codec dictionary match =] algorithm with <var>ignoreLevels</var>
set to <code>true</code>, [=map/remove=] <var>encoding</var>.{{RTCRtpEncodingParameters/codec}}.</p>
</li>
</ol>
</li>
Expand Down Expand Up @@ -9086,7 +9086,8 @@ <h2>
</li>
<li id="setparameters-codec-validation-4">
Any <var>encoding</var> in encodings [=map/exists|contains=] a codec
[= codec dictionary match | not found =] in <var>choosableCodecs</var>.
not found in <var>choosableCodecs</var>, using the [= codec dictionary match =]
algorithm with <var>ignoreLevels</var> set to <code>true</code>.
</li>
</ul>
</li>
Expand Down Expand Up @@ -9791,8 +9792,16 @@ <h2>
<dfn data-idl>codec</dfn> of type <span class="idlMemberType">RTCRtpCodec</span>
</dt>
<dd>
<p>Optional value selecting which codec is used for this encoding's
RTP stream. If absent, the user agent can chose to use any negotiated codec.</p>
<p>
Optional value selecting which codec is used for this encoding's
RTP stream. If absent, the user agent can chose to use any negotiated codec.
</p>
<p>
When {{codec}} is set and {{RTCRtpSender/[[SendCodecs]]}} have been negotiated,
the user agent SHOULD use the first {{RTCRtpSender/[[SendCodecs]]}} matching
{{codec}} for sending, according to the [= codec dictionary match =] algorithm
with <var>ignoreLevels</var> set to <code>true</code>.
</p>
</dd>
</div>
<div>
Expand Down Expand Up @@ -11466,10 +11475,13 @@ <h2>
</ol>
</div>
<div id="codecmatch-algorithm">
<p> The <dfn class="export">codec dictionary match</dfn> algorithm
given two {{RTCRtpCodec}} dictionaries
<var>first</var> and <var>second</var> is as follows:
<p>
The <dfn class="export">codec dictionary match</dfn> algorithm given two
{{RTCRtpCodec}} dictionaries <var>first</var> and <var>second</var>, and
an <var>ignoreLevels</var> boolean defaulting to <code>false</code> if not
henbos marked this conversation as resolved.
Show resolved Hide resolved
specified, is as follows:
</p>
</div>
<ol class=algorithm>
<li>
<p>
Expand Down Expand Up @@ -11497,18 +11509,56 @@ <h2>
<p>
If either (but not both) of <var>first</var>.{{RTCRtpCodec/sdpFmtpLine}}
and <var>second</var>.{{RTCRtpCodec/sdpFmtpLine}} are [= map/exist | missing =],
or if they both [=map/exist=] and <var>first</var>.{{RTCRtpCodec/sdpFmtpLine}}
is different from <var>second</var>.{{RTCRtpCodec/sdpFmtpLine}}, return
<code>false</code>.
return <code>false</code>.
aboba marked this conversation as resolved.
Show resolved Hide resolved
</p>
</li>
<li>
<p>
If both <var>first</var>.{{RTCRtpCodec/sdpFmtpLine}} and
<var>second</var>.{{RTCRtpCodec/sdpFmtpLine}} [=map/exist=], run the following
henbos marked this conversation as resolved.
Show resolved Hide resolved
steps:
</p>
<ol>
<li>
<p>
Let <var>firstMediaFormat</var> be a key-value map of the media formats constructed
from <var>first</var>.{{RTCRtpCodec/sdpFmtpLine}} and <var>secondMediaFormat</var>
be a key-value map of the media formats constructed from
<var>second</var>.{{RTCRtpCodec/sdpFmtpLine}}.
</p>
<p class="note">
Which FMTP parameters make up the media format is codec specific. In some cases
a parameter can be omitted and still be inferred, in which case it is also a part
of the media format of that codec.
</p>
</li>
<li>
<p>
If <var>firstMediaFormat</var> is not equal to <var>secondMediaFormat</var>, return
<code>false</code>.
</p>
</li>
<li>
<div id="codec-match-without-level-id">
<p>
If <var>ignoreLevels</var> is <code>false</code> and the highest complying bitstream
levels inferred from <var>first</var>.{{RTCRtpCodec/sdpFmtpLine}} and
<var>second</var>.{{RTCRtpCodec/sdpFmtpLine}} are different, return <code>false</code>.
</p>
<p class="note">
Even if <var>ignoreLevels</var> is <code>true</code>, some codecs (such as H.264) include
levels in the media format, so that the level cannot be ignored in this algorithm.
Copy link
Contributor Author

@henbos henbos Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

More accurately "..., so that ignoring the level requires codec-specific parsing" to reflect level ID in H.264 being something that can be parsed e.g. last byte

</p>
</div>
</li>
</ol>
</li>
<li>
<p>
Return <code>true</code>.
Copy link
Member

Choose a reason for hiding this comment

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

If one of the inputs is garbage the old algorithm returned false. Doesn't this risk returning true if the line is not parseable?

</p>
</li>
</ol>
</div>
<p class="note">
If set, the offerer's receive codec preferences will decide the order
of the codecs in the offer. If the answerer does not have any
Expand Down