Closed Bug 1388591 Opened 7 years ago Closed 7 years ago

Implement OfflineAudioContextOptions

Categories

(Core :: Web Audio, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: wisniewskit, Assigned: wisniewskit)

References

()

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

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
Paul, care to triage?
Flags: needinfo?(padenot)
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
Assignee: nobody → wisniewskit
Rank: 18
Flags: needinfo?(padenot)
Priority: -- → P1
Thanks Thomas, that's great! Let me know when you're ready for review!
Sure. The try-run only had some unrelated intermittents, so I've published the review request.
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.
Attachment #8895328 - Flags: review?(padenot) → review+
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.
Attachment #8895328 - Flags: review?(bugs)
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
Attachment #8895328 - Flags: review?(bugs) → review+
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);
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.
I guess we should go ahead and request check-in for this patch, then?
I'll land for you right now.
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/autoland/rev/654d5ec3370c
Implement OfflineAudioCanvas dict constructor; r=padenot,smaug
https://hg.mozilla.org/mozilla-central/rev/654d5ec3370c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: