1388591 - Implement OfflineAudioContextOptions

Status: RESOLVED FIXED

(Core :: Web Audio, enhancement, P1)

Description

[Thomas Wisniewski]

OfflineAudioContext now has a constructor variant taking a dictionary of parameters, OfflineAudioContextOptions: https://webaudio.github.io/web-audio-api/#offlineaudiocontextoptions

Chrome intends to ship this in version 62: https://www.chromestatus.com/feature/5635820090294272

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Paul, care to triage?

Comment 2

[Thomas Wisniewski]

Just FYI, I had some time tonight and ended up writing a patch that's going through try (I'll put it up on reviewboard if it passes): https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc11a87d76f85edff6b2eb31573ef13b60659e03

Comment 3

[Paul Adenot (:padenot)]

Thanks Thomas, that's great! Let me know when you're ready for review!

Comment 4

[Thomas Wisniewski]

Attachment: Bug 1388591 - Implement OfflineAudioCanvas dict constructor;

Review commit: https://reviewboard.mozilla.org/r/166376/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166376/

Comment 5

[Thomas Wisniewski]

Sure. The try-run only had some unrelated intermittents, so I've published the review request.

Comment 6

[Paul Adenot (:padenot)]

Comment on attachment 8895328 [details]
Bug 1388591 - Implement OfflineAudioCanvas dict constructor;

https://reviewboard.mozilla.org/r/166376/#review171664

Looks good. This is modifying a webidl file, though, we need a DOM peer to review this.

Comment 7

[Paul Adenot (:padenot)]

Comment on attachment 8895328 [details]
Bug 1388591 - Implement OfflineAudioCanvas dict constructor;

smaug, can you have a look at this one? The spec text is in comment 0.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8895328 [details]
Bug 1388591 - Implement OfflineAudioCanvas dict constructor;

https://reviewboard.mozilla.org/r/166376/#review171746

::: dom/media/webaudio/test/test_OfflineAudioContext.html:35
(Diff revision 1)
>  }
>  
>  SimpleTest.waitForExplicitFinish();
>  addLoadEvent(function() {
> -  var ctx = new OfflineAudioContext(2, 100, 22050);
> +  let ctxs = [
> +    new OfflineAudioContext(2, 100, 22050),

I hope there are tests also for some crazy values, like millions of channels or so

Comment 9

[Thomas Wisniewski]

Not for millions of values, but there are tests for larger-than-expected numbers of channels and other unexpected/unsupported values:

>  expectException(function() {
>    new OfflineAudioContext(2, 100, 0);
>  }, DOMException.NOT_SUPPORTED_ERR);
>  expectException(function() {
>    new OfflineAudioContext(2, 100, -1);
>  }, DOMException.NOT_SUPPORTED_ERR);
>  expectException(function() {
>    new OfflineAudioContext(0, 100, 44100);
>  }, DOMException.NOT_SUPPORTED_ERR);
>  new OfflineAudioContext(32, 100, 44100);
>  expectException(function() {
>    new OfflineAudioContext(33, 100, 44100);
>  }, DOMException.NOT_SUPPORTED_ERR);
>  expectException(function() {
>    new OfflineAudioContext(2, 0, 44100);
>  }, DOMException.NOT_SUPPORTED_ERR);

Comment 10

[Paul Adenot (:padenot)]

Curiously, we seem to have tests for illegal channel count, but not for crazy high rate. We have tests for negative rate though. I filed bug 1388802.

This is probably exercised in crashtests of web platform tests. If this is the case, I'll close bug 1388802.

Comment 11

[Thomas Wisniewski]

I guess we should go ahead and request check-in for this patch, then?

Comment 12

[Paul Adenot (:padenot)]

I'll land for you right now.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/654d5ec3370c
Implement OfflineAudioCanvas dict constructor; r=padenot,smaug

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/654d5ec3370c

Comment 15

[Eric Shepherd [:sheppy]]

Updated:

https://developer.mozilla.org/en-US/docs/Web/API/OfflineAudioContext/OfflineAudioContext
https://developer.mozilla.org/en-US/docs/Web/API/OfflineAudioContext (compat table primarily)

Also added to Firefox 57 for developers.