Skip to content

Commit

Permalink
Re-enable and improve h264-unidirectional-codec.offer.https.html.
Browse files Browse the repository at this point in the history
Update WPTs to reflect WebRTC roll of [1] and add a new test.

The H265 tests (both offer to send and offer to receive) now pass
(virtual because H265 is behind flag).

The H264 offer to send test no longer throws in setCodecPreferences()
and it successfully negotiates H264, however the test still fails for
reasons unrelated to the WebRTC fix, which is:
- H264 always offers profile-level-id=64001f (3.1) regardless of which
  level we ask it to offer, in my case 640034 (5.2).
  This is https://crbug.com/391750825.

The new test being added tests that even if we setCodecPreferences()
with a recvonly H264 codec on a sendrecv transceiver, we should still
offer H264 (but with a different level-id) because of codec filtering
being based on ignoring levels.
- This reveals a real bug in webrtc::IsSameRtpCodecIgnoringLevel that
  we should fix in a later CL. The problem is that it only ignores
  level for H265. Other codec comparison algorithms in WebRTC exists
  that do ignore levels for all codecs.

[1] https://webrtc-review.googlesource.com/c/src/+/374520

Bug: chromium:381407888
Change-Id: I2586abc01a955168cb9da1a7f0834a3e0aee049c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6190353
Commit-Queue: Henrik Boström <[email protected]>
Auto-Submit: Henrik Boström <[email protected]>
Commit-Queue: Harald Alvestrand <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1410251}
  • Loading branch information
henbos authored and chromium-wpt-export-bot committed Jan 23, 2025
1 parent ef244b7 commit 78bb416
Showing 1 changed file with 78 additions and 44 deletions.
122 changes: 78 additions & 44 deletions webrtc/protocol/h264-unidirectional-codec-offer.https.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,36 +52,31 @@
return codecs;
}

