Add frameDuration attribute to OpusEncoderConfig

[bdrtc]

This fixes #371

---

Preview | Diff

[tguilbert-google]

@aboba, @padenot : does this need editor's approval? If so, please take a look.

[padenot]

We probably want to have something to do the error checking/validation/rounding to nearest here. Only a very small set of numbers is valid. What happens if you pass something that Opus can't work with?

[bdrtc]

if we want add error checking, it should be run in audioencoder configure method, by checking the validation of AudioEncodeConfig, Add one more check step like this:

2. if codec is opus and frameDuration exist, frameDuration is not in {2.5, 5, 10, 20, 40, 60}, return false.
   what's more, frameDuration type should be changed to double.
   @tguilbert-google @padenot any more sugguestion about this ? thanks.

[aboba]

I think the allowable values are a bit broader. From RFC 7587 Section 6.1:

" ptime: the preferred duration of media represented by a packet (according to Section 6 of [RFC4566]) that a decoder wants to receive, in milliseconds rounded up to the next full integer value. Possible values are 3, 5, 10, 20, 40, 60, or an arbitrary multiple of an Opus frame size rounded up to the next full integer value, up to a maximum value of 120, as defined in Section 4. If no value is specified, the default is 20."

[tguilbert-google]

2. if codec is opus and frameDuration exist, frameDuration is not in {2.5, 5, 10, 20, 40, 60}, return false.

I think we are trying to avoid codec-specific steps in the main spec. We could instead add a generic validation step referring to the registries, along the lines of "If the current codec's registry defines a codec-specific Additional configuration validation steps, run the algorithm defined in the registry. If the return value is ...", and add a section validating OpusEncoderConfig.

There is probably better wording though.

[padenot]

I agree with @tguilbert-google.

[bdrtc]

We add a new commit according to the disscussion above, please take a review. @tguilbert-google

[helaozhao]

another commit, please review it.

[tguilbert-google]

Looks good to me!

I'm leaving editors @aboba and @padenot to do the final approval and merge.

Thanks for the patch!

[bdrtc]

@aboba @padenot any thoughts about new commit?

[tguilbert-google]

Assuming no objections from @padenot, I am merging this change.

Chromium implementation work will be tracked in crbug.com/1372152

[tguilbert-google]

Thanks for the PR!