New issue
Advanced search Search tips

Issue 645776 link

Starred by 30 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 622082



Sign in to add a comment

Immediate scheduling of AudioParam using setValueAtTime occasionally does not work.

Reported by yotamm...@gmail.com, Sep 10 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.101 Safari/537.36

Example URL:
https://jsfiddle.net/yotammann/8jrudcs8/9/

Steps to reproduce the problem:
1. Load the url
2. Occasionally setValueAtTime does not change the AudioParam and the value stays the same
3. If 'error' is not shown in the first 15 seconds, try reloading the page.

What is the expected behavior?
The frequency AudioParam should alternate between 440 and 220 at the rate of the setInterval. Occasionally it is set, but the change does not take affect and the previously scheduled value is heard instead.

What went wrong?
Every once and a while after the frequency AudioParam is set using setValueAtTime, it remains at the previously scheduled value. This is something that you can hear when it happens, but i also log these moments in the DOM when the .value attribute is not what it is expected to be. 

Did this work before? Yes Chrome 52

Is it a problem with Flash or HTML5? HTML5

Does this work in other browsers? Yes 

Chrome version: 53.0.2785.101  Channel: stable
OS Version: OS X 10.10.5
Flash Version: Shockwave Flash 22.0 r0

Here is another example using a button. The note should change every time the button is pressed, but occasionally it stays on the same note for multiple presses.  

http://jsbin.com/yefilahapu/edit?js,output

This works in Chrome 52, Safari 9.1.3 and FF 48 as expected.

NOTE: FF does not accurately report scheduled changes with the .value attribute of the AudioParam, but you can still hear that the value is being changed. 

I also noticed that I can no longer schedule a change with a time value of '0', which used to immediately change the scheduled value and still has the expected behavior in FF and Safari. Now there is no change when 0 is passed into the second argument of setValueAtTime.
 

Comment 1 by yotamm...@gmail.com, Sep 10 2016

Here's another jsfiddle that illustrates how passing in a value of 0 as the time in the second argument of setValueAtTime has no affect.

https://jsfiddle.net/yotammann/1dLcv0yL/1/

According to the specification (https://webaudio.github.io/web-audio-api/#AudioParam): "If one of these events is added at a time where there is already an event of the exact same type, then the new event will replace the old one"

So the scheduled value should be overwritten as it is in FF and Safari and Chrome 52.

Perhaps this is related to how the above issue where setValueAtTime occasionally doesn't change the value.

Thanks for looking into this. This issue greatly affects many of the my Web Audio projects.

Comment 2 by yotamm...@gmail.com, Sep 11 2016

Here's an interesting discovery. If I schedule the event exactly 128 sample (one block) in the future, this error happens much more often:

https://jsfiddle.net/yotammann/8jrudcs8/10/

Comment 3 by rtoy@chromium.org, Sep 11 2016

Components: -Internals>Media Blink>WebAudio

Comment 4 by rtoy@chromium.org, Sep 12 2016

Status: Available (was: Unconfirmed)
Ok, for https://jsfiddle.net/yotammann/1dLcv0yL/1/, you are calling setValueAtTime with a time of 0.  We haven't yet implemented the (fairly recent) proposal that times in the past are clamped to currentTime.

Comment 5 by rtoy@chromium.org, Sep 12 2016

I have a tentative fix for the issues shown in http://jsbin.com/yefilahapu/edit?js,output and https://jsfiddle.net/yotammann/1dLcv0yL/1/.

Much more testing and think required; this was a hack to clamp the time to currentTime.  And https://jsfiddle.net/yotammann/8jrudcs8/9/ still produces errors; don't know what's up with that.

Comment 6 by rtoy@chromium.org, Sep 13 2016

Ok, I captured some timing data for https://jsfiddle.net/yotammann/8jrudcs8/9/. One main issue is that since the value getter is running on the main thread and is grabbing the value at "random" times independent of the audio thread that is handling the setValueAtTime automations, you will get flakiness. (I have not yet tested on Chrome 52).

Here are some output from my test build. I've annotated the output with // comments:

// called setValueAtTime(220, 0.522...) at currentTime=0.522.
setValueAtTime: 0.522448979591837 220 ctime 0.522448979591837

// Audio thread is now processing the timeline at time 0.5224489
firstEventTime 0: 0.522448979591837
Processing 0: etime = 0.522448979591837 current 0.522448979591837

// Another call to setValue to 440 at time 0.5340589
setValueAtTime: 0.5340589569161 440 ctime 0.5340589569161

// Reading the value back returns 220.  Because the timeline hasn't been run
// to update the value to 440. (Ignore the ERROR stuff. I used LOG(ERROR)
// to print this out.)
[1:1:0913/103833:ERROR:AudioParam.cpp(372)] getValue "Oscillator.frequency": 0.5340589569161: 220

[1:1:0913/103833:ERROR:AudioParam.cpp(372)] getValue "Oscillator.frequency": 0.5340589569161: 220

// Audiothread runs the timeline
firstEventTime 0: 0.5340589569161
Processing 0: etime = 0.5340589569161 current 0.5340589569161
// Value is now read back as 440, as expected from the setValue
[1:1:0913/103833:ERROR:AudioParam.cpp(372)] getValue "Oscillator.frequency": 0.557278911564626: 440

[1:1:0913/103833:ERROR:AudioParam.cpp(372)] getValue "Oscillator.frequency": 0.557278911564626: 440

// setValue to 220 at time 0.580498
setValueAtTime: 0.580498866213152 220 ctime 0.580498866213152

// Audio thread runs the timeline.
firstEventTime 0: 0.580498866213152
Processing 0: etime = 0.580498866213152 current 0.580498866213152

// Hmm. Not sure what happened here.  Possibly grabbed the data before the
// the audio thread has finished rendering the data with the new value?
[1:1:0913/103834:ERROR:AudioParam.cpp(372)] getValue "Oscillator.frequency": 0.580498866213152: 440

// But by the next call, time has progressed, and we have the new value.
[1:1:0913/103834:ERROR:AudioParam.cpp(372)] getValue "Oscillator.frequency": 0.592108843537415: 220

So, I think this test is inherently flaky.

I do not know why Chrome 52 was apparently better on this test.

Comment 7 by yotamm...@gmail.com, Sep 13 2016

Thanks for looking into this. The inconsistency with the .value getter is a much lower priority, but you can hear when the value is not being scheduled correctly (though it definitely does make it harder to write a validation test for). 

Re: comment #4, I just tested on Chrome 52, and values in the past used to clamp to the currentTime. This https://jsfiddle.net/L9eyzj5w/5/ works on 52, but not on 53. Scheduling events 1 sample in the past.

Maybe, those moments of inconsistent failure are when the main thread gets a little backed up and by the time it processes the event the time argument has passed in the audio thread. and since it no longer clamps to the currentTime it is just dropped instead. Does that seem correct? 

Seems like a temporary fix for that is to make events in the past just clamp to the current time instead of being dropped.

Comment 8 by rtoy@chromium.org, Sep 13 2016

Oh, I think 52 had lots of little issues with scheduling, possibly incorrectly start automations one sample before or after the actual time.  Seems small, but I think lots of these are audible.  We cleaned up the timeline scheduling and processing quite a bit over the last year.

I'm currently working on cleaning up my hack to clamp to current time, as required by the spec.

Comment 9 by rtoy@chromium.org, Sep 13 2016

Blockedon: 622082
I'm not sure if I have the same issue or a different one, but I'm having problems with setValueAtTime consistently being ignored, even with a future value. I haven't been able to produce an isolated case yet, but the code I have is doing basic beatmatching and works like a dream in Chromium <= 52.

// Logging to prove that we are changing the playback rate, and that the second argument of setValueAtTime is indeed in the future
console.log(bpm / newTempoLoop.bpm, nextPeriod(), context.currentTime);
source.playbackRate.setValueAtTime(bpm / newTempoLoop.bpm, nextPeriod()); // doesn't happen

Log showing around 2 seconds lead time:
1.0490215856364737 248.2428114162752 246.775873015873

I have a gig in a couple of weeks with 3000 guests. Halp!? :)
In my case at least (comment 10), I don't think 647974 is related unless they share a cause. I had currently playing sounds with missed playbackRate.setValueAtTime(someValue, sometimeInTheFuture) calls.

I've been able to force Chrome down to version 51 and stay put in the meantime.

Comment 13 by rtoy@chromium.org, Oct 7 2016

Cc: kkaluri@chromium.org rtoy@chromium.org
 Issue 653744  has been merged into this issue.
Similar to https://jsfiddle.net/yotammann/8jrudcs8/10/ - I'm also seeing ignored timeline values especially when using: 

currentTime + 128 / sampleRate

Can there be a bug related to placing events at render quantum boundaries?

My workaround is to "misalign" the time values by a few samples to avoid the boundaries, .e.g.

currentTime + 138 / sampleRate

I'm also able to reproduce an issue with setValueAtTime events being ignored sometimes if they land at the first frame of a buffer. I'm not sure if this is the same problem the original poster but it looks like the same problem as ro...@mindsocket.com.au and Bjorn. I'm attaching a small .js file that reproduces the issue on both Windows and OSX for me here.

The problem appears to be in AudioParamTimeline::valuesForFrameRangeImpl(), if the event should occur on the first frame of a buffer then there is a chance that it will be missed because the event will be discarded before being processed. Here is some debug output that may help explain:

[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(447)] valuesForFrameRangeImpl(388608, 388736)
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(471)] Checking lastEventTime=8.81486 < currentTime=8.81197
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(527)] currentFrame/sampleRate=8.81197, endFrame/sampleRate=8.81488
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(528)] event: time=0, value=0
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(530)] nextEvent: time=8.81486, value=1
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(757)] SetValue: currentFrame=388608, fillToEndFrame=388736, value=0
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(1069)] returning value: 0

We have an event that should fire at 8.81486 which corresponds to the endFrame of this call (endFrame's time is 8.81488), and thus the event should be serviced as the very first frame of the next valuesForFrameRangeImpl() call which happens here:

[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(447)] valuesForFrameRangeImpl(388736, 388864)
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(471)] Checking lastEventTime=8.81486 < currentTime=8.81488
[68017:27951:1102/113846:ERROR:AudioParamTimeline.cpp(488)] return defaultValue=0

But here there is a check to clear out old events that compares the startFrame's time to lastEventTime and since lastEventTime is earlier the events list is cleared. Just changing this line:

    if (lastEventTime < currentTime && lastEventType != ParamEvent::SetTarget) {
 
to this:

    if (lastEventTime < (currentTime - 1/sampleRate) && lastEventType != ParamEvent::SetTarget) {

seems to resolve the issue on my end by accounting for a precision difference that's less than 1/sampleRate but I'm not sure if this would be the correct fix.
AudioParam-missed-event.js
883 bytes View Download

Comment 16 by rtoy@chromium.org, Nov 2 2016

Thanks for another test case. I need to look at this some more, but I think the fundamental issue is that you're grabbing the currentTime in the main thread, but the actual currentTime in the audio thread may have already been incremented.  So when the timeline is being processed, the time of the event is already in the past.

When  issue 622082  is fixed, clamping the time to the currentTime in the audio thread, this issue should go away.
I'm not sure that's applicable in my case, though I'd be happy to wait and see what happens with  issue 622082 . I was calling setValueAtTime with a context time several seconds ahead of the expected event (see comment 10). 

Thanks for looking into it, either way :)

Comment 18 by rtoy@chromium.org, Nov 2 2016

roger@: Can you provide a simple repro case?  If you're scheduling things in advance, it's definitely not this particular issue, and it should be working. If it's not, we really want to fix it.

Feel free to file a new issue, and be sure to set the component to Blink>WebAudio so we see it right away.
Sorry for the confusion, I should have been clearer in my repro script (comment 15). The fact that I'm using one setValueAtTime call with currentTime is misleading as this can be a future value and the problem still occurs (for example currentTime + 3). It sounds like this is separate from the original issue as you suggest. I believe that what I'm seeing could be related to Roger's issue as it applies to future events only (in my case at currentTime + ~5 secs). I can file a separate issue tomorrow around this and modify the .js repro to make it clear that all events are future ones. Thanks!

Comment 20 by rtoy@chromium.org, Nov 2 2016

andrew@: I can reproduce the issue using Chrome 55 (beta).  But this doesn't happen with Chromium ToT or Chrome 56 (Dev). I think this particular issue is fixed. (Don't know what fixed it though, but there have been a few fixes recently for the AudioParam stuff.)
rtoy@: Thanks for checking it out. The build I had been using was from Monday but I pulled and rebuilt this morning (56.0.2909.0) and still see the same behavior on OSX Sierra here. I also tried a Canary build on Windows 10 (56.0.2907.0) and see the behavior there as well.

The 'extra' value in the repro script can also be increased or decreased by multiples of exactly one buffer length in seconds (128/44100) and the problem still occurs but goes away if any other length is used (for example 127/44100 or 129/44100).

Let me know what you think, it's certainly possible that there's something funny going on with my setup on this end though. Thanks!

Comment 22 by rtoy@chromium.org, Nov 16 2016

For the record, with  issue 622082  fixed, http://jsbin.com/yefilahapu/edit?js,output and https://jsfiddle.net/yotammann/1dLcv0yL/1/ produce the desired output now.  That is, the former plays different tones for every button press and the latter plays different frequencies.

The original URL, https://jsfiddle.net/yotammann/8jrudcs8/9/, still prints error, but that is expected per https://bugs.chromium.org/p/chromium/issues/detail?id=645776#c6.

Still need to test https://bugs.chromium.org/p/chromium/issues/detail?id=645776#c15

Comment 23 by rtoy@chromium.org, Nov 16 2016

andrew.macpherson@  Ran your test case with Chrome Dev.  The console prints out "Everything is ok"  Does that mean the test is ok?  (But Chrome 55 and 54 also prints out ok.)
rtoy@: Yes that means the problem is not occurring, interesting that it's passing on your end. I just retried it here on OSX and Windows with Chrome 54 and 56 and am still seeing it so I'm not sure what the difference may be.

I created a fiddle here which predicts when an event will fail to occur, on my end it's creating a failing event then a successful one in a loop with different times:

https://jsfiddle.net/juxen5vw/
It seems to be okay in Chrome 57.0.2987.37 beta (64-bit) on Windows 7 for me. The above jsfiddle code says passed for every line.
Great! I can confirm that the jsfiddle from #24 is now passing for me as well in latest Canary 58.0.3007.0 (and still failing in latest stable 56.0.2924.87).

From what I had seen earlier I think this patch may have been the one that fixed it: https://codereview.chromium.org/2567183002

Comment 27 by rtoy@chromium.org, Feb 13 2017

Status: Fixed (was: Available)
Based on comments 24, 25, and 26, I'm considered this issue is fixed.  

If not please re-open or file a new issue.
Updating from Chrome 56 to 57 fixed the issue for me.

Comment 29 by rtoy@chromium.org, Mar 23 2017

Fantastic!

Sign in to add a comment