Bug 215946

Summary: AudioParam.value setter should call setValueAtTime(value, now)
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web AudioAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, jer.noble, kondapallykalyan, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://www.w3.org/TR/webaudio/#dom-audioparam-value
See Also: https://bugs.webkit.org/show_bug.cgi?id=215901
Bug Depends on:    
Bug Blocks: 212611    
Attachments:
Description Flags
Patch none

Description Chris Dumez 2020-08-28 12:57:12 PDT
AudioParam.value setter should call setValueAtTime(value, now), as per:
- https://www.w3.org/TR/webaudio/#dom-audioparam-value
Comment 1 Chris Dumez 2020-08-28 13:19:33 PDT
Created attachment 407498 [details]
Patch
Comment 2 Darin Adler 2020-08-28 14:27:23 PDT
Comment on attachment 407498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407498&action=review

> Source/WebCore/Modules/webaudio/AudioParam.cpp:94
> +    setValue(value);
> +    auto result = setValueAtTime(m_value, context().currentTime());

Why call both setValue and setValueAtTime?
Comment 3 EWS 2020-08-28 14:27:33 PDT
Committed r266293: <https://trac.webkit.org/changeset/266293>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407498 [details].
Comment 4 Darin Adler 2020-08-28 14:27:47 PDT
Comment on attachment 407498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407498&action=review

>> Source/WebCore/Modules/webaudio/AudioParam.cpp:94
>> +    auto result = setValueAtTime(m_value, context().currentTime());
> 
> Why call both setValue and setValueAtTime?

I guess we are using setValue to clamp the value for us.
Comment 5 Radar WebKit Bug Importer 2020-08-28 14:28:18 PDT
<rdar://problem/67961959>
Comment 6 Darin Adler 2020-08-28 14:29:17 PDT
Comment on attachment 407498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407498&action=review

>>> Source/WebCore/Modules/webaudio/AudioParam.cpp:94
>>> +    auto result = setValueAtTime(m_value, context().currentTime());
>> 
>> Why call both setValue and setValueAtTime?
> 
> I guess we are using setValue to clamp the value for us.

I’m really unclear on whether this is intentional and correct or not. And don’t understand the tests well enough to know if it’s tested.