Support sample rates up to 384 kHz.

Support sample rates up to 384 kHz.

Allow AudioContext and OfflineAudioContext to support sample rates up
to 384 kHz. The <video> tag supports this already. This also means
that AudioBuffer's also support these rates.

Update tests to test for this.

BUG=644683
TEST=dom-exceptions.html, offlineaudiocontext-constructor.html

Committed: https://crrev.com/b907faaeab1c40d2081e951a198248e32fdf6242
Cr-Commit-Position: refs/heads/master@{#424801}

[Raymond Toy, 2016-09-12 18:50:14]

The CQ bit was checked by rtoy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 18:50:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raymond Toy, 2016-09-12 18:50:42]

rtoy@chromium.org changed reviewers:
+ hongchan@chromium.org

[Raymond Toy, 2016-09-12 18:50:43]

PTAL.

[Raymond Toy, 2016-09-12 18:54:07]

rtoy@chromium.org changed reviewers:
+ mpearson@chromium.org

[Raymond Toy, 2016-09-12 18:54:08]

mpearson@ Can you take a look at the histogram changes in
OfflineAudioContext.cpp?  We're widening the range from 192000 to 384000 and I'm
not sure if this is ok to change the histograms this way.

[commit-bot: I haz the power, 2016-09-12 22:55:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-12 22:55:56]

Dry run: This issue passed the CQ dry run.

[Mark P, 2016-09-13 19:06:39]

https://codereview.chromium.org/2334773003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webaudio/OfflineAudioContext.cpp (right):

https://codereview.chromium.org/2334773003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/OfflineAudioContext.cpp:109:
("WebAudio.OfflineAudioContext.SampleRate", 3000, 384000, 50));
Widening the histogram like this is messy because all the buckets will shift. 
(The 50 buckets are spread exponentially between the min and maximum values, and
you're changing the maximum values.)  Technically this is okay -- entries are
emitted to the histogram under the same conditions as before -- but the
dashboard will display a mess because different buckets will overlap.

I suggest changing the name of the histogram.  (This means marking the old
histogram as obsolete and adding a new one with the same description.)

If data continuity is important to you, you can also keep emits to the old
histogram name and delete the old histogram after a while.  I.e., have two
overlapping histograms emitted for some time.

[Raymond Toy, 2016-09-13 20:12:28]

https://codereview.chromium.org/2334773003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webaudio/OfflineAudioContext.cpp (right):

https://codereview.chromium.org/2334773003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/OfflineAudioContext.cpp:109:
("WebAudio.OfflineAudioContext.SampleRate", 3000, 384000, 50));
On 2016/09/13 19:06:39, Mark P wrote:
> Widening the histogram like this is messy because all the buckets will shift. 
> (The 50 buckets are spread exponentially between the min and maximum values,
and
> you're changing the maximum values.)  Technically this is okay -- entries are
> emitted to the histogram under the same conditions as before -- but the
> dashboard will display a mess because different buckets will overlap.
> 
> I suggest changing the name of the histogram.  (This means marking the old
> histogram as obsolete and adding a new one with the same description.)
> 
> If data continuity is important to you, you can also keep emits to the old
> histogram name and delete the old histogram after a while.  I.e., have two
> overlapping histograms emitted for some time.
> 

Yeah, I suspected it would make a mess of the existing data.  I'll add a new
histogram for this and think about whether to keep the old one around for a bit.

Thanks!

[Mark P, 2016-09-15 23:16:31]

histograms.xml lgtm once you *carefully* resolve the serious issues below

--mark

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp (right):

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp:273:
("WebAudio.AudioBuffer.SampleRateRatio384K", 0, 12800, 50));
Min of histogram should always be "1".  There will always be a "underflow"
bucket to catch 0s (and negative values) that's not included in the min.

https://codereview.chromium.org/2334773003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2334773003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:65845: +<histogram
name="WebAudio.AudioBuffer.SampleRate384" units="Hz">
You left off the "k" in the histogram name.

Also, your histograms sometimes use "k" and sometimes use "K" in the emit lines,
though always seem to use "k" (when you remember it!) here.  Please be
consistent (upper or lower case) for all these histograms, and definitely be
consistent for the same histogram between the emit line and histograms.xml.

[hongchan, 2016-09-26 17:17:54]

lgtm with nits

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/offlineaudiocontext-constructor-expected.txt
(right):

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/offlineaudiocontext-constructor-expected.txt:7:
PASS context = new OfflineAudioContext(2, 512, 2999) threw exception
IndexSizeError: Failed to construct 'OfflineAudioContext': The sampleRate
provided (2999) is outside the range [3000, 384000]..
Hmm. Why is this IndexSizedError? Shouldn't this be NotSupported?

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/offlineaudiocontext-constructor-expected.txt:12:
PASS context = new OfflineAudioContext(2, 1024, 384001) threw exception
IndexSizeError: Failed to construct 'OfflineAudioContext': The sampleRate
provided (384001) is outside the range [3000, 384000]..
Ditto.

