Skip to content

Add frameDuration attribute to OpusEncoderConfig #551

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 6, 2022
Merged

Add frameDuration attribute to OpusEncoderConfig #551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e1bc353
Add frameDuration attribute to OpusEncoderConfig
bdrtc Sep 5, 2022
31b0ec4
change type of frameDuration in OpusEncoderConfig to unsigned long lo…
bdrtc Sep 7, 2022
8a91ad7
Merge branch 'main' into 371-add-frameduration-to-opus-encoder-config
bdrtc Sep 7, 2022
bcdda20
Merge branch 'main' into 371-add-frameduration-to-opus-encoder-config
helaozhao Sep 21, 2022
7376fda
change the type of frameDuration in OpusEncoderConfig to unsigned lon…
helaozhao Sep 21, 2022
7d10b29
fix wrong reference of RFC7587 in opus_codec_registration
bdrtc Sep 23, 2022
67cc867
wrap ptime with backticks in opus_codec_registration.src.html
bdrtc Sep 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion index.src.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2073,7 +2073,10 @@
run these steps:
1. If {{AudioEncoderConfig/codec}} is not a <a>valid codec string</a>, return
`false`.
2. Return `true`.
2. If the {{AudioEncoderConfig}} has a codec-specific extension and the corresponding
registration in the [[WEBCODECS-CODEC-REGISTRY]] defines steps to check whether the
the extension is a valid extension, return the result of running those steps.
3. Return `true`.

<dl>
<dt><dfn dict-member for=AudioEncoderConfig>codec</dfn></dt>
Expand Down
12 changes: 11 additions & 1 deletion opus_codec_registration.src.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
NOTE: Once the initialization has succeeded, any packet can be decoded at any
time without error, but this might not result in the expected audio output.

AudiEncoderConfig extensions {#audioencoderconfig-extensions}
AudioEncoderConfig extensions {#audioencoderconfig-extensions}
=============================================================

<pre class='idl'>
Expand All @@ -131,16 +131,26 @@
<xmp>
dictionary OpusEncoderConfig {
OpusBitstreamFormat format = "opus";
[EnforceRange] unsigned long frameDuration;
Copy link
Contributor

@dalecurtis dalecurtis Sep 6, 2022

Choose a reason for hiding this comment

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

Should probably be "long long" and in microseconds like all other time fields in WebCodecs.

Copy link
Contributor Author

@bdrtc bdrtc Sep 7, 2022

Choose a reason for hiding this comment

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

we do find duration in audioData is in microseconds,
as described in #371 ,
frameSize = numberOfChannels*sampleRate * frameDuration / 1000
most audio encoder use millseconds as frame duration(opus ptime for example), we can change it to micoseconds, then ,the frameSize Calculation will be:
frameSize = numberOfChannels*sampleRate * frameDuration / 1000/1000

Thanks.

Copy link
Contributor Author

@bdrtc bdrtc Sep 9, 2022

Choose a reason for hiding this comment

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

@dalecurtis Do you see the new commit according to your comments? This is my first PR that need some modifications, I'm afraid of missing an important action.

Copy link
Member

@tguilbert-google tguilbert-google Sep 9, 2022

Choose a reason for hiding this comment

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

We can see the new commits. This issue will be discussed at TPAC next week, so we might hold off on merging this PR until those discussions happen.

Copy link
Contributor

@dalecurtis dalecurtis Sep 9, 2022

Choose a reason for hiding this comment

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

I defer to Thomas for the final decision, I was just driving by.

Copy link
Member

@tguilbert-google tguilbert-google Sep 17, 2022

Choose a reason for hiding this comment

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

@aboba, @padenot: any thoughts on microseconds vs milliseconds?

IIUC, the valid frameDurations in milliseconds are {2.5, 5, 10, 20, 40, 60} according to the spec. Leaving it in microseconds might give the impression of finer grain support.

Copy link
Collaborator

@aboba aboba Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

As you say, Opus ptimes have a limited set of values, are expressed in ms and are rounded up. So if you're willing to make the lowest frameDuration 3 instead of 2.5, you don't really need microseconds.

};
</xmp>
</pre>

To check if an {{OpusEncoderConfig}} is valid, run these steps:
1. If {{OpusEncoderConfig/frameDuration}} is not a valid `ptime` value,
which is described in Section 6.1 of [[RFC7587]], return `false`.
2. Return `true`.

<dl>
<dt><dfn dict-member for=OpusEncoderConfig>format</dfn></dt>
<dd>
Configures the format of output {{EncodedAudioChunk}}s. See
{{OpusBitstreamFormat}}.
</dd>
<dt><dfn dict-member for=OpusEncoderConfig>frameDuration</dfn></dt>
<dd>
Configures the frame duration, in milliseconds, of output {{EncodedAudioChunk}}s.
</dd>
</dl>

OpusBitstreamFormat {#opus-bitstream-format}
Expand Down