Skip to content

Commit

Permalink
Reland "Allow createEncodedStreams on PCs without encodedInsertableSt…
Browse files Browse the repository at this point in the history
…reams param"

Change since previous: Reduce number of frames/packets waited for in
insertable-streams web test and add it to SlowTests. See Patchset
1 -> Patchset 5 diff.

This is a reland of commit e5518aa87b722a83596d1fb2a7506d0ab76f1207

Original change's description:
> Allow createEncodedStreams on PCs without encodedInsertableStreams param
>
> Allow creating Encoded Transforms for any Receivers and Senders, so
> long as createEncodedStreams() is called by JS synchronously after
> sender/receiver creation. This is achieved by setting up a WebRTC
> transform on all transceivers, but "short circuiting" it if JS hasn't
> set up its own transform within an event loop spin of creation. This
> will make the transform just pass frames to be transformed directly back
> without invoking the cost of a thread hop or any JS work.
>
> The existing behaviour (dropping all frames until a JS transform is
> installed) is preserved for PCs created with {encodedInsertableStreams:
> true}.
>
> This implements the algorithm defined in https://www.w3.org/TR/2023/WD-webrtc-encoded-transform-20231012/#stream-creation,
> but for the Chromium createEncodedStreams() method, improving
> conformance incrementally.
>
>
> Bug: 1502781
> Change-Id: Ie36d8ed8f431afa97307030646d3b207bf14cf7a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5040731
> Commit-Queue: Tony Herre <[email protected]>
> Reviewed-by: Guido Urdaneta <[email protected]>
> Reviewed-by: Harald Alvestrand <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1230611}

Bug: 1502781
Change-Id: I70b366f375e1b6c8e90567f5c994423fd738c8fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5077034
Reviewed-by: Guido Urdaneta <[email protected]>
Commit-Queue: Tony Herre <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1231330}
  • Loading branch information
