webcodecs: Make AudioEncoder emit decoder config with extra

Eugene Zemtsov (Gerrit)

unread,

Feb 11, 2021, 11:46:59 PM2/11/21

to Thomas Guilbert, Chrome Cunningham, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Eugene Zemtsov

Attention is currently required from: Thomas Guilbert, Chrome Cunningham.

Eugene Zemtsov would like Thomas Guilbert and Chrome Cunningham to review this change.

View Change

webcodecs: Make AudioEncoder emit decoder config with extra_data

Bug: 1177021
Change-Id: I6d6fe69433b954b65cfec3fd6bc6dc78564ee168
---
M third_party/blink/renderer/modules/webcodecs/audio_encoder.cc
M third_party/blink/renderer/modules/webcodecs/audio_encoder.h
M third_party/blink/renderer/modules/webcodecs/encoded_audio_chunk_output_callback.idl
M third_party/blink/web_tests/external/wpt/webcodecs/audio-encoder.any.js
4 files changed, 84 insertions(+), 10 deletions(-)

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Eugene Zemtsov (Gerrit)

unread,

Feb 11, 2021, 11:47:07 PM2/11/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Chrome Cunningham, Thomas Guilbert, chromium...@chromium.org

Attention is currently required from: Thomas Guilbert, Chrome Cunningham.

View Change

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Blink WPT Bot (Gerrit)

unread,

Feb 12, 2021, 12:01:09 AM2/12/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Chromium LUCI CQ, Chrome Cunningham, Thomas Guilbert, chromium...@chromium.org

Attention is currently required from: Eugene Zemtsov, Thomas Guilbert, Chrome Cunningham.

Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/27600.

When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.

WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md#Automatic-export-process

View Change

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Thomas Guilbert (Gerrit)

unread,

Feb 12, 2021, 1:37:24 PM2/12/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Thomas Guilbert, Blink WPT Bot, Chromium LUCI CQ, Chrome Cunningham, chromium...@chromium.org

Attention is currently required from: Eugene Zemtsov, Chrome Cunningham.

Patch set 2:Code-Review +1

View Change

2 comments:

Patchset:
- Patch Set #2:
  LGTM % 1 minor comment.
File third_party/blink/web_tests/external/wpt/webcodecs/audio-encoder.any.js:
- Patch Set #2, Line 199:
```
      if (decoder_config)
        assert_equals(config, null);
```
  Can you make it more obvious that this is guaranteed to be hit during the test? E.g. Add a comment on L#215 calling out that this assert and |output_count| > 1 guarantees that configs are only emitted once.

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Eugene Zemtsov (Gerrit)

unread,

Feb 12, 2021, 6:05:52 PM2/12/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Thomas Guilbert, Blink WPT Bot, Chromium LUCI CQ, Chrome Cunningham, chromium...@chromium.org

Attention is currently required from: Chrome Cunningham.

View Change

1 comment:

File third_party/blink/web_tests/external/wpt/webcodecs/audio-encoder.any.js:

Patch Set #2, Line 199:

      if (decoder_config)
        assert_equals(config, null);

- Can you make it more obvious that this is guaranteed to be hit during the test? E.g. […]
  Done

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Eugene Zemtsov (Gerrit)

unread,

Feb 12, 2021, 7:17:38 PM2/12/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Thomas Guilbert, Blink WPT Bot, Chromium LUCI CQ, Chrome Cunningham, chromium...@chromium.org

Attention is currently required from: Chrome Cunningham.

Patch set 4:Commit-Queue +2

View Change

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Feb 12, 2021, 7:17:59 PM2/12/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Thomas Guilbert, Blink WPT Bot, Chrome Cunningham, chromium...@chromium.org

Attention is currently required from: Chrome Cunningham.

CQ is trying the patch.

Note: The patchset #4 "optional decoder_config" sent to CQ was uploaded after this CL was CR+1-ed.
Reviewer, please verify there is nothing unexpected https://chromium-review.googlesource.com/c/2691321/4

Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/2691321/4

Bot data: {"action": "start", "triggered_at": "2021-02-13T00:17:26.0Z", "revision": "2e79f0aa2bece66aae47f708f592e4d3b3de1a53"}

View Change

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Feb 12, 2021, 7:22:47 PM2/12/21

to Eugene Zemtsov, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Thomas Guilbert, Blink WPT Bot, Chrome Cunningham, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change

Approvals:
  Thomas Guilbert: Looks good to me
  Eugene Zemtsov: Commit

webcodecs: Make AudioEncoder emit decoder config with extra_data

Bug: 1177021
Change-Id: I6d6fe69433b954b65cfec3fd6bc6dc78564ee168

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2691321
Commit-Queue: Eugene Zemtsov <eug...@chromium.org>
Reviewed-by: Thomas Guilbert <tgui...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#853747}


---
M third_party/blink/renderer/modules/webcodecs/audio_encoder.cc
M third_party/blink/renderer/modules/webcodecs/audio_encoder.h
M third_party/blink/renderer/modules/webcodecs/encoded_audio_chunk_output_callback.idl
M third_party/blink/web_tests/external/wpt/webcodecs/audio-encoder.any.js

4 files changed, 91 insertions(+), 10 deletions(-)

The change was submitted with unreviewed changes in the following files:

The name of the file: third_party/blink/renderer/modules/webcodecs/encoded_audio_chunk_output_callback.idl
Insertions: 1, Deletions: 1.

@@ -8:9, +8:9 @@
-    void(EncodedAudioChunk output, AudioDecoderConfig decoder_config);
+    void(EncodedAudioChunk output, optional AudioDecoderConfig decoder_config);


The name of the file: third_party/blink/web_tests/external/wpt/webcodecs/audio-encoder.any.js
Insertions: 7, Deletions: 3.

@@ +197:199 @@
+        // Only the first invocation of the output callback is supposed to have
+        // a |config| in it.

@@ -198:201, +200:202 @@
-        if (decoder_config)
-          assert_equals(config, null);
-        else
+        if (output_count == 1) {
+          assert_not_equals(config, null);

@@ +203:206 @@
+        } else {
+          assert_equals(config, null);
+        }

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Blink WPT Bot (Gerrit)

unread,

Feb 12, 2021, 8:06:58 PM2/12/21

to Eugene Zemtsov, Chromium LUCI CQ, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, Thomas Guilbert, Chrome Cunningham, chromium...@chromium.org

The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/27600

View Change

To view, visit change 2691321. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward

webcodecs: Make AudioEncoder emit decoder config with extra_data [chromium/src : master]

Eugene Zemtsov (Gerrit)

Eugene Zemtsov (Gerrit)

Blink WPT Bot (Gerrit)

Thomas Guilbert (Gerrit)

Eugene Zemtsov (Gerrit)

Eugene Zemtsov (Gerrit)

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ (Gerrit)

Blink WPT Bot (Gerrit)