849230 - Limit the range of sample rates according to the spec

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[(no longer active)]

We should limit the range of accepted sampling rates to 22050 through 96000.

Comment 1

[Paul Adenot (:padenot)]

If we want WebRTC to play nice with WebAudio, we should allow a samplerate of 8000Hz, since G711 (one of the two codecs supported) has a samplerate of 8000Hz.

Comment 2

[(no longer active)]

(In reply to Paul Adenot (:padenot) from comment #1)
> If we want WebRTC to play nice with WebAudio, we should allow a samplerate
> of 8000Hz, since G711 (one of the two codecs supported) has a samplerate of
> 8000Hz.

Ah, very good point.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 722845 [details] [diff] [review]
Patch (v1)

Review of attachment 722845 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 5

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/effe123d3844

Comment 6

[phi2x]

Why restricting the sampling rate only to 96kHz max?
It sounds quite arbitrary to me, and I can't see any hard technical limits to justify it.

But if you are serious in wanting to impose arbitrary limitations to web devs, please, make the spec support at least up to 192kHz sample rate.

Here's some evidences that there is need for it:
http://www.intel.com/content/www/us/en/chipsets/high-definition-audio.html
"Intel HD Audio hardware is capable of delivering the support and sound quality for up to eight channels at 192 kHz/32-bit quality"

http://en.wikipedia.org/wiki/BD-ROM#Audio
Specification of BD-ROM Primary audio streams:
For LPCM, Dolby TrueHD & DTS-HD Master Audio, max. channel: 6 @ 192 kHz.

Comment 7

[(no longer active)]

(In reply to comment #6)
> Why restricting the sampling rate only to 96kHz max?
> It sounds quite arbitrary to me, and I can't see any hard technical limits to
> justify it.
> 
> But if you are serious in wanting to impose arbitrary limitations to web devs,
> please, make the spec support at least up to 192kHz sample rate.
> 
> Here's some evidences that there is need for it:
> http://www.intel.com/content/www/us/en/chipsets/high-definition-audio.html
> "Intel HD Audio hardware is capable of delivering the support and sound quality
> for up to eight channels at 192 kHz/32-bit quality"
> 
> http://en.wikipedia.org/wiki/BD-ROM#Audio
> Specification of BD-ROM Primary audio streams:
> For LPCM, Dolby TrueHD & DTS-HD Master Audio, max. channel: 6 @ 192 kHz.

Can you please comment on <https://www.w3.org/Bugs/Public/show_bug.cgi?id=21223>?  I'm fine with changing this to whatever we end up with on the spec side.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/effe123d3844

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 722845 [details] [diff] [review]
Patch (v1)

Review of attachment 722845 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/test/test_AudioBuffer.html
@@ +39,5 @@
> +    context.createBuffer(2, 2048, 7999);
> +  }, DOMException.DOM_SYNTAX_ERR);
> +  expectException(function() {
> +    context.createBuffer(2, 2048, 96001);
> +  }, DOMException.DOM_SYNTAX_ERR);

I don't think you meant to pass DOMException.DOM_SYNTAX_ERR === undefined here.

Comment 10

[Marat Tanalin]

I believe this is a mistake. Such limitation is out of the spec scope.

See:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=21223#c6

Comment 11

[(no longer active)]

(In reply to :Ms2ger from comment #9)
> Comment on attachment 722845 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 722845 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/webaudio/test/test_AudioBuffer.html
> @@ +39,5 @@
> > +    context.createBuffer(2, 2048, 7999);
> > +  }, DOMException.DOM_SYNTAX_ERR);
> > +  expectException(function() {
> > +    context.createBuffer(2, 2048, 96001);
> > +  }, DOMException.DOM_SYNTAX_ERR);
> 
> I don't think you meant to pass DOMException.DOM_SYNTAX_ERR === undefined
> here.

Follow-up pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/8fc27301f35c

Thanks for catching this!

Comment 12

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/8fc27301f35c

Comment 13

[(no longer active)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.