Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(setTargetValueAtTime): AudioParam.setTargetValueAtTime is misleading and underdefined #152

Closed
olivierthereaux opened this issue Sep 11, 2013 · 10 comments

Comments

Projects
None yet
1 participant
@olivierthereaux
Copy link
Contributor

commented Sep 11, 2013

Originally reported on W3C Bugzilla ISSUE-17334 Tue, 05 Jun 2012 11:25:34 GMT
Reported by Philip Jägenstedt
Assigned to

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.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Rogers on W3C Bugzilla. Thu, 14 Jun 2012 00:46:35 GMT

I disagree that the name is misleading. It's a target value which we start approaching at precisely the time given.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Olivier Thereaux on W3C Bugzilla. Thu, 28 Jun 2012 13:51:51 GMT

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.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Olivier Thereaux on W3C Bugzilla. Thu, 05 Jul 2012 14:34:01 GMT

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)
»

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Olivier Thereaux on W3C Bugzilla. Mon, 09 Jul 2012 08:52:11 GMT

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

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Jussi Kalliokoski on W3C Bugzilla. Mon, 09 Jul 2012 17:42:51 GMT

(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

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by on W3C Bugzilla. Mon, 09 Jul 2012 17:50:27 GMT

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 :)

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Olivier Thereaux on W3C Bugzilla. Thu, 23 Aug 2012 10:01:37 GMT

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:

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Rogers on W3C Bugzilla. Fri, 28 Sep 2012 22:30:13 GMT

Marcus was present in one of the teleconferences where we decided to change:
setTargetValueAtTime() -> setTargetAtTime()

The spec has now been updated.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Marcus Geelnard (Opera) on W3C Bugzilla. Tue, 02 Oct 2012 09:51:53 GMT

(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.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Olivier Thereaux on W3C Bugzilla. Tue, 16 Oct 2012 13:56:54 GMT

No objection in 2 weeks, vetted: closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.