Skip to content

Commit

Permalink
Revert "webcodecs: Stop closing frames in Audio|VideoEncoder.encode()"
Browse files Browse the repository at this point in the history
This reverts commit 477ea10ee350777a599a4541b6e436ec42563597.

Reason for revert:

It looks like this CL is responsible for breaking the step
"blink_web_tests" on the builder "WebKit Linux MSAN".

First failing build:
https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Linux%20MSAN/9927

Original change's description:
> webcodecs: Stop closing frames in Audio|VideoEncoder.encode()
>
> Change-Id: I0281c1bdd836a94f968bf8fdf4fe8d09392845c7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2803617
> Reviewed-by: Thomas Guilbert <[email protected]>
> Reviewed-by: Chrome Cunningham <[email protected]>
> Commit-Queue: Eugene Zemtsov <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#869112}

Change-Id: Iacb9e31ff575129a7b320954a5d65dfaf6274b19
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2805814
Bot-Commit: Rubber Stamper <[email protected]>
Owners-Override: Fabian Sommer <[email protected]>
Auto-Submit: Fabian Sommer <[email protected]>
Commit-Queue: Fabian Sommer <[email protected]>
Cr-Commit-Position: refs/heads/master@{#869156}
  • Loading branch information
Fabian-Sommer authored and chromium-wpt-export-bot committed Apr 5, 2021
1 parent 9c622c8 commit f37d80e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
8 changes: 3 additions & 5 deletions webcodecs/audio-encoder.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ promise_test(async t => {
let frame = make_audio_frame(timestamp_us, config.numberOfChannels,
config.sampleRate, frame_length);
encoder.encode(frame);
frame.close();
timestamp_us += frame_duration_s * 1_000_000;
}
await encoder.flush();
Expand Down Expand Up @@ -103,7 +102,6 @@ async function checkEncodingError(config, good_frames, bad_frame) {
encoder.configure(config);
for (let frame of good_frames) {
encoder.encode(frame);
frame.close();
}
await encoder.flush();

Expand Down Expand Up @@ -208,7 +206,7 @@ promise_test(async t => {
for (let i = 0; i < frame_count; i++) {
let frame = make_audio_frame(timestamp_us, config.numberOfChannels,
config.sampleRate, frame_length);
input_frames.push(frame);
input_frames.push(clone_frame(frame));
encoder.encode(frame);
timestamp_us += frame_duration_s * 1_000_000;
}
Expand Down Expand Up @@ -276,7 +274,7 @@ promise_test(async t => {

let long_frame = make_audio_frame(0, encoder_config.numberOfChannels,
encoder_config.sampleRate, encoder_config.sampleRate);
encoder.encode(long_frame);
encoder.encode(clone_frame(long_frame));
await encoder.flush();

// Long frame produced more than one output, and we've got decoder_config
Expand All @@ -297,7 +295,7 @@ promise_test(async t => {
output_count = 0;
encoder_config.bitrate = 256000;
encoder.configure(encoder_config);
encoder.encode(long_frame);
encoder.encode(clone_frame(long_frame));
await encoder.flush();

// After reconfiguring encoder should produce decoder config again
Expand Down
40 changes: 31 additions & 9 deletions webcodecs/video-encoder.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ promise_test(async t => {
let frame1 = await createVideoFrame(640, 480, 0);
let frame2 = await createVideoFrame(640, 480, 33333);

encoder.encode(frame1);
encoder.encode(frame2);
encoder.encode(frame1.clone());
encoder.encode(frame2.clone());

// Could be 0, 1, or 2. We can't guarantee this check runs before the UA has
// processed the encodes.
Expand Down Expand Up @@ -148,7 +148,6 @@ promise_test(async t => {
let frame = new VideoFrame(bitmap, { timestamp: timestamp });
timestamp += timestamp_step;
encoder.encode(frame);
frame.close();
}

await t.step_wait(() => reset_completed,
Expand All @@ -168,7 +167,6 @@ promise_test(async t => {
let frame = await createVideoFrame(800, 600, timestamp + 1);
timestamp += timestamp_step;
encoder.encode(frame);
frame.close();
}
await encoder.flush();

Expand Down Expand Up @@ -196,10 +194,10 @@ promise_test(async t => {
let frame1 = await createVideoFrame(640, 480, 0);
let frame2 = await createVideoFrame(640, 480, 33333);

encoder.encode(frame1);
encoder.encode(frame1.clone());
encoder.configure(config);

encoder.encode(frame2);
encoder.encode(frame2.clone());

await encoder.flush();

Expand All @@ -226,21 +224,45 @@ promise_test(async t => {
let frame3 = await createVideoFrame(640, 480, 66666);
let frame4 = await createVideoFrame(640, 480, 100000);

encoder.encode(frame3);
encoder.encode(frame3.clone());

// Verify that a failed call to configure does not change the encoder's state.
let badConfig = { ...defaultConfig };
badConfig.codec = 'bogus';
assert_throws_js(TypeError, () => encoder.configure(badConfig));

encoder.encode(frame4);
encoder.encode(frame4.clone());

await encoder.flush();

assert_equals(output_chunks[0].timestamp, frame3.timestamp);
assert_equals(output_chunks[1].timestamp, frame4.timestamp);
}, 'Test successful encode() after re-configure().');

promise_test(async t => {
let output_chunks = [];
let codecInit = getDefaultCodecInit(t);
codecInit.output = chunk => output_chunks.push(chunk);

let encoder = new VideoEncoder(codecInit);

let timestamp = 33333;
let frame = await createVideoFrame(640, 480, timestamp);

encoder.configure(defaultConfig);
assert_equals(encoder.state, "configured");

encoder.encode(frame);

// |frame| is not longer valid since it has been closed.
assert_not_equals(frame.timestamp, timestamp);
assert_throws_dom("InvalidStateError", () => frame.clone());

encoder.close();

return endAfterEventLoopTurn();
}, 'Test encoder consumes (closes) frames.');

promise_test(async t => {
let encoder = new VideoEncoder(getDefaultCodecInit(t));

Expand All @@ -266,6 +288,6 @@ promise_test(async t => {
encoder.configure(defaultConfig);

assert_throws_dom("OperationError", () => {
encoder.encode(frame);
encoder.encode(frame)
});
}, 'Verify encoding closed frames throws.');

0 comments on commit f37d80e

Please sign in to comment.