Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 210973002: Supporting a playbackTime attribute on AudioprocessingEvent. (Closed)

Created:
6 years, 6 months ago by KhNo
Modified:
5 years, 11 months ago
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Supporting a playbackTime attribute on AudioprocessingEvent. According to specification, onaudioprocess() evenhandler should delever playbackTime attribute to JS. The time when the audio will be played in the same time coordinate system as AudioContext.currentTime. playbackTime allows for very tight synchronization between processing directly in JavaScript with the other events in the context's rendering graph. BUG=332782 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170505

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : Fix some naming of valiable on test case. #

Total comments: 3

Patch Set 4 : Fix typo and casting #

Patch Set 5 : Rebase #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -8 lines) Patch
M LayoutTests/webaudio/scriptprocessornode.html View 1 2 3 4 3 chunks +11 lines, -1 line 1 comment Download
M Source/modules/webaudio/AudioContext.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/webaudio/AudioProcessingEvent.h View 1 2 3 4 2 chunks +4 lines, -2 lines 0 comments Download
M Source/modules/webaudio/AudioProcessingEvent.cpp View 1 2 3 4 2 chunks +4 lines, -3 lines 0 comments Download
M Source/modules/webaudio/AudioProcessingEvent.idl View 1 chunk +1 line, -0 lines 0 comments Download
M Source/modules/webaudio/ScriptProcessorNode.cpp View 1 2 3 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 25 (0 generated)
KhNo
Raymond, Could you review this patch?
6 years, 6 months ago (2014-03-25 17:55:00 UTC) #1
KhNo
rtoy@chromium.org: Please review changes in
6 years, 6 months ago (2014-03-25 17:56:19 UTC) #2
Raymond Toy
On 2014/03/25 17:56:19, KhNo wrote: > mailto:rtoy@chromium.org: Please review changes in In general, this looks ...
6 years, 6 months ago (2014-03-26 17:57:36 UTC) #3
KhNo
On 2014/03/26 17:57:36, Raymond Toy wrote: > On 2014/03/25 17:56:19, KhNo wrote: > > mailto:rtoy@chromium.org: ...
6 years, 6 months ago (2014-03-27 00:47:40 UTC) #4
Raymond Toy
On 2014/03/27 00:47:40, KhNo wrote: > On 2014/03/26 17:57:36, Raymond Toy wrote: > > On ...
6 years, 6 months ago (2014-03-27 18:45:47 UTC) #5
Raymond Toy
https://codereview.chromium.org/210973002/diff/20001/LayoutTests/webaudio/scriptprocessornode.html File LayoutTests/webaudio/scriptprocessornode.html (right): https://codereview.chromium.org/210973002/diff/20001/LayoutTests/webaudio/scriptprocessornode.html#newcode46 LayoutTests/webaudio/scriptprocessornode.html:46: shouldBeCloseTo("playbackTime", expectedTime, precision, true); Is there a reason playbackTime ...
6 years, 6 months ago (2014-03-27 18:52:20 UTC) #6
KhNo
> Ok, I talked this over with Chris. We didn't discuss how users can use ...
6 years, 6 months ago (2014-03-28 03:02:06 UTC) #7
KhNo
@Raymond. PTAL and Please refer my comment. https://codereview.chromium.org/210973002/diff/20001/LayoutTests/webaudio/scriptprocessornode.html File LayoutTests/webaudio/scriptprocessornode.html (right): https://codereview.chromium.org/210973002/diff/20001/LayoutTests/webaudio/scriptprocessornode.html#newcode46 LayoutTests/webaudio/scriptprocessornode.html:46: shouldBeCloseTo("playbackTime", expectedTime, ...
6 years, 6 months ago (2014-03-28 04:41:37 UTC) #8
Raymond Toy
On 2014/03/28 03:02:06, KhNo wrote: > > Ok, I talked this over with Chris. We ...
6 years, 5 months ago (2014-03-28 18:01:42 UTC) #9
Raymond Toy
https://codereview.chromium.org/210973002/diff/130001/LayoutTests/webaudio/scriptprocessornode.html File LayoutTests/webaudio/scriptprocessornode.html (right): https://codereview.chromium.org/210973002/diff/130001/LayoutTests/webaudio/scriptprocessornode.html#newcode48 LayoutTests/webaudio/scriptprocessornode.html:48: // There may be a littl time gap which ...
6 years, 5 months ago (2014-03-28 18:14:13 UTC) #10
KhNo
PTAL :) Thank you. https://codereview.chromium.org/210973002/diff/130001/Source/modules/webaudio/ScriptProcessorNode.cpp File Source/modules/webaudio/ScriptProcessorNode.cpp (right): https://codereview.chromium.org/210973002/diff/130001/Source/modules/webaudio/ScriptProcessorNode.cpp#newcode270 Source/modules/webaudio/ScriptProcessorNode.cpp:270: double playbackTime = (context()->currentSampleFrame() + ...
6 years, 5 months ago (2014-03-29 00:24:46 UTC) #11
Raymond Toy
On 2014/03/29 00:24:46, KhNo wrote: > PTAL :) Thank you. > > https://codereview.chromium.org/210973002/diff/130001/Source/modules/webaudio/ScriptProcessorNode.cpp > File ...
6 years, 5 months ago (2014-03-31 16:40:53 UTC) #12
KhNo
The CQ bit was checked by keonho07.kim@samsung.com
6 years, 5 months ago (2014-03-31 17:15:04 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keonho07.kim@samsung.com/210973002/190001
6 years, 5 months ago (2014-03-31 17:15:11 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-03-31 18:27:14 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_rel
6 years, 5 months ago (2014-03-31 18:27:15 UTC) #16
KhNo
The CQ bit was checked by keonho07.kim@samsung.com
6 years, 5 months ago (2014-04-01 00:17:09 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keonho07.kim@samsung.com/210973002/190001
6 years, 5 months ago (2014-04-01 00:18:04 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-04-01 00:29:35 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_rel
6 years, 5 months ago (2014-04-01 00:29:38 UTC) #20
KhNo
The CQ bit was checked by keonho07.kim@samsung.com
6 years, 5 months ago (2014-04-01 02:13:57 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keonho07.kim@samsung.com/210973002/190001
6 years, 5 months ago (2014-04-01 02:14:04 UTC) #22
commit-bot: I haz the power
Change committed as 170505
6 years, 5 months ago (2014-04-01 02:58:50 UTC) #23
mickelsen.eric
I think there may be an additional test needed here. Not sure if it's possible. ...
5 years, 11 months ago (2014-10-07 02:05:41 UTC) #24
Raymond Toy
5 years, 11 months ago (2014-10-07 03:47:33 UTC) #25
Message was sent while issue was closed.
On 2014/10/07 02:05:41, mickelsen.eric wrote:
> I think there may be an additional test needed here.  Not sure if it's
possible.
>  (Sorry, I'm just starting to familiarize myself with the implementation of
> webaudio.)
> 
>
https://codereview.chromium.org/210973002/diff/190001/LayoutTests/webaudio/sc...
> File LayoutTests/webaudio/scriptprocessornode.html (right):
> 
>
https://codereview.chromium.org/210973002/diff/190001/LayoutTests/webaudio/sc...
> LayoutTests/webaudio/scriptprocessornode.html:51:
> shouldBeCloseTo("playbackTime", expectedTime, allowedTimeGap, true);
> I'm not sure how to track this down, but I have a strong suspicion that
> different audio processing event handlers in the graph are receiving different
> playbackTime values - closer to something like what time it is now, rather
than
> what the spec. requires, which is what time the audio will play, which causes
> desynchronization.  I suspect this affects AudioContext and not
> OfflineAudioContext.  Any idea how this might be testable?

Rather than discuss this here on this closed CL, please open a new bug at
crbug.com/new. Be sure to cc me, and add the label cr-blink-webaudio.

Thanks!

Powered by Google App Engine
This is Rietveld 408576698