893020 - Make AudioParam.value getter return a value based on the AudioContext.currentTime

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P3)

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug XXXXXX - Make AudioParam.value return a value based on the AudioContext.currentTime. r=

Comment 2

[Paul Adenot (:padenot)]

Attachment: Make AudioParam.value return a value based on the AudioContext.currentTime. r=

I removed the compiledtest, but added a mochitest for this.

There is another solution to do this, pass the context all the way down to the
AudioEventTimeline so we can get the context's currentTime timing, and return
value from the AudioEventTimeline. We would have to mock the AudioContext to
test this. More memory usage because we stuff a AudioContext pointer in each
timeline, but better testability since we can keep testing this with a
compiledtest.

Comment 3

[(no longer active)]

Comment on attachment 774698 [details] [diff] [review]
Make AudioParam.value return a value based on the AudioContext.currentTime. r=

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

Wait a second.  This violates the spec.  Why do you want to do this?

If WebKit and Blink do this, please file bugs on them.  They should fix their code.

Comment 4

[Paul Adenot (:padenot)]

Quoting the spec:

> An intrinsic parameter value will be calculated at each time, which is either the value set 
> directly to the .value attribute, or, if there are any scheduled parameter changes 
> (automation events) with times before or at this time, the value as calculated from these
> events.

Then,

>  When read, the .value attribute always returns the intrinsic value for the current time.

You even have a comment about this in the code [1].

[1]: http://mxr.mozilla.org/mozilla-central/source/content/media/AudioEventTimeline.h#181

Comment 5

[(no longer active)]

I'd really like to avoid having to implement AudioParam.value like this.

https://www.w3.org/Bugs/Public/show_bug.cgi?id=21545#c2

Comment 7

[hunyeti]

Will there be anything done about this, either in the code or in the documentation? I'm not sure it's a bug but on MDN this written about AudioParam.value: "AudioParam.value
Represents the parameter current floating-point value. " so after this it is expected from in to return the current value. The w3 spec really isn't very clear about this how it should be handled. I would much rather prefer a method like getValueAtTime.

Comment 9

[Karl Tomlinson (:karlt)]

Since the change in bug 944851, this would return the intrinsic parameter value rather than the computed value.  i.e. it won't include connections from node outputs, and so doesn't depend on anything in the graph except currentTime.

Gecko's implementation of currentTime, at least currently, doesn't advance until JS yields and so script reading value would get a consistent snapshot of the intrinsic value as proposed in http://lists.w3.org/Archives/Public/public-audio/2012OctDec/0623.html

Does that address your concerns, Ehsan?

Comment 10

[(no longer active)]

Yes, that sounds good.  We should also test to make sure that the intrinsic value is not changing until we yield.

Comment 11

[Karl Tomlinson (:karlt)]

If the WG continues with its plans to make the .value setter an alias for a transient effect, then it would be better to leave the getter returning the last value passed to the setter.  Some other readonly intrinsicValue property or getIntrinsicValueAtTime method could return transient effect if there is a need for this.

http://lists.w3.org/Archives/Public/public-audio/2013OctDec/0340.html

Comment 14

[Karl Tomlinson (:karlt)]

The WG has decided not to change the .value setter, so we can proceed here.
https://github.com/WebAudio/web-audio-api/issues/76#issuecomment-60685015

Comment 16

[Paul Adenot (:padenot)]

We can close this, in light of bug 1171438.

Comment 17

[Karl Tomlinson (:karlt)]

Although "The effect of setting this attribute is equivalent to calling
<code>setValueAtTime()</code> with the current <code>AudioContext</code>'s
<code>currentTime</code> and the requested value. Subsequent accesses to this
attribute's getter will return the same value." [1],
"When read, the value attribute always returns the intrinsic value for the
current time." [2]

That leads me to conclude that "return the same value" applies only until
currentTime changes past the next event.

[1] https://github.com/WebAudio/web-audio-api/commit/5070bcc5efe91ef5117005c20c7d189fb3708e11
[2] https://www.w3.org/TR/2015/WD-webaudio-20151208/#computation-of-value

Comment 20

[Lajos Mészáros]

Added a temporary workaround in JS for value computation in my repo:
https://github.com/meszaros-lajos-gyorgy/webkitAudioContext-patch/blob/master/src/value-fix.js

It is based on this repo:
https://github.com/mohayonao/pseudo-audio-param

Comment 22

[Griva]

I don't know current/final implementation of setter/setter for 'gain.value' but after using any methods of gain like linearRampToValueAtTime(), value property stops working - It's not possible to set value or get value and it returns last set value. 

In short: using any method removes possibility to get current volume completely.
How to get current value then?

Comment 23

[Paul Adenot (:padenot)]

This is not possible, and our behavior is correct per spec [0]. We know it's not great, but it's unlikely to change in the short term.

[0]: https://webaudio.github.io/web-audio-api/#widl-AudioParam-value

Comment 24

[Griva]

What is "short term" :) ?

Is there any possible hack to restore value getter or using any method deactivate value property permanently until application lifetime?
Only possible way is just store final value of automation method and possibly if needed - calculate manually approximately value for current time?
I must say that this bug rly break many things...

Comment 25

[Paul Adenot (:padenot)]

Short term is, say, not this year, and probably not next year. We are not changing any features in the Web Audio API until we have the current set of feature and behavior stabilized.

You can have a look at the solutions in comment 10 for possible workarounds.

Comment 26

[Paul Adenot (:padenot)]

I was wrong: https://github.com/WebAudio/web-audio-api/issues/1081#issuecomment-309840606

Comment 27

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 28

[Karl Tomlinson (:karlt)]

Summary of spec changes and why I do not think we should implement that.
https://github.com/WebAudio/web-audio-api/issues/1788#issuecomment-437185411

I still think returning the intrinsic value is reasonable.

Comment 30

[Paul Adenot (:padenot)]

This is now stable in the spec: the value to return is the value of the automation timeline, without any audio input contributing to the final value of the AudioParam. This is easy to implement in Gecko, since we have a copy of the timeline on the main thread.

Comment 31

[Paul Adenot (:padenot)]

Attachment: Bug 893020 - Make AudioParam.value getter return a value based on the AudioContext.currentTime. r?karlt

AudioWG resolution: https://github.com/WebAudio/web-audio-api/issues/1788#issuecomment-468345987

Comment 32

[Paul Adenot (:padenot)]

This patch passes the tests, which I find very surprising. I'm going to try to find why and/or write more tests.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/048ad3e2a011
Make AudioParam.value getter return a value based on the AudioContext.currentTime. r=karlt

Comment 34

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/048ad3e2a011

Comment 35

[Eric Shepherd [:sheppy]]

Documentation updates:

• Added Usage notes to the article on AudioParam.value. Moved into it the information about the single/double precision issue and added a section describing how the value is maintained over time given the ways it can be affected.
• Updated AudioParam main page to no longer claim that the value getter always returns the default or most recently explicitly set value, ignoring changes that have happened automatically over time. This is no longer true.
• Submitted BCD PR 4570 to add notes to AudioParam.value describing Firefox's changes there.
• Updated Firefox 69 for developers