New issue
Advanced search Search tips

Issue 717723 link

Starred by 5 users

Issue metadata

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



Sign in to add a comment

decodeAudioData callbacks not getting called

Reported by fin...@gmail.com, May 2 2017

Issue description

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

Steps to reproduce the problem:
1.  load audio file(mp3/ogg) via XHR
2.  use audioContext.decodeAudioData to load buffer

What is the expected behavior?
the callback of decodeAudioData could be called , and there is a buffer is passed into the callback

What went wrong?
the callback isn't called sometimes.

Did this work before? Yes Chrome 57.x

Does this work in other browsers? Yes

Chrome version: 60.0.3080.5  Channel: dev
OS Version: OS X 10.12.4
Flash Version: Shockwave Flash 26.0 r0
 

Comment 1 Deleted

Comment 2 by fin...@gmail.com, May 2 2017

In my case , I load many audio files when app start. 
I load them one by one.

When loaded a audio ,  it will load the next one in decodeAudioData's callback.

But sometimes , the workflow can't work . it will stop at someone decodeAudioData 

Comment 3 by rtoy@chromium.org, May 3 2017

Status: Available (was: Unconfirmed)
This was broken when the AudioBufferCallback was removed since doesn't match the spec.  It seems that the callback function might be getting GCed too soon.

Comment 4 by rtoy@chromium.org, May 4 2017

Summary: decodeAudioData callbacks not getting called (was: WebAudio can work correctly sometimes.)
Update summary to reflect actual issue

Comment 5 by rtoy@chromium.org, May 5 2017

Owner: rtoy@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, May 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5786a2c36b624710f558a12644adeaa93e25dd68

commit 5786a2c36b624710f558a12644adeaa93e25dd68
Author: rtoy <rtoy@chromium.org>
Date: Fri May 05 19:45:11 2017

Revert removal of AudioBufferCallback

Manually revert https://codereview.chromium.org/2820113006/
That CL causes decodeAudioData callbacks not to be called sometimes.
Apparently, it might be a GC problem where the callbacks are collected
before decodeAudioData calls them.

BUG= 717723 ,  707338 
TEST=manually run box2d test and verify convolver.buffer exists

