New issue
Advanced search Search tips

Issue 707338 link

Starred by 2 users
Status: Fixed
Owner:
rtoy@chromium.org
Closed: Jul 2017
Cc:
foolip@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 674593


Show other hotlists

Hotlists containing this issue:
Non-Standard-IDL


Sign in to add a comment

Standardize or remove AudioBufferCallback

Project Member Reported by lunalu@chromium.org, Mar 31 2017 
I don't see a spec link in AudioBufferCallback.idl.
I checked with Gecko and WebKit, Gecko doesn't have a callback and WebKit's callback looks very different from ours. 
Should we be somehow standardizing this or considering to remove?

Comment 1 by hongchan@chromium.org, Mar 31 2017

Status: Available (was: Untriaged)
Yes, I am not sure why they exist. This callback is tied to decodeAudioData() method and must have been created to make "exposed" callback mechanism work.

They need to be removed.

Comment 2 by foolip@chromium.org, Apr 7 2017

Cc: foolip@chromium.org

Comment 3 by rtoy@chromium.org, Apr 18 2017

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

Comment 4 by bugdroid1@chromium.org, Apr 21 2017 

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

commit 91e558ce715fc5990700cfd003084b6c4a8ed3fa
Author: rtoy <rtoy@chromium.org>
Date: Fri Apr 21 16:39:52 2017

Remove AudioBufferCallback

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

BUG= 707338 
TEST=callback behavior covered by existing tests

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

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

Comment 5 by rtoy@chromium.org, Apr 24 2017

Status: Fixed (was: Started)

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

Status: Assigned (was: Fixed)
Reopening because this causes decodeAdioData callbacks not to be called sometimes.  Current guess is that the callbacks are getting GCed too soon.

We'll try landing this once we figure out what's causing the callbacks from getting called.
Project Member

Comment 7 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 8 by rtoy@chromium.org, Jul 19 2017

Status: Fixed (was: Assigned)
https://chromium-review.googlesource.com/c/521805/ removed AudioBufferCallback, but forgot to link to this bug.

AudioBufferCallback is gone.
►

Sign in to add a comment