// Returns an array of { payloadType, sdpFmtpLine } entries in the order that
// they appeared in the SDP.
// Returns an array of { mimeType, payloadType, sdpFmtpLine } entries in the
// order that they appeared in the SDP.
function parseCodecsFromSdp(sdp) {
const codecs = [];
// For each a=rtpmap:...
const kRtpMapLineStart = 'a=rtpmap:';
const kFmtpLineStart = 'a=fmtp:';
for (let rtpmapIndex = 0;; ) {
rtpmapIndex = sdp.indexOf(kRtpMapLineStart, rtpmapIndex);
if (rtpmapIndex == -1) {
break;
}
rtpmapIndex += kRtpMapLineStart.length;
const payloadType =
Number(sdp.slice(rtpmapIndex, sdp.indexOf(' ', rtpmapIndex)));
const fmtpLineWithPT = kFmtpLineStart + payloadType;
const kRtpMapLineRegex = /\r\na=rtpmap:/g;
for (const match of sdp.matchAll(kRtpMapLineRegex)) {
const rtpmapIndex = match.index + 11;
const rtpmapSpaceIndex = sdp.indexOf(' ', rtpmapIndex)
const payloadType = Number(sdp.slice(rtpmapIndex, rtpmapSpaceIndex));
const codecName = sdp.slice(rtpmapSpaceIndex + 1,
sdp.indexOf('/', rtpmapSpaceIndex));
let sdpFmtpLine = undefined; // Can be undefined e.g. VP8.
const fmtpLineWithPT = `\r\na=fmtp:${payloadType} `;
let fmtpIndex = sdp.indexOf(fmtpLineWithPT, rtpmapIndex);
if (fmtpIndex == -1) {
throw `Parse error: Missing expected ${fmtpLineWithPT} line`;
}
fmtpIndex += fmtpLineWithPT.length + 1; // +1 to skip the space separator.
const fmtpLineEnd = sdp.indexOf('\r\n', fmtpIndex);
if (fmtpLineEnd == -1) {
throw 'Parse error: Missing expected end of FMTP line';
if (fmtpIndex != -1) {
fmtpIndex += fmtpLineWithPT.length;
const fmtpLineEnd = sdp.indexOf('\r\n', fmtpIndex);
if (fmtpLineEnd == -1) {
throw 'Parse error: Missing expected end of FMTP line';
}
sdpFmtpLine = sdp.slice(fmtpIndex, fmtpLineEnd);
}
const sdpFmtpLine = sdp.slice(fmtpIndex, fmtpLineEnd);
const codec = { payloadType, sdpFmtpLine };
const codec = { mimeType: `video/${codecName}`, payloadType, sdpFmtpLine };
codecs.push(codec);
// Continue the loop after end of current fmtp line.
rtpmapIndex = fmtpLineEnd;
}
return codecs;
}
Expand Down Expand Up @@ -129,8 +124,8 @@
// makes modifying the SDP answer easier, but because we cannot send the
// recvonly codec we have to fake it with remote SDP munging.
const sendRecvCodecs = getCodecsWithDirection('video/H264', 'sendrecv');
// If the folling optional asserts fail the test ends with
// [PRECONDITION_FAILED] as opposed to test failures.
// If any of the following optional asserts fail the test ends with
// [PRECONDITION_FAILED] as opposed to [FAIL].
assert_implements_optional(
recvOnlyCodecs.length > 0,
`There are no recvonly H264 codecs available in getCapabilities.`);
Expand All @@ -147,10 +142,12 @@
await pc1.setLocalDescription();
const offeredCodecs = parseCodecsFromSdp(pc1.localDescription.sdp);
assert_equals(offeredCodecs.length, 2, 'Two codecs should be offered');
assert_equals(offeredCodecs[0].sdpFmtpLine, recvOnlyCodec.sdpFmtpLine,
'The first offered codec should be the recvonly codec.');
assert_equals(offeredCodecs[1].sdpFmtpLine, sendRecvCodec.sdpFmtpLine,
'The second offered codec should be the sendrecv codec.');
assert_equals(offeredCodecs[0].mimeType, 'video/H264');
assert_true(offeredCodecs[0].sdpFmtpLine == recvOnlyCodec.sdpFmtpLine,
`The first offered codec's sdpFmtpLine is the recvonly one.`);
assert_equals(offeredCodecs[1].mimeType, 'video/H264');
assert_true(offeredCodecs[1].sdpFmtpLine == sendRecvCodec.sdpFmtpLine,
`The second offered codec's sdpFmtpLine is the sendrecv one.`);
await pc2.setRemoteDescription(pc1.localDescription);

// Answer to send.
Expand All @@ -161,15 +158,49 @@
// been removed from the SDP answer.
const answeredCodecs = parseCodecsFromSdp(pc2.localDescription.sdp);
assert_equals(answeredCodecs.length, 1, 'One codec should be answered');
assert_equals(answeredCodecs[0].sdpFmtpLine, sendRecvCodec.sdpFmtpLine,
'The first answered codec should be the sendrecv codec.');
assert_equals(answeredCodecs[0].mimeType, 'video/H264');
assert_true(answeredCodecs[0].sdpFmtpLine == sendRecvCodec.sdpFmtpLine,
`The answered codec's sdpFmtpLine is the sendrecv one.`);
// Trick `pc1` into thinking `pc2` can send the codec by modifying the SDP.
// Receiving media is not testable but this ensures that the SDP is accepted.
const modifiedSdp = replaceCodecInSdp(
pc2.localDescription.sdp, answeredCodecs[0], offeredCodecs[0]);
await pc1.setRemoteDescription({type: 'answer', sdp: modifiedSdp});
}, `Offer to receive a recvonly H264 codec`);
}, `Offer to receive a recvonly H264 codec on a recvonly transceiver`);

promise_test(async t => {
const pc = new RTCPeerConnection();
t.add_cleanup(() => pc.close());

const h264RecvOnlyCodecs = getCodecsWithDirection('video/H264', 'recvonly');
const vp8SendRecvCodecs = getCodecsWithDirection('video/VP8', 'sendrecv');
// If any of the following optional asserts fail the test ends with
// [PRECONDITION_FAILED] as opposed to [FAIL].
assert_implements_optional(
h264RecvOnlyCodecs.length > 0,
`There are no recvonly H264 codecs available in getCapabilities.`);
assert_implements_optional(
vp8SendRecvCodecs.length > 0,
`There are no sendrecv VP8 codecs available in getCapabilities.`);
const h264RecvOnlyCodec = h264RecvOnlyCodecs[0];
const vp8SendRecvCodec = vp8SendRecvCodecs[0];

const transceiver = pc.addTransceiver('video', {direction: 'sendrecv'});
transceiver.setCodecPreferences([h264RecvOnlyCodec, vp8SendRecvCodec]);

await pc.setLocalDescription();
const offeredCodecs = parseCodecsFromSdp(pc.localDescription.sdp);
// Even though this H264 codec with its level ID is recvonly, we should still
// offer to sendrecv H264 but with a different profile-level-id for sendrecv.
assert_equals(offeredCodecs.length, 2, 'Two codecs are offered (H264, VP8).');
assert_equals(offeredCodecs[0].mimeType, 'video/H264',
'The first offered codec is H264.');
assert_true(offeredCodecs[0].sdpFmtpLine != h264RecvOnlyCodec.sdpFmtpLine,
'The offered H264 profile-level-id should be different from ' +
'the recvonly one.');
assert_equals(offeredCodecs[1].mimeType, 'video/VP8',
'The second offered codec is VP8.');
}, `Offering a recvonly codec on a sendrecv transceiver`);

promise_test(async t => {
const pc1 = new RTCPeerConnection();
Expand All @@ -183,8 +214,8 @@
// makes modifying the SDP answer easier, but because we cannot receive the
// sendonly codec we have to fake it with remote SDP munging.
const sendRecvCodecs = getCodecsWithDirection('video/H264', 'sendrecv');
// If the folling optional asserts fail the test ends with
// [PRECONDITION_FAILED] as opposed to test failures.
// If any of the following optional asserts fail the test ends with
// [PRECONDITION_FAILED] as opposed to [FAIL].
assert_implements_optional(
sendOnlyCodecs.length > 0,
`There are no sendonly H264 codecs available in getCapabilities.`);
Expand All @@ -201,10 +232,12 @@
await pc1.setLocalDescription();
const offeredCodecs = parseCodecsFromSdp(pc1.localDescription.sdp);
assert_equals(offeredCodecs.length, 2, 'Two codecs should be offered');
assert_equals(offeredCodecs[0].sdpFmtpLine, sendOnlyCodec.sdpFmtpLine,
'The first offered codec should be the sendonly codec.');
assert_equals(offeredCodecs[1].sdpFmtpLine, sendRecvCodec.sdpFmtpLine,
'The second offered codec should be the sendrecv codec.');
assert_equals(offeredCodecs[0].mimeType, 'video/H264');
assert_true(offeredCodecs[0].sdpFmtpLine == sendOnlyCodec.sdpFmtpLine,
`The first offered codec's sdpFmtpLine is the sendonly one.`);
assert_equals(offeredCodecs[1].mimeType, 'video/H264');
assert_true(offeredCodecs[1].sdpFmtpLine == sendRecvCodec.sdpFmtpLine,
`The second offered codec's sdpFmtpLine is the sendrecv one.`);
await pc2.setRemoteDescription(pc1.localDescription);

// Answer to receive.
Expand All @@ -213,8 +246,9 @@
// been removed from the SDP answer.
const answeredCodecs = parseCodecsFromSdp(pc2.localDescription.sdp);
assert_equals(answeredCodecs.length, 1, 'One codec should be answered');
assert_equals(answeredCodecs[0].sdpFmtpLine, sendRecvCodec.sdpFmtpLine,
'The first answered codec should be the sendrecv codec.');
assert_equals(answeredCodecs[0].mimeType, 'video/H264');
assert_true(answeredCodecs[0].sdpFmtpLine == sendRecvCodec.sdpFmtpLine,
`The answered codec's sdpFmtpLine is the sendrecv one.`);
// Trick `pc1` into thinking `pc2` can receive the codec by modifying the SDP.
const modifiedSdp = replaceCodecInSdp(
pc2.localDescription.sdp, answeredCodecs[0], offeredCodecs[0]);
Expand All @@ -226,7 +260,7 @@
`Only one codec should have been negotiated`);
assert_equals(params.codecs[0].payloadType, offeredCodecs[0].payloadType,
`The sendonly codec's payloadType shows up in getParameters()`);
assert_equals(params.codecs[0].sdpFmtpLine, offeredCodecs[0].sdpFmtpLine,
`The sendonly codec's sdpFmtpLine shows up in getParameters()`);
}, `Offer to send a sendonly H264 codec`);
assert_true(params.codecs[0].sdpFmtpLine == offeredCodecs[0].sdpFmtpLine,
`The sendonly codec's sdpFmtpLine shows up in getParameters()`);
}, `Offer to send a sendonly H264 codec on a sendonly transceiver`);
</script>

0 comments on commit 78bb416

Please sign in to comment.