Review-Url: https://codereview.chromium.org/2866503002
Cr-Commit-Position: refs/heads/master@{#469759}

[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/bindings/modules/v8/generated.gni
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/AsyncAudioDecoder.cpp
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/AsyncAudioDecoder.h
[add] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/AudioBufferCallback.h
[add] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/AudioBufferCallback.idl
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/BUILD.gn
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h
[modify] https://crrev.com/5786a2c36b624710f558a12644adeaa93e25dd68/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.idl

Comment 7 by rtoy@chromium.org, May 8 2017

Status: Fixed (was: Started)
Offending change reverted; previous behavior should be restored

Comment 8 by fin...@gmail.com, May 12 2017

Today , I update the last Chrome dev ,  60.0.3095.5 dev (64 bit) is still with this bug.

Which version will fix ?

Thanks.

Comment 9 by phistuck@gmail.com, May 12 2017

Should be fixed in -
MAJOR=60
MINOR=0
BUILD=3091
PATCH=0


So I am not sure why you are still seeing this issue.

Comment 10 by rtoy@chromium.org, May 12 2017

Labels: Needs-Feedback
Status: Assigned (was: Fixed)
The CL in c#6 reverted everything back to the way it was.

I tested this myself with box2djs () and that works now but wasn't working before.

Can you please provide a test case?

Comment 11 by fin...@gmail.com, May 13 2017

Hi , Now everything is OK . It's my fault. sorry.

Comment 12 by rtoy@chromium.org, May 15 2017

Status: Verified (was: Assigned)
Thanks for letting us now that things are working once again.  And sorry for the trouble!

Comment 13 by rtoy@chromium.org, Jun 1 2017

Status: Started (was: Verified)
Reopening because the CL was reverted.
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79281d8e311b2e55c57610f147640f4ee7e2fe17

commit 79281d8e311b2e55c57610f147640f4ee7e2fe17
Author: Raymond Toy <rtoy@chromium.org>
Date: Wed Jun 07 01:01:00 2017

Remove AudioBufferCallback

According to the WebAudio spec, the AudioBufferCallback interface
should not exist.  Instead we should have a DecodeSuccessCallback and
DecodeErrorCallback functions.

However, we need to keep a reference to the callbacks so that they
don't get GC'ed before decodeAudioData has a chance to call them.
These references are placed on a HeapVector when decodeAudioData is
called and removed after the callback is called when decodeAudioData
is finished.

BUG= 717723 
TEST=none

Change-Id: Ib71a21c0a95181d0ff35d266768605efe07a7279
Reviewed-on: https://chromium-review.googlesource.com/521805
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477497}
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/bindings/modules/v8/generated.gni
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/AsyncAudioDecoder.cpp
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/AsyncAudioDecoder.h
[delete] https://crrev.com/f3aa784ba3d16323a3b6cfe88f987fa141aa1599/third_party/WebKit/Source/modules/webaudio/AudioBufferCallback.h
[delete] https://crrev.com/f3aa784ba3d16323a3b6cfe88f987fa141aa1599/third_party/WebKit/Source/modules/webaudio/AudioBufferCallback.idl
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/BUILD.gn
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h
[modify] https://crrev.com/79281d8e311b2e55c57610f147640f4ee7e2fe17/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.idl

Comment 15 by rtoy@chromium.org, Jun 7 2017

finscn@ I'd really appreciate it if you could try out a canary build. The issues with decodeAudioData not calling the callbacks should be fixed.

Comment 16 by rtoy@chromium.org, Jun 27 2017

Status: Fixed (was: Started)
Haven't heard of any more issues, so closing as Fixed.

Please reopen or file a new issue if this comes up again.

Comment 17 by psu...@gmail.com, Aug 17 2017

Found this bug again in version "Version 61.0.3163.49 (Official Build) beta (64-bit)".

But everything seems fixed and working on latest Canary build Version 62.0.3188.0 (Official Build) canary (64-bit).

Will Stable version 61 contains that fix?

Comment 18 by rtoy@chromium.org, Aug 17 2017

Hmm. Chrome 61.0.3163.49 corresponds to build position 488528.  The CL in c#14 says it landed at position 477497 which was Chrome 61.0.3123.0.  Thus your build should have the fix.

Please provide a repro link or test case and we'll take a look.

Comment 19 Deleted

Comment 20 by psu...@gmail.com, Aug 18 2017

I just created repro demo for this issue: https://psulek.github.io/pub/chromebug-717723/index.html

Please retry more times to hit this isses (sometimes takes to refresh 10-15 times or open dev tools to repro issue).
When error occurs, loading stops before 100%, eg:

Preloader -> Audio resource "sounds/8.mp3" [sound] loaded (loaded 8/13)
69.23076923076923% (8/13)

Sample code uses soundjs lib (http://www.createjs.com/soundjs), which internally using AudioContext. 
When you debug soundjs-0.6.2.combined.js, it stops on code:

   Loader.context.decodeAudioData(this._rawResult,
	         createjs.proxy(this._handleAudioDecoded, this),
	         createjs.proxy(this._sendError, this));

Those 2 proxies are callbacks for DecodeSuccessCallback and DecodeErrorCallback. When everything works, those callbacks are called for all audio files. 
When error occurs, it starts calling Loader.context.decodeAudioData but does not fire any of above callbacks and loading of audio files are broken.

I am able to repro this issue in latest BETA ("Version 61.0.3163.49 (Official Build) beta (64-bit)").
But on Canary build (Version 62.0.3188.0 (Official Build) canary (64-bit)), i cant repro this issue.

Comment 21 by rtoy@chromium.org, Aug 18 2017

Labels: -Needs-Feedback
Status: Available (was: Fixed)
Reopening bug.

Thanks for the repro case. I'll take a look shortly.

Comment 22 by rtoy@chromium.org, Aug 18 2017

I can reproduce this with 61.0.3163.49.  Files stop loading.  Can't reproduce this in Chromium ToT 62.0.3190.0 (today).

I don't recall anything changing with decodeAudioData between these versions.  It will take some time to figure this out.

Comment 23 by di_oo...@yahoo.com, Aug 21 2017

I think I'm seeing this same issue in my app, so I'm sharing a link in case it's useful for verification. The problem is reproducible 100% of the time for me in 61.0.3163.49 but has not yet occurred in the current canary build (62.0.3192.0). My app is created in Dart and compiled to js with dart2js. It loads a soundfont .sf3 file which contains many individual ogg audio samples that are decoded via decodeAudioData in succession. It fails at different points each time, as you can see by watching the console where the sample names are listed as they are passed in for decoding. Note that it takes a 5-10 seconds to load the 12MB sf3 file before starting to decode. Upon successful completion, five piano notes sound. http://community-band.com/experiments/sf3test/

Comment 24 by rtoy@chromium.org, Aug 21 2017

Thanks for the repro test. It does seem to be 100% reliable.

A bisect-builds run shows that the offending CL is
https://chromium.googlesource.com/chromium/src/+log/419624707ee5057f5a02895434d03ef72892e919..6b7030d491f6a37fda078a059f71b9e55b0c3528

This points to the CL in c#14 where AudioBufferCallback was removed (again).

Comment 25 by psu...@gmail.com, Aug 21 2017

Question is will be stable v61 affected with this bug? Because it's pretty much breaker for web apps depending on correct audio functionality. Please confirm if it will be fixed for stable 61 released in early September. Thanks. 

Comment 26 by rtoy@chromium.org, Aug 21 2017

Another bisect-build says that everything started working again in 62:

https://chromium.googlesource.com/chromium/src/+log/c00100dc25d5ca4a2ca0da89cdb46547663897d3..cd39e5b0ea3c0da423e54066bd97571f82dffaad

I guess this has been broken since 61, and got fixed when the trace wrapper stuff was updated.

Comment 27 by rtoy@chromium.org, Aug 21 2017

The fix for this is being tracked in  issue 757563 .
Project Member

Comment 28 by bugdroid1@chromium.org, Aug 21 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59ff33d70ae25244291794b11c06afd69bc70b81

commit 59ff33d70ae25244291794b11c06afd69bc70b81
Author: Raymond Toy <rtoy@chromium.org>
Date: Mon Aug 21 22:43:25 2017

Apply TraceWrapper correctly for decudeAudioData callbacks.

emplace_back does not construct the object to include the
TraceWrapperMember.  Instead use push_back with explicit construction
of the TraceWrapperMember to wrap the callback pointers.

Manually tested this fix using the repro case from
https://bugs.chromium.org/p/chromium/issues/detail?id=717723#c23 and
verify that a tone is heard, as expected.

Bug:  757563 ,  717723 
Test: Run repro from 717723
Change-Id: If49501e0997c42f44e05f63323f3c42e6cee1913
Reviewed-on: https://chromium-review.googlesource.com/624620
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#738}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/59ff33d70ae25244291794b11c06afd69bc70b81/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp

Comment 29 by rtoy@chromium.org, Aug 22 2017

Status: Fixed (was: Available)
Fix landed on M61 branch.  

Comment 30 by psu...@gmail.com, Aug 22 2017

Thanks!
Labels: Needs-Feedback
@finsc: Could you please provide us the clear Repro steps to verify the issue form our end?

Thanks!!

Comment 32 Deleted

Sign in to add a comment