Closed Bug 893020 Opened 11 years ago Closed 5 years ago

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

Categories

(Core :: Web Audio, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: padenot, Assigned: padenot)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [blocking-webaudio-])

Attachments

(2 files, 1 obsolete file)

      No description provided.
Summary: Bug XXXXXX - Make AudioParam.value return a value based on the AudioContext.currentTime. r= → Make AudioParam.value return a value based on the AudioContext.currentTime
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.
Attachment #774698 - Flags: review?(ehsan)
Attachment #774689 - Attachment is obsolete: true
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.
Attachment #774698 - Flags: review?(ehsan) → review-
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
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
Whiteboard: [blocking-webaudio-]
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.
Priority: -- → P2
Depends on: 944851
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?
Flags: needinfo?(ehsan)
Yes, that sounds good.  We should also test to make sure that the intrinsic value is not changing until we yield.
Flags: needinfo?(ehsan)
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
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
We can close this, in light of bug 1171438.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
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
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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
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?
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
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...
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.
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
Summary: Make AudioParam.value return a value based on the AudioContext.currentTime → Make AudioParam.value getter return a value based on the AudioContext.currentTime
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.

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.

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

Blocks: 1563729
Pushed by padenot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/048ad3e2a011
Make AudioParam.value getter return a value based on the AudioContext.currentTime. r=karlt
Status: REOPENED → RESOLVED
Closed: 9 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Whiteboard: [blocking-webaudio-] → [blocking-webaudio-], [qa-69b-p2]
QA Whiteboard: [qa-69b-p2]
Whiteboard: [blocking-webaudio-], [qa-69b-p2] → [blocking-webaudio-]

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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: