Bug 17334 - (setTargetValueAtTime): AudioParam.setTargetValueAtTime is misleading and underdefined
(setTargetValueAtTime): AudioParam.setTargetValueAtTime is misleading and und...
Status: CLOSED FIXED
Product: AudioWG
Classification: Unclassified
Component: Web Audio API
unspecified
PC All
: P2 normal
: TBD
Assigned To: Chris Rogers
This bug has no owner yet - up for the taking
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-05 11:25 UTC by Philip Jägenstedt
Modified: 2012-10-16 13:56 UTC (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Jägenstedt 2012-06-05 11:25:34 UTC
Audio-ISSUE-38 (setTargetValueAtTime): AudioParam.setTargetValueAtTime is misleading and underdefined [Web Audio API]

http://www.w3.org/2011/audio/track/issues/38

Raised by: Philip Jägenstedt
On product: Web Audio API

The naming of this function is misleading, since it does not in fact set the target value at the given time.

timeConstant is underdefined, what is the exponent? A "first-order filter (exponential)" is mentioned, but not clarified further.

Finally, it is not well defined what the difference to exponentialRampToValueAtTime is, but we suspect that setTargetValueAtTime is something like v=1-e-kt. The equations involving the 3 parameters should be included in the spec, normatively.
Comment 1 Chris Rogers 2012-06-14 00:46:35 UTC
I disagree that the name is misleading.  It's a *target* value which we start approaching at precisely the time given.

* timeConstant is now defined.
* precise v(t) equation is now given:
https://dvcs.w3.org/hg/audio/rev/d86920562bf1
Comment 2 Olivier Thereaux 2012-06-28 13:51:51 UTC
Philip, does this answer your concerns? 

Unless you or someone else can suggest a convincingly better name for setTargetValueAtTime, I think we can close this.
Comment 3 Olivier Thereaux 2012-07-05 14:34:01 UTC
Mailing-list discussion on this topic between Ray Bellis, Chris Rogers, Marcus Geelnard and myself:
http://lists.w3.org/Archives/Public/public-audio/2012AprJun/thread.html#msg856

Marcus has a pretty good summary, and raises something interesting:
http://lists.w3.org/Archives/Public/public-audio/2012AprJun/0865.html

«
Once you understand what the function does, the name makes sense. However,  
the name alone does not make it easy to understand what it does.

I think that the confusion (at least for me) is that "setTargetValue" is  
very similar to "setValue", and I fear that many will have them mixed up.  
Generally speaking, the "set" term indicates a zero-duration operation.  
I'd much rather see that methods that cause gradual changes use a  
consistent naming convention, excluding the term "set".

I think that a more appropriate name could be "approachValueAtTime" or  
"startApproachingValueAtTime".

Similarly the name "setValueCurveAtTime" would do better without "set".  
Any suggestions? (I'm out of ideas right now)
»
Comment 4 Olivier Thereaux 2012-07-09 08:52:11 UTC
Rob Baxter suggested the following on the mailing-list:

AudioParam.set(value, time)
AudioParam.linearRamp(value, time)
AudioParam.exponentialRamp(value, time)
AudioParam.approach(target, time, k)
AudioParam.curve(value_set, time, duration)


The suggestion already received two +1's, from Marcus and me.
http://lists.w3.org/Archives/Public/public-audio/2012JulSep/0017.html
http://lists.w3.org/Archives/Public/public-audio/2012JulSep/0030.html
Comment 5 Jussi Kalliokoski 2012-07-09 17:42:51 UTC
(In reply to comment #4)
> Rob Baxter suggested the following on the mailing-list:
> 
> AudioParam.set(value, time)
> AudioParam.linearRamp(value, time)
> AudioParam.exponentialRamp(value, time)
> AudioParam.approach(target, time, k)
> AudioParam.curve(value_set, time, duration)
> 
> 
> The suggestion already received two +1's, from Marcus and me.
> http://lists.w3.org/Archives/Public/public-audio/2012JulSep/0017.html
> http://lists.w3.org/Archives/Public/public-audio/2012JulSep/0030.html

+1
Comment 6 heiskr 2012-07-09 17:50:27 UTC
Having just gone through the process of figuring out what those different things mean for something I'm working on (that image on the spec is awesome by the way), hooray for easier names :)
Comment 7 Olivier Thereaux 2012-08-23 10:01:37 UTC
We had a lengthy conversation about this during the group's teleconference on 22nd August 2012:
http://www.w3.org/2012/08/22-audio-minutes#item03

There was agreement that the name was imperfect, but there was a compromise to be reached between using a nice, short method name and one that describes the behaviour of the method well. We discussed whether to remove the "at time", whether to remove "set", and so on.  

In the end, we reached the following proposed resolution:

* Change the setTargetValueAtTime method to setTargetAtTime
* The rest of the methods are a decent enough compromise, will be kept as is

Related changes were also proposed, which would make the description and behaviour of AudioParam automation easier to understand and use:

* Use startTime / endTime parameter names for AudioParam automation methods
https://www.w3.org/Bugs/Public/show_bug.cgi?id=18661

* Setting audioparam value while there is an automation curve will cancel that automation curve and set value immediately
https://www.w3.org/Bugs/Public/show_bug.cgi?id=18662

* Need a method to get a readonly reading of the combined value when using AudioParam automation curve
https://www.w3.org/Bugs/Public/show_bug.cgi?id=18663
Comment 8 Chris Rogers 2012-09-28 22:30:13 UTC
Marcus was present in one of the teleconferences where we decided to change:
setTargetValueAtTime() -> setTargetAtTime()

The spec has now been updated.
Comment 9 Marcus Geelnard (Opera) 2012-10-02 09:51:53 UTC
(In reply to comment #8)
> The spec has now been updated.

I assume you're referring to this change: http://dvcs.w3.org/hg/audio/rev/aea45daa1200

Looks ok to me.
Comment 10 Olivier Thereaux 2012-10-16 13:56:54 UTC
No objection in 2 weeks, vetted: closing.