Bug 150385 – ConvolverNode.buffer must have same sample rate as the AudioContext

• Component: Web Audio
• Status: RESOLVED FIXED

[Hyemi Shin]

ConvolverNode.buffer must be of the same sample-rate as the AudioContext or an NOT_SUPPORTED_ERR exception MUST be thrown.

[Hyemi Shin]

Created attachment 263664 [details]
Patch

[Darin Adler]

Comment on attachment 263664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263664&action=review

> Source/WebCore/ChangeLog:11
> +        No new tests.

Why? We need a test demonstrating this correct behavior.

[Hyemi Shin]

Created attachment 263769 [details]
Patch

[Hyemi Shin]

I'm not sure I made new test correctly.. Please take a look.

[Build Bot]

Comment on attachment 263769 [details]
Patch

Attachment 263769 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/318438

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263771 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 263769 [details]
Patch

Attachment 263769 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/318564

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263773 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 263769 [details]
Patch

Attachment 263769 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/318523

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263775 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Hyemi Shin]

Created attachment 263779 [details]
Patch

[Build Bot]

Comment on attachment 263779 [details]
Patch

Attachment 263779 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/319280

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263783 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 263779 [details]
Patch

Attachment 263779 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/319292

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263784 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Hyemi Shin]

Could anyone help me to fix test failiures on mavericks and yosemite only?

[Build Bot]

Comment on attachment 263779 [details]
Patch

Attachment 263779 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/319293

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263785 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Hyemi Shin]

Created attachment 263792 [details]
Patch

[Build Bot]

Comment on attachment 263792 [details]
Patch

Attachment 263792 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/319993

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263794 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Hyemi Shin]

Created attachment 263795 [details]
Patch

[Build Bot]

Comment on attachment 263795 [details]
Patch

Attachment 263795 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/320134

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263796 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Hyemi Shin]

Created attachment 263797 [details]
Patch

[Jer Noble]

