865231 - Implement AudioParam.setValueCurveAtTime

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[(no longer active)]

This is the only method in AudioParam which we currently don't support...

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Bug 866434 (connect AudioNode output to AudioParam) is really an AudioParam feature too, and is harder to implement than this.

Comment 2

[(no longer active)]

(In reply to comment #1)
> Bug 866434 (connect AudioNode output to AudioParam) is really an AudioParam
> feature too, and is harder to implement than this.

Right.  Note that these tracking bugs are mostly about changes to interfaces, which is why I charged bug 866434 to AudioNode.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 746065 [details] [diff] [review]
Patch (v1)

Review of attachment 746065 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/AudioEventTimeline.h
@@ +117,2 @@
>    };
> +  float* mCurve;

Add a comment identifying the sample rate used here

@@ +396,5 @@
> +  {
> +    double ratio = (t - startTime) / duration;
> +    if (ratio >= 1.0) {
> +      return aCurve[aCurveLength - 1];
> +    }

I think it would be a good idea to handle negative ratios here.

::: content/media/webaudio/compiledtest/TestAudioEventTimeline.cpp
@@ +9,5 @@
>  #include <sstream>
>  #include <limits>
>  
>  using namespace mozilla::dom;
> +using mozilla::ArrayLength;

using namespace mozilla;

Comment 5

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> Comment on attachment 746065 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 746065 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/AudioEventTimeline.h
> @@ +117,2 @@
> >    };
> > +  float* mCurve;
> 
> Add a comment identifying the sample rate used here

Hmm, there is no sample rate per se, we just intrapolate our position inside the curve buffer depending on how far along in time we are.  I'll add a comment to make that more clear.

> @@ +396,5 @@
> > +  {
> > +    double ratio = (t - startTime) / duration;
> > +    if (ratio >= 1.0) {
> > +      return aCurve[aCurveLength - 1];
> > +    }
> 
> I think it would be a good idea to handle negative ratios here.

The ratio can never be negative because the code in GetValueAtTime ensures that this is only called for ticks after mTime.  I'll add a MOZ_ASSERT to document that.

Comment 6

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50c905b6d44e

Comment 7

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/50c905b6d44e

Comment 8

[(no longer active)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.