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.

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

Add OpusEncoderConfig.[signal/application] #777

Merged
merged 3 commits into from Mar 19, 2024

Conversation

tguilbert-google
Copy link
Member

Follow up work for #759.

AudioEncoderConfig.contenHint was not added, since only the Opus codec would benefit from it. Instead, we favored adding the individual codec configuration parameters directly to the Opus registry.

This PR adds OpusEncoderConfig.signal and OpusEncoderConfig.application.

@@ -46,6 +46,12 @@
"title": "RFC 7845: Ogg Encapsulation for the Opus Audio Codec",
"publisher": "IETF",
"date": "April 2016"
},
"OBJECT-RTC": {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if we're allowed to reference this, considering the draft status? I also don't know if WebCodecs wants to take a "dependency" here too.

Copy link
Collaborator

@aboba aboba Mar 5, 2024

Choose a reason for hiding this comment

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

I would keep the reference non-normative. Or you can just copy the text, not have a reference at all and add an acknowledgement citing ORTC.

</pre>

The {{OpusSignal}} describes the type of audio signal being encoded. See
[secion 9.3.1.1](https://draft.ortc.org/#opus-codec-options*) of [[OBJECT-RTC]].
Copy link
Member Author

Choose a reason for hiding this comment

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

Again, I'm not sure if referencing that section is allowed and adds much. The signal and application parameters are not defined with a lot of details there.

Copy link
Collaborator

@aboba aboba Mar 5, 2024

Choose a reason for hiding this comment

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

I would keep the reference non-normative. Or you can just copy the text, not have a reference at all and add an acknowledgement citing ORTC.

@@ -115,6 +121,8 @@
<xmp>
dictionary OpusEncoderConfig {
OpusBitstreamFormat format = "opus";
OpusSignal signal = "auto";
OpusApplication application = "audio";
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no default in libopus for OPUS_SET_APPLICATION (unlike the default of "auto" for OPUS_SET_SIGNAL). I think "audio" is the most generic/least surprising as a default.

@Djuffin Djuffin merged commit a5e076e into w3c:main Mar 19, 2024
16 of 17 checks passed
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 19, 2024
SHA: a5e076e
Reason: push, by Djuffin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 23, 2024
This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 15, 2024
This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 16, 2024
This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387232
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288112}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 16, 2024
This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387232
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288112}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 16, 2024
This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387232
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288112}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 23, 2024
…n], a=testonly

Automatic update from web-platform-tests
Add OpusEncoderConfig.[signal/application]

This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387232
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288112}

--

wpt-commits: 911ae4d05e14feedfe20fb0e83597ad485b77d10
wpt-pr: 45290
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Apr 24, 2024
…n], a=testonly

Automatic update from web-platform-tests
Add OpusEncoderConfig.[signal/application]

This CL adds optional configuration flags to the OpusEncoderConfig.

See matching spec change: w3c/webcodecs#777

Bug: 330949853
Change-Id: Iaa39ba0a710ffb482aa767deb8992f67c52ebb93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387232
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288112}

--

wpt-commits: 911ae4d05e14feedfe20fb0e83597ad485b77d10
wpt-pr: 45290
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants