Implement OfflineAudioContextOptions

RESOLVED FIXED in Firefox 57

Status

()

enhancement
P1
normal
Rank:
18
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: wisniewskit, Assigned: wisniewskit)

Tracking

({dev-doc-complete})

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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)
(Assignee)

Comment 2

2 years ago
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!
Comment hidden (mozreview-request)
(Assignee)

Comment 5

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

Comment 6

2 years ago
mozreview-review
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 8

2 years ago
mozreview-review
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+
(Assignee)

Comment 9

2 years ago
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.
(Assignee)

Comment 11

2 years ago
I guess we should go ahead and request check-in for this patch, then?
I'll land for you right now.

Comment 13

2 years ago
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/autoland/rev/654d5ec3370c
Implement OfflineAudioCanvas dict constructor; r=padenot,smaug

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/654d5ec3370c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.