Tony Herre authored and chromium-wpt-export-bot committed Nov 30, 2023
1 parent 0b3dc93 commit 37fc071
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 113 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
</head>
<body>
<script>
async function testAudioFlow(t, negotiationFunction, perFrameCallback = () => {}) {
const caller = new RTCPeerConnection({encodedInsertableStreams:true});
async function testAudioFlow(t, negotiationFunction, setConstructorParam, perFrameCallback = () => {}) {
const caller = new RTCPeerConnection(setConstructorParam ? {encodedInsertableStreams:true} : {});
t.add_cleanup(() => caller.close());
const callee = new RTCPeerConnection({encodedInsertableStreams:true});
const callee = new RTCPeerConnection(setConstructorParam ? {encodedInsertableStreams:true} : {});
t.add_cleanup(() => callee.close());

await setMediaPermission("granted", ["microphone"]);
Expand All @@ -34,15 +34,26 @@
const numFramesModifyData = 5;
const numFramesToSend = numFramesPassthrough + numFramesReplaceData + numFramesModifyData;

let streamsCreatedAtNegotiation;

const ontrackPromise = new Promise(resolve => {
callee.ontrack = t.step_func(() => {
const audioReceiver = callee.getReceivers().find(r => r.track.kind === 'audio');
assert_not_equals(audioReceiver, undefined);

const receiverStreams =
let receiverReader;
let receiverWriter;
if (streamsCreatedAtNegotiation) {
const audioStreams = streamsCreatedAtNegotiation.find(r => r.kind === 'audio');
assert_true(!!audioStreams);
receiverReader = audioStreams.streams.readable.getReader();
receiverWriter = audioStreams.streams.writable.getWriter();
} else {
const receiverStreams =
audioReceiver.createEncodedStreams();
const receiverReader = receiverStreams.readable.getReader();
const receiverWriter = receiverStreams.writable.getWriter();
receiverReader = receiverStreams.readable.getReader();
receiverWriter = receiverStreams.writable.getWriter();
}

const maxFramesToReceive = numFramesToSend;
let numVerifiedFrames = 0;
Expand All @@ -66,7 +77,7 @@
});

exchangeIceCandidates(caller, callee);
await negotiationFunction(caller, callee);
await negotiationFunction(caller, callee, (streams) => {streamsCreatedAtNegotiation = streams;});

// Pass frames as they come from the encoder.
for (let i = 0; i < numFramesPassthrough; i++) {
Expand Down Expand Up @@ -125,53 +136,15 @@
return ontrackPromise;
}

promise_test(async t => {
return testAudioFlow(t, exchangeOfferAnswer);
}, 'Frames flow correctly using insertable streams');

promise_test(async t => {
return testAudioFlow(t, exchangeOfferAnswerReverse);
}, 'Frames flow correctly using insertable streams when receiver starts negotiation');
for (const setConstructorParam of [false, true]) {
promise_test(async t => {
return testAudioFlow(t, exchangeOfferAnswer, setConstructorParam);
}, 'Frames flow correctly using insertable streams' + (setConstructorParam ? ' with param' : ''));

promise_test(async t => {
const caller = new RTCPeerConnection();
t.add_cleanup(() => caller.close());
const callee = new RTCPeerConnection();
t.add_cleanup(() => callee.close());
const stream = await navigator.mediaDevices.getUserMedia({audio:true});
const audioTrack = stream.getAudioTracks()[0];
t.add_cleanup(() => audioTrack.stop());

exchangeIceCandidates(caller, callee);
await exchangeOfferAnswer(caller, callee);

const audioSender = caller.addTrack(audioTrack);
assert_throws_dom("InvalidStateError", () => audioSender.createEncodedStreams());
}, 'RTCRtpSender.createEncodedStream() throws if not requested in PC configuration');

promise_test(async t => {
const caller = new RTCPeerConnection();
t.add_cleanup(() => caller.close());
const callee = new RTCPeerConnection();
t.add_cleanup(() => callee.close());
const stream = await navigator.mediaDevices.getUserMedia({audio:true});
const audioTrack = stream.getAudioTracks()[0];
t.add_cleanup(() => audioTrack.stop());

const audioSender = caller.addTrack(audioTrack);
const ontrackPromise = new Promise(resolve => {
callee.ontrack = t.step_func(() => {
const audioReceiver = callee.getReceivers().find(r => r.track.kind === 'audio');
assert_not_equals(audioReceiver, undefined);
assert_throws_dom("InvalidStateError", () => audioReceiver.createEncodedStreams());
resolve();
});
});

exchangeIceCandidates(caller, callee);
await exchangeOfferAnswer(caller, callee);
return ontrackPromise;
}, 'RTCRtpReceiver.createEncodedStream() throws if not requested in PC configuration');
promise_test(async t => {
return testAudioFlow(t, exchangeOfferAnswerReverse, setConstructorParam);
}, 'Frames flow correctly using insertable streams when receiver starts negotiation' + (setConstructorParam ? ' with param' : ''));
}

promise_test(async t => {
const caller = new RTCPeerConnection({encodedInsertableStreams:true});
Expand Down Expand Up @@ -223,7 +196,7 @@
};

await testAudioFlow(
t, exchangeOfferAnswer, verifyFramesSerializeAndDeserialize);
t, exchangeOfferAnswer, /*setConstructorParam=*/false, verifyFramesSerializeAndDeserialize);

// Ensure all of our cloned frames are still alive and well, despite the
// originals having been sent through the PeerConnection.
Expand All @@ -247,7 +220,7 @@
// Run audio flows which will assert that the frames received have the
// rtp timestamp set by our modification.
await testAudioFlow(
t, exchangeOfferAnswer, rewriteFrameTimestamps);
t, exchangeOfferAnswer, /*setConstructorParam=*/false, rewriteFrameTimestamps);
}, 'Modifying rtp timestamp');

</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,47 +12,6 @@
</head>
<body>
<script>
promise_test(async t => {
const caller = new RTCPeerConnection();
t.add_cleanup(() => caller.close());
const callee = new RTCPeerConnection();
t.add_cleanup(() => callee.close());
await setMediaPermission("granted", ["camera"]);
const stream = await navigator.mediaDevices.getUserMedia({video:true});
const videoTrack = stream.getVideoTracks()[0];
t.add_cleanup(() => videoTrack.stop());

exchangeIceCandidates(caller, callee);
await exchangeOfferAnswer(caller, callee);

const videoSender = caller.addTrack(videoTrack);
assert_throws_dom("InvalidStateError", () => videoSender.createEncodedStreams());
}, 'RTCRtpSender.createEncodedStream() throws if not requested in PC configuration');

promise_test(async t => {
const caller = new RTCPeerConnection();
t.add_cleanup(() => caller.close());
const callee = new RTCPeerConnection();
t.add_cleanup(() => callee.close());
const stream = await navigator.mediaDevices.getUserMedia({video:true});
const videoTrack = stream.getVideoTracks()[0];
t.add_cleanup(() => videoTrack.stop());

const videoSender = caller.addTrack(videoTrack);
const ontrackPromise = new Promise(resolve => {
callee.ontrack = t.step_func(() => {
const videoReceiver = callee.getReceivers().find(r => r.track.kind === 'video');
assert_not_equals(videoReceiver, undefined);
assert_throws_dom("InvalidStateError", () => videoReceiver.createEncodedStreams());
resolve();
});
});

exchangeIceCandidates(caller, callee);
await exchangeOfferAnswer(caller, callee);
return ontrackPromise;
}, 'RTCRtpReceiver.createEncodedStream() throws if not requested in PC configuration');

promise_test(async t => {
const caller = new RTCPeerConnection({encodedInsertableStreams:true});
t.add_cleanup(() => caller.close());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
</head>
<body>
<script>
async function testVideoFlow(t, negotiationFunction, frameCallback = () => {}) {
const caller = new RTCPeerConnection({encodedInsertableStreams:true});
async function testVideoFlow(t, negotiationFunction, setConstructorParam, frameCallback = () => {}) {
const caller = new RTCPeerConnection(setConstructorParam ? {encodedInsertableStreams:true} : {});
t.add_cleanup(() => caller.close());
const callee = new RTCPeerConnection({encodedInsertableStreams:true});
const callee = new RTCPeerConnection(setConstructorParam ? {encodedInsertableStreams:true} : {});
t.add_cleanup(() => callee.close());

await setMediaPermission("granted", ["camera"]);
Expand All @@ -34,15 +34,25 @@
const numFramesModifyData = 5;
const numFramesToSend = numFramesPassthrough + numFramesReplaceData + numFramesModifyData;

let streamsCreatedAtNegotiation;
const ontrackPromise = new Promise(resolve => {
callee.ontrack = t.step_func(() => {
const videoReceiver = callee.getReceivers().find(r => r.track.kind === 'video');
assert_not_equals(videoReceiver, undefined);

const receiverStreams =
videoReceiver.createEncodedStreams();
const receiverReader = receiverStreams.readable.getReader();
const receiverWriter = receiverStreams.writable.getWriter();
let receiverReader;
let receiverWriter;
if (streamsCreatedAtNegotiation) {
const videoStreams = streamsCreatedAtNegotiation.find(r => r.kind === 'video');
assert_true(!!videoStreams);
receiverReader = videoStreams.streams.readable.getReader();
receiverWriter = videoStreams.streams.writable.getWriter();
} else {
const receiverStreams =
videoReceiver.createEncodedStreams();
receiverReader = receiverStreams.readable.getReader();
receiverWriter = receiverStreams.writable.getWriter();
}

const maxFramesToReceive = numFramesToSend;
let numVerifiedFrames = 0;
Expand All @@ -66,7 +76,7 @@
});

exchangeIceCandidates(caller, callee);
await negotiationFunction(caller, callee);
await negotiationFunction(caller, callee, (streams) => {streamsCreatedAtNegotiation = streams;});

// Pass frames as they come from the encoder.
for (let i = 0; i < numFramesPassthrough; i++) {
Expand Down Expand Up @@ -127,16 +137,18 @@
return ontrackPromise;
}

promise_test(async t => {
return testVideoFlow(t, exchangeOfferAnswer);
}, 'Frames flow correctly using insertable streams');
for (const setConstructorParam of [false, true]) {
promise_test(async t => {
return testVideoFlow(t, exchangeOfferAnswer, setConstructorParam);
}, 'Frames flow correctly using insertable streams' + (setConstructorParam ? ' with param' : ''));

promise_test(async t => {
return testVideoFlow(t, exchangeOfferAnswerReverse);
}, 'Frames flow correctly using insertable streams when receiver starts negotiation');
promise_test(async t => {
return testVideoFlow(t, exchangeOfferAnswerReverse, setConstructorParam);
}, 'Frames flow correctly using insertable streams when receiver starts negotiation' + (setConstructorParam ? ' with param' : ''));
}

promise_test(async t => {
const caller = new RTCPeerConnection({encodedInsertableStreams:true});
const caller = new RTCPeerConnection();
t.add_cleanup(() => caller.close());
const stream = await navigator.mediaDevices.getUserMedia({video:true});
const track = stream.getTracks()[0];
Expand All @@ -155,7 +167,7 @@
clonedFrames.push(clone);
};

await testVideoFlow(t, exchangeOfferAnswer, verifyFramesSerializeAndDeserialize);
await testVideoFlow(t, exchangeOfferAnswer, /*setConstructorParam=*/false, verifyFramesSerializeAndDeserialize);

// Ensure all of our cloned frames are still alive and well, despite the
// originals having been sent through the PeerConnection.
Expand Down
12 changes: 11 additions & 1 deletion webrtc-encoded-transform/RTCPeerConnection-insertable-streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,18 @@ async function exchangeOfferAnswer(pc1, pc2) {
await pc1.setRemoteDescription(answer);
}

async function exchangeOfferAnswerReverse(pc1, pc2) {
async function exchangeOfferAnswerReverse(pc1, pc2, encodedStreamsCallback) {
const offer = await pc2.createOffer({offerToReceiveAudio: true, offerToReceiveVideo: true});
if (encodedStreamsCallback) {
// RTCRtpReceivers will have been created during the above createOffer call, so if the caller
// wants to createEncodedStreams synchronously after creation to ensure all frames pass
// through the transform, it will have to be done now.
encodedStreamsCallback(
pc2.getReceivers().map(r => {
return {kind: r.track.kind, streams: r.createEncodedStreams()};
}));
}

// Munge the SDP to enable the GFD extension in order to get correct metadata.
const sdpABS = enableExtension(offer.sdp, ABS_V00_EXTENSION);
const sdpGFD = enableExtension(sdpABS, GFD_V00_EXTENSION);
Expand Down

0 comments on commit 37fc071

Please sign in to comment.