[Raymond Toy, 2016-09-26 17:35:09]

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/offlineaudiocontext-constructor-expected.txt
(right):

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/offlineaudiocontext-constructor-expected.txt:7:
PASS context = new OfflineAudioContext(2, 512, 2999) threw exception
IndexSizeError: Failed to construct 'OfflineAudioContext': The sampleRate
provided (2999) is outside the range [3000, 384000]..
On 2016/09/26 17:17:54, hoch wrote:
> Hmm. Why is this IndexSizedError? Shouldn't this be NotSupported?

That appears to be a bug.  The spec doesn't say exactly what should happen, but
since these
are used to create a buffer, createBuffer says NotSupportedError for invalid
values.

[Raymond Toy, 2016-09-28 21:34:09]

On 2016/09/15 23:16:31, Mark P wrote:
> histograms.xml lgtm once you *carefully* resolve the serious issues below
> 
> --mark
> 
>
https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp (right):
> 
>
https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp:273:
> ("WebAudio.AudioBuffer.SampleRateRatio384K", 0, 12800, 50));
> Min of histogram should always be "1".  There will always be a "underflow"
> bucket to catch 0s (and negative values) that's not included in the min.
> 
>
https://codereview.chromium.org/2334773003/diff/40001/tools/metrics/histogram...
> File tools/metrics/histograms/histograms.xml (right):
> 
>
https://codereview.chromium.org/2334773003/diff/40001/tools/metrics/histogram...
> tools/metrics/histograms/histograms.xml:65845: +<histogram
> name="WebAudio.AudioBuffer.SampleRate384" units="Hz">
> You left off the "k" in the histogram name.
> 
> Also, your histograms sometimes use "k" and sometimes use "K" in the emit
lines,
> though always seem to use "k" (when you remember it!) here.  Please be
> consistent (upper or lower case) for all these histograms, and definitely be
> consistent for the same histogram between the emit line and histograms.xml.

Sorry about that! I wanted to save my state before TPAC, and I knew it wasn't
ready but forgot to mark this as WIP.

[Raymond Toy, 2016-10-05 15:24:58]

The CQ bit was checked by rtoy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 15:25:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 17:15:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 17:15:15]

Dry run: This issue passed the CQ dry run.

[Raymond Toy, 2016-10-10 16:28:32]

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp (right):

https://codereview.chromium.org/2334773003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp:273:
("WebAudio.AudioBuffer.SampleRateRatio384K", 0, 12800, 50));
On 2016/09/15 23:16:31, Mark P wrote:
> Min of histogram should always be "1".  There will always be a "underflow"
> bucket to catch 0s (and negative values) that's not included in the min.

Done.

https://codereview.chromium.org/2334773003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2334773003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:65845: +<histogram
name="WebAudio.AudioBuffer.SampleRate384" units="Hz">
On 2016/09/15 23:16:31, Mark P wrote:
> You left off the "k" in the histogram name.
> 
> Also, your histograms sometimes use "k" and sometimes use "K" in the emit
lines,
> though always seem to use "k" (when you remember it!) here.  Please be
> consistent (upper or lower case) for all these histograms, and definitely be
> consistent for the same histogram between the emit line and histograms.xml.

Renamed them all to say "384kHz".

[Raymond Toy, 2016-10-11 20:47:00]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-10-11 20:47:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org, hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2334773003/#ps100001
(title: "Rebase and fix up histograms.xml")

[commit-bot: I haz the power, 2016-10-11 20:47:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 20:55:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 20:55:02]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raymond Toy, 2016-10-12 17:11:54]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2016-10-12 17:12:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 18:39:00]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-12 18:42:07]

Description was changed from

==========
Support sample rates up to 384 kHz.

Allow AudioContext and OfflineAudioContext to support sample rates up
to 384 kHz. The <video> tag supports this already. This also means
that AudioBuffer's also support these rates.

Update tests to test for this.

BUG=644683
TEST=dom-exceptions.html, offlineaudiocontext-constructor.html
==========

to

==========
Support sample rates up to 384 kHz.

Allow AudioContext and OfflineAudioContext to support sample rates up
to 384 kHz. The <video> tag supports this already. This also means
that AudioBuffer's also support these rates.

Update tests to test for this.

BUG=644683
TEST=dom-exceptions.html, offlineaudiocontext-constructor.html

Committed: https://crrev.com/b907faaeab1c40d2081e951a198248e32fdf6242
Cr-Commit-Position: refs/heads/master@{#424801}
==========

[commit-bot: I haz the power, 2016-10-12 18:42:08]

Patchset 6 (id:??) landed as
https://crrev.com/b907faaeab1c40d2081e951a198248e32fdf6242
Cr-Commit-Position: refs/heads/master@{#424801}