Closed Bug 1265403 Opened 4 years ago Closed 4 years ago

Support chaining AudioParam automation methods

Categories

(Core :: Web Audio, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: padenot, Assigned: padenot)

Details

Attachments

(1 file)

This simply returns the AudioParam so that a chaining style can be used.

Spec changes at https://github.com/webaudio/web-audio-api/commit/1874c4395503a6b286bfc09fb91a6f1961896ca0.
Priority: -- → P2
Assignee: nobody → padenot
Comment on attachment 8742395 [details]
MozReview Request: Bug 1265403 - Support chaining AudioParam automation methods. r?smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47187/diff/1-2/
Comment on attachment 8742395 [details]
MozReview Request: Bug 1265403 - Support chaining AudioParam automation methods. r?smaug

https://reviewboard.mozilla.org/r/47187/#review43769

Those issues in tests fixed, r+

::: dom/media/webaudio/AudioParam.h:54
(Diff revision 2)
> +                                  double aDuration,
> +                                  ErrorResult& aRv)
>    {
>      if (!WebAudioUtils::IsTimeValid(aStartTime)) {
>        aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
> -      return;
> +      return this;

I guess returning 'this' in case we throw is safer from C++ point of view than returning null. JS gets an exception anyhow.

::: dom/media/webaudio/test/test_audioParamChaining.html:35
(Diff revision 2)
> +
> +var gain = oc.createGain();
> +
> +source.connect(gain).connect(oc.destination);
> +
> +// We chain three uutomation methods, making a gain step.

uutomation?

::: dom/media/webaudio/test/test_audioParamChaining.html:39
(Diff revision 2)
> +
> +// We chain three uutomation methods, making a gain step.
> +var rv = gain.gain.setValueAtTime(0, frameToTime(0, RATE))
> +                  .setValueAtTime(0.5, frameToTime(22000, RATE))
> +                  .setValueAtTime(1, frameToTime(44000, RATE));
> +

Could you test all the methods you change. At least that foobar.method(...) == foo;
Attachment #8742395 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/7df73cfe3c2f
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.