Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Issue 2666063003: Continue to process AnalyserNode if inputs are silent. (Closed)

Created:
3 years, 10 months ago by Raymond Toy
Modified:
3 years, 10 months ago
Reviewers:
hongchan
CC:
chromium-reviews, blink-reviews, haraken, Raymond Toy, hongchan
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Continue to process AnalyserNode if inputs are silent. If the input to an AnalyserNode is not connected or it is silent (because of the silent hint), we must still process the node so that the internal state is updated with the silent input. This requires several things to be changed: 1. propagatesSilence() must return false so that AnalyserNode::process() is called even if the input is silent. 2. Can't disable output of an AnalyserNode when the input disconnects. If the output were disabled, the node would not get called to process any more. 3. If the input is disconnected and the output is not connected, we cannot remove the node from the automatic pull list because then process() stops getting called. Basically, process() must always get called as long as the AnalyserNode is alive. BUG=683188 TEST=Analyser/handle-silent-inputs.html Review-Url: https://codereview.chromium.org/2666063003 Cr-Commit-Position: refs/heads/master@{#449379} Committed: https://chromium.googlesource.com/chromium/src/+/c9ef493e1d238a655011a872886903a1d44d52bd

Patch Set 1 #

Patch Set 2 : Add test #

Total comments: 6

Patch Set 3 : Specialize updatePullStatus for AnalyserNodes #

Patch Set 4 : Adjust comment per review. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+201 lines, -7 lines) Patch
A third_party/WebKit/LayoutTests/webaudio/Analyser/handle-silent-inputs.html View 1 2 1 chunk +137 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/webaudio/AnalyserNode.h View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp View 1 2 3 2 chunks +39 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.h View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp View 1 2 1 chunk +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/webaudio/AudioNode.cpp View 1 chunk +6 lines, -1 line 0 comments Download

Messages

Total messages: 14 (6 generated)
Raymond Toy
PTAL
3 years, 10 months ago (2017-01-31 19:47:03 UTC) #3
hongchan
https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right): https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp#newcode58 third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update the internal state even if there ...
3 years, 10 months ago (2017-02-01 01:12:11 UTC) #4
Raymond Toy
https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp File third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp (right): https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp#newcode124 third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp:124: // right time and FFT data. On 2017/02/01 01:12:11, ...
3 years, 10 months ago (2017-02-01 21:21:02 UTC) #5
Raymond Toy
https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right): https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp#newcode58 third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update the internal state even if there ...
3 years, 10 months ago (2017-02-01 21:23:47 UTC) #6
hongchan
lgtm with nit https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right): https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp#newcode58 third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update the internal state ...
3 years, 10 months ago (2017-02-03 18:47:40 UTC) #7
Raymond Toy
https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right): https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp#newcode58 third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update the internal state even if there ...
3 years, 10 months ago (2017-02-03 18:50:00 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2666063003/50001
3 years, 10 months ago (2017-02-09 18:19:55 UTC) #11
commit-bot: I haz the power
3 years, 10 months ago (2017-02-09 20:00:05 UTC) #14
Message was sent while issue was closed.
Committed patchset #4 (id:50001) as
https://chromium.googlesource.com/chromium/src/+/c9ef493e1d238a655011a8728869...

Powered by Google App Engine
This is Rietveld 408576698