Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2712933003:
PeriodicWave returns sine wave generator if no coefficients given (Closed)
|
Created:
3 years, 10 months ago by Raymond Toy Modified:
3 years, 9 months ago Reviewers:
hongchan CC:
chromium-reviews, blink-reviews, haraken, Raymond Toy, hongchan Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPeriodicWave returns sine wave generator if no coefficients given
If the PeriodWaveOptions option does not specify either real or imag
components, the returned PeriodicWave should result in a sine wave when
used with an OscillatorNode.
The spec was unclear and incorrect in saying that the optional
PeriodicWaveOptions dictionary had optional members specified, but actually
required at least one to be specified. The spec was updated
to make this clear that everything is correctly optional, and
specifies what PeriodicWave should do PeriodicWaveOptions is given.
Tests updated for this and an additional test to verify that the
output is a sine wave.
BUG=695513
TEST=constructor/periodicwave.html
Review-Url: https://codereview.chromium.org/2712933003
Cr-Commit-Position: refs/heads/master@{#457824}
Committed: https://chromium.googlesource.com/chromium/src/+/10c77a53e77e2ae475a00cbddb5da81f0f2965c4
Patch Set 1 #Patch Set 2 : Rebase #Patch Set 3 : Rebase #Patch Set 4 : Refactor and add test #
Total comments: 4
Patch Set 5 : Address review comments. #
Messages
Total messages: 16 (11 generated)
Description was changed from ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the return PeriodicWave should be a sine wave. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ==========
Description was changed from ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ==========
Description was changed from ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ==========
Description was changed from ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ==========
Description was changed from ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear an incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear and incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ==========
rtoy@chromium.org changed reviewers: + hongchan@chromium.org
PTAL
lgtm with nits https://codereview.chromium.org/2712933003/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webaudio/constructor/periodicwave.html (right): https://codereview.chromium.org/2712933003/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/constructor/periodicwave.html:320: }) A missing semicolon! https://codereview.chromium.org/2712933003/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/constructor/periodicwave.html:328: .then(taskDone); Is this indentation from clang-format? If so, clang-format must have caught the missing semicolon above.
https://codereview.chromium.org/2712933003/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webaudio/constructor/periodicwave.html (right): https://codereview.chromium.org/2712933003/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/constructor/periodicwave.html:320: }) On 2017/03/16 21:50:21, hongchan wrote: > A missing semicolon! Done. https://codereview.chromium.org/2712933003/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/constructor/periodicwave.html:328: .then(taskDone); On 2017/03/16 21:50:21, hongchan wrote: > Is this indentation from clang-format? If so, clang-format must have caught the > missing semicolon above. Yes, clang format. And rerunning after fixing the semicolon didn't change the result. I can't make heads or tails out of how to wants to format things.
Description was changed from ========== PeriodicWave returns sine wave generator if no coeffients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear and incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coefficients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear and incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ==========
The CQ bit was checked by rtoy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hongchan@chromium.org Link to the patchset: https://codereview.chromium.org/2712933003/#ps80001 (title: "Address review comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489769382248590,
"parent_rev": "bba421837cf80e6b786c736235f9ec10fa931302", "commit_rev":
"10c77a53e77e2ae475a00cbddb5da81f0f2965c4"}
Message was sent while issue was closed.
Description was changed from ========== PeriodicWave returns sine wave generator if no coefficients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear and incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html ========== to ========== PeriodicWave returns sine wave generator if no coefficients given If the PeriodWaveOptions option does not specify either real or imag components, the returned PeriodicWave should result in a sine wave when used with an OscillatorNode. The spec was unclear and incorrect in saying that the optional PeriodicWaveOptions dictionary had optional members specified, but actually required at least one to be specified. The spec was updated to make this clear that everything is correctly optional, and specifies what PeriodicWave should do PeriodicWaveOptions is given. Tests updated for this and an additional test to verify that the output is a sine wave. BUG=695513 TEST=constructor/periodicwave.html Review-Url: https://codereview.chromium.org/2712933003 Cr-Commit-Position: refs/heads/master@{#457824} Committed: https://chromium.googlesource.com/chromium/src/+/10c77a53e77e2ae475a00cbddb5d... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/10c77a53e77e2ae475a00cbddb5d... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
