New issue
Advanced search Search tips

Issue 764396 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: 2018-01-22
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 496282
issue 744804



Sign in to add a comment

AudioParam value setter should behave like setValueAtTime

Project Member Reported by rtoy@chromium.org, Sep 12 2017

Issue description

According to https://webaudio.github.io/web-audio-api/#widl-AudioParam-value, setting the value directly via the setter should behave as if setValueAtTime were called.

Make it so.

But also note the spec issue: https://github.com/WebAudio/web-audio-api/issues/1296 which should clarify exactly what the effect is.

Note also that this might break existing use of the value setter because it might throw under some conditions where it didn't before.  This can also change the behavior if there are any automations running at the time the setter is called.
 

Comment 1 by rtoy@chromium.org, Sep 13 2017

Blockedon: 496282

Comment 2 by rtoy@chromium.org, Sep 13 2017

Blockedon: 744804

Comment 3 by rtoy@chromium.org, Sep 13 2017

Cc: rtoy@chromium.org
 Issue 744804  has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25 2017

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

commit 48cba11435e21f6050c663bec57ea70b37dcf6aa
Author: Raymond Toy <rtoy@chromium.org>
Date: Wed Oct 25 20:45:06 2017

Deprecation warning for value setter is setValueAtTime

Print a deprecation warning if the value setter is called in the
middle of an event. This is only done once, independent of the number
of calls to the setter.

Also keep track of how often this conflict happens versus how many
times the value setter was called so we can report the percentage
statistic.  This is done for every call to the value setter.  The
statistics are uploaded when the context is closed. 

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/y4s3-aXbMOw/9s34pPQVBgAJ

Bug:  764396 
Test: 
Change-Id: I0fcf20661845c60a80d364ccbf733193e1c57777
Reviewed-on: https://chromium-review.googlesource.com/721572
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511577}
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/LayoutTests/http/tests/webaudio/autoplay-crossorigin-expected.txt
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range-expected.txt
[add] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/LayoutTests/webaudio/AudioParam/value-setter-warnings-expected.txt
[add] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/LayoutTests/webaudio/AudioParam/value-setter-warnings.html
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/LayoutTests/webaudio/AudioParam/worklet-warnings-expected.txt
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/LayoutTests/webaudio/unit-tests/audit-expected.txt
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/core/frame/Deprecation.cpp
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/AudioContext.h
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/AudioParam.cpp
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/AudioParam.h
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/48cba11435e21f6050c663bec57ea70b37dcf6aa/tools/metrics/histograms/histograms.xml

Comment 5 by rtoy@chromium.org, Oct 27 2017

Cc: -rtoy@chromium.org
Owner: rtoy@chromium.org
Status: Started (was: Available)

Comment 6 by rtoy@chromium.org, Jan 4 2018

NextAction: 2018-01-22

Comment 7 by rtoy@chromium.org, Jan 8 2018

Based on the statistics that have been gathered, there are (surprisingly!) very few calls to the setter.  And therefore very few conflicts between the value setter happening when a setCurveAtTime automation running.

It should be pretty safe to make this change in time for M66.  Too close to M65 to consider this change.
The NextAction date has arrived: 2018-01-22
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 6 2018

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

commit 02938fe25079c7ed527dc2481fadb03e5ff732b0
Author: Raymond Toy <rtoy@chromium.org>
Date: Tue Feb 06 18:50:15 2018

Value setter must call setValueAtTime

When using the AudioParam value setter, the setter must behave as if
setValueAtTime were called with the given value at the current audio
context time.  This includes throwing an error if the time is in the
middle of a setValueCurveAtTime automation.

This is a change in the old behavior where errors weren't thrown.
This also clarifies the behavior of the value setter when automations
are running.  Previously, the interaction between the setter and
automations were not very well specified.

Bug:  764396 
Test: AudioParam/audioparam-value-setter-error.html
Change-Id: Ic1da32c27f9e4f29e2df3bcbb6dd0bd485409e0e
Reviewed-on: https://chromium-review.googlesource.com/665318
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534737}
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/http/tests/webaudio/autoplay-crossorigin-expected.txt
[add] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioListener/audiolistener-set-position.html
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-initial-event.html
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range-expected.txt
[add] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-value-setter-error.html
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioParam/value-setter-warnings-expected.txt
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioParam/value-setter-warnings.html
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/AudioParam/worklet-warnings-expected.txt
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/no-dezippering.html
[add] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/Panner/panner-set-position.html
[add] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/LayoutTests/webaudio/resources/set-position-vs-curve-test.js
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/AudioListener.cpp
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/AudioListener.h
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/AudioListener.idl
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/AudioParam.cpp
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/AudioParam.h
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/AudioParam.idl
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/PannerNode.cpp
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/PannerNode.h
[modify] https://crrev.com/02938fe25079c7ed527dc2481fadb03e5ff732b0/third_party/WebKit/Source/modules/webaudio/PannerNode.idl

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 8 2018

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

commit 25c2cecdfa4f68a1e83c65c04bcff97152b00cd9
Author: Raymond Toy <rtoy@chromium.org>
Date: Thu Feb 08 20:23:08 2018

Remove value setter deprecation messages

Rmove the deprecation messages about the AudioParam value setter
changing to be exactly equivalent to AudioParam.setValueAtTime()

Bug:  764396 
Test: AudioParam/value-setter-warnings.html
Change-Id: Ia6f15f2994f33ba742a5b63b57b9e8716539721f
Reviewed-on: https://chromium-review.googlesource.com/905085
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535492}
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/LayoutTests/webaudio/AudioParam/value-setter-warnings-expected.txt
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/core/frame/Deprecation.cpp
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/AudioContext.h
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/AudioParam.cpp
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/AudioParam.h
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/25c2cecdfa4f68a1e83c65c04bcff97152b00cd9/tools/metrics/histograms/histograms.xml

Comment 11 by rtoy@chromium.org, Feb 12 2018

Status: Fixed (was: Started)
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 1

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

commit a751300d83b471921c752bbc2fe4f117d019eadb
Author: Raymond Toy <rtoy@chromium.org>
Date: Wed Aug 01 01:35:16 2018

Remove unused methods

To support deprecating the AudioParam value setter in favor of
setValueAtTime, some methods were added.  The deprecation has happened
and these are no longer needed.

Bug:  764396 
Test: 
Change-Id: I6e296e17582a078cacc01f91fb56af812079db38
Reviewed-on: https://chromium-review.googlesource.com/1157213
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579660}
[modify] https://crrev.com/a751300d83b471921c752bbc2fe4f117d019eadb/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc
[modify] https://crrev.com/a751300d83b471921c752bbc2fe4f117d019eadb/third_party/blink/renderer/modules/webaudio/audio_param_timeline.h

Sign in to add a comment