Issue metadata
Sign in to add a comment
|
decodeAudioData callbacks not getting called
Reported by
fin...@gmail.com,
May 2 2017
|
||||||||||||||||||||
Issue descriptionUserAgent: 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
,
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
,
May 3 2017
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.
,
May 4 2017
Update summary to reflect actual issue
,
May 5 2017
,
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
,
May 8 2017
Offending change reverted; previous behavior should be restored
,
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.
,
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.
,
May 12 2017
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?
,
May 13 2017
Hi , Now everything is OK . It's my fault. sorry.
,
May 15 2017
Thanks for letting us now that things are working once again. And sorry for the trouble!
,
Jun 1 2017
Reopening because the CL was reverted.
,
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
,
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.
,
Jun 27 2017
Haven't heard of any more issues, so closing as Fixed. Please reopen or file a new issue if this comes up again.
,
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?
,
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.
,
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.
,
Aug 18 2017
Reopening bug. Thanks for the repro case. I'll take a look shortly.
,
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.
,
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/
,
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).
,
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.
,
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.
,
Aug 21 2017
The fix for this is being tracked in issue 757563 .
,
Aug 21 2017
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
,
Aug 22 2017
Fix landed on M61 branch.
,
Aug 22 2017
Thanks!
,
Aug 23 2017
@finsc: Could you please provide us the clear Repro steps to verify the issue form our end? Thanks!! |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 Deleted