(In reply to comment #25)
> Created attachment 263797 [details]
> Patch

r=me. Thanks!

[Hyemi Shin]

(In reply to comment #26)
> (In reply to comment #25)
> > Created attachment 263797 [details]
> > Patch
> 
> r=me. Thanks!

Thanks! could you give me cq+?

[Chris Dumez]

Comment on attachment 263797 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263797&action=review

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:1
> +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<!DOCTYPE html>

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:9
> +<div id="description"></div>

Not needed.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:10
> +<div id="console"></div>

Not needed.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:15
> +function runTest()

You don't really need a function.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:17
> +    if (window.testRunner) {

This whole if block is not needed.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:22
> +    window.jsTestIsAsync = true;

I don't see why this test needs to be async.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:25
> +    var conv = context.createConvolver();

No abbreviations in variable names please.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27
> +    try {

shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error: NOT_SUPPORTED_ERR'"); (or similar).

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37
> +    try {

Could use shouldThrow() as well.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:44
> +    finishJSTest();

Not needed.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:48
> +successfullyParsed = true;

Should not be needed.

[Chris Dumez]

Comment on attachment 263797 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263797&action=review

> Source/WebCore/ChangeLog:11
> +        Test added to LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html (amended)

Test: webaudio/convolver-setBuffer-different-samplerate.html

[Hyemi Shin]

(In reply to comment #28)
> Comment on attachment 263797 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=263797&action=review
> 
> > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27
> > +    try {
> 
> shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error:
> NOT_SUPPORTED_ERR'"); (or similar).
> 
> > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37
> > +    try {
> 
> Could use shouldThrow() 

I used shouldThrow() and shouldNotThrow() at first, but it made an error on Mavericks and Yosemite. That's because why I use try-catch. Rest of comments will be applied in next patch attachment. Thanks.

[Chris Dumez]

(In reply to comment #30)
> (In reply to comment #28)
> > Comment on attachment 263797 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=263797&action=review
> > 
> > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27
> > > +    try {
> > 
> > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error:
> > NOT_SUPPORTED_ERR'"); (or similar).
> > 
> > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37
> > > +    try {
> > 
> > Could use shouldThrow() 
> 
> I used shouldThrow() and shouldNotThrow() at first, but it made an error on
> Mavericks and Yosemite. That's because why I use try-catch. Rest of comments
> will be applied in next patch attachment. Thanks.

There is no reason for shouldThrow() to behave different on one platform vs another.

[Chris Dumez]

(In reply to comment #31)
> (In reply to comment #30)
> > (In reply to comment #28)
> > > Comment on attachment 263797 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=263797&action=review
> > > 
> > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27
> > > > +    try {
> > > 
> > > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error:
> > > NOT_SUPPORTED_ERR'"); (or similar).
> > > 
> > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37
> > > > +    try {
> > > 
> > > Could use shouldThrow() 
> > 
> > I used shouldThrow() and shouldNotThrow() at first, but it made an error on
> > Mavericks and Yosemite. That's because why I use try-catch. Rest of comments
> > will be applied in next patch attachment. Thanks.
> 
> There is no reason for shouldThrow() to behave different on one platform vs
> another.

I looked at your previous diffs and saw the following error:
+PASS conv.buffer = context.createBuffer(1, 100, 22050) threw exception ReferenceError: Can't find variable: conv.

The issue was that the 'conv' variable was in the runTest() function scope, which is not compatible with should*() functions. Once you get rid of the runTest() function as I suggested, you WILL be able to use shouldThrow() again (please do).

Note that alternatively, you could have declared 'conv' at global level by omitting the 'var'.

[Hyemi Shin]

(In reply to comment #32)
> (In reply to comment #31)
> > (In reply to comment #30)
> > > (In reply to comment #28)
> > > > Comment on attachment 263797 [details]
> > > > Patch
> > > > 
> > > > View in context:
> > > > https://bugs.webkit.org/attachment.cgi?id=263797&action=review
> > > > 
> > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27
> > > > > +    try {
> > > > 
> > > > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error:
> > > > NOT_SUPPORTED_ERR'"); (or similar).
> > > > 
> > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37
> > > > > +    try {
> > > > 
> > > > Could use shouldThrow() 
> > > 
> > > I used shouldThrow() and shouldNotThrow() at first, but it made an error on
> > > Mavericks and Yosemite. That's because why I use try-catch. Rest of comments
> > > will be applied in next patch attachment. Thanks.
> > 
> > There is no reason for shouldThrow() to behave different on one platform vs
> > another.
> 
> I looked at your previous diffs and saw the following error:
> +PASS conv.buffer = context.createBuffer(1, 100, 22050) threw exception
> ReferenceError: Can't find variable: conv.
> 
> The issue was that the 'conv' variable was in the runTest() function scope,
> which is not compatible with should*() functions. Once you get rid of the
> runTest() function as I suggested, you WILL be able to use shouldThrow()
> again (please do).
> 
> Note that alternatively, you could have declared 'conv' at global level by
> omitting the 'var'.

Ok, i will try. Thanks a lot.

[Hyemi Shin]

Created attachment 263890 [details]
Patch

[Chris Dumez]

Comment on attachment 263890 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263890&action=review

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:15
> +shouldThrow("convolver.buffer = context.createBuffer(1, 256, 22050)");

Please specify the exception as second parameter:
shouldThrow("convolver.buffer = context.createBuffer(1, 256, 22050)", "'Error: NotSupportedError: DOM Exception 9'");

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:18
> +runTest();

Forgot to remove this.

[Chris Dumez]

Comment on attachment 263890 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263890&action=review

> Source/WebCore/ChangeLog:11
> +        Test added to LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html

Also, this should look like:
Test: webaudio/convolver-setBuffer-different-samplerate.html

[Chris Dumez]

Comment on attachment 263890 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263890&action=review

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:1
> +<!DOCTYPE HTML">

again, this should be:
<!DOCTYPE html>

[Hyemi Shin]

Created attachment 263893 [details]
Patch

[Chris Dumez]

Comment on attachment 263893 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263893&action=review

This will NOT pass EWS.

> Source/WebCore/ChangeLog:11
> +        Test : webaudio/convolver-setBuffer-different-samplerate.html

Test:

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate-expected.txt:5
> +PASS conv.buffer = context.createBuffer(1, 256, 22050) threw exception Error: NotSupportedError: DOM Exception 9.

conv -> convolver

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate-expected.txt:6
> +PASS conv.buffer = context.createBuffer(1, 256, 44100) did not throw exception

ditto.

> LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:16
> +shouldNotThrow("conv.buffer = context.createBuffer(1, 256, 44100)");

Please run your test..

conv -> convolver

[Hyemi Shin]

Created attachment 263895 [details]
Patch

[Build Bot]

Comment on attachment 263895 [details]
Patch

Attachment 263895 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/325579

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263897 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Chris Dumez]

How do you generate your test expectations?

The diff looks like so:
http://pastebin.com/dn0d7FD5

[Chris Dumez]

(In reply to comment #43)
> How do you generate your test expectations?
> 
> The diff looks like so:
> http://pastebin.com/dn0d7FD5

We usually do:
Tools/Scripts/run-webkit-tests --reset-results webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Comment on attachment 263895 [details]
Patch

Attachment 263895 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/325589

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263898 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 263895 [details]
Patch

Attachment 263895 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/325629

New failing tests:
webaudio/convolver-setBuffer-different-samplerate.html

[Build Bot]

Created attachment 263901 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Hyemi Shin]

Created attachment 263905 [details]
Patch

[Chris Dumez]

Comment on attachment 263905 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263905&action=review

r=me

> Source/WebCore/ChangeLog:11
> +        Test : webaudio/convolver-setBuffer-different-samplerate.html

In the future, please no space before the ':'. No need to reupload for this though.

[Hyemi Shin]

(In reply to comment #50)
> Comment on attachment 263905 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=263905&action=review
> 
> r=me
> 
> > Source/WebCore/ChangeLog:11
> > +        Test : webaudio/convolver-setBuffer-different-samplerate.html
> 
> In the future, please no space before the ':'. No need to reupload for this
> though.

Thank you for your advices!

[WebKit Commit Bot]

Comment on attachment 263905 [details]
Patch

Clearing flags on attachment: 263905

Committed r191492: <http://trac.webkit.org/changeset/191492>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.