Bug 215301

Summary: Align existing AudioParam API with the specification
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web AudioAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: clark_wang, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, jer.noble, kondapallykalyan, philipj, sam, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://www.w3.org/TR/webaudio/#AudioParam
Bug Depends on: 215289    
Bug Blocks: 212611    
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2020-08-07 16:38:02 PDT
Align existing AudioParam API with the specification: - https://www.w3.org/TR/webaudio/#AudioParam
Attachments
WIP Patch (12.71 KB, patch)
2020-08-07 17:24 PDT, Chris Dumez
no flags
WIP Patch (59.92 KB, patch)
2020-08-07 18:23 PDT, Chris Dumez
no flags
WIP Patch (157.86 KB, patch)
2020-08-07 19:34 PDT, Chris Dumez
no flags
Patch (163.21 KB, patch)
2020-08-07 19:42 PDT, Chris Dumez
no flags
Patch (163.37 KB, patch)
2020-08-10 11:06 PDT, Chris Dumez
no flags
Patch (173.63 KB, patch)
2020-08-10 11:58 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2020-08-07 17:24:08 PDT
Created attachment 406234 [details] WIP Patch
Chris Dumez
Comment 2 2020-08-07 18:23:31 PDT
Created attachment 406238 [details] WIP Patch
Chris Dumez
Comment 3 2020-08-07 19:34:08 PDT
Created attachment 406241 [details] WIP Patch
Chris Dumez
Comment 4 2020-08-07 19:42:41 PDT
Sam Weinig
Comment 5 2020-08-10 10:15:04 PDT
Comment on attachment 406242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406242&action=review > Source/WebCore/ChangeLog:19 > + - Throw exceptions when events overlap. This part is based on Chromium's implementation here: > + - https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc#L513 Is this not spec'd? If not, is there bug filed on the spec to add it? > Source/WebCore/Modules/webaudio/AudioParamTimeline.h:90 > + double time() const { return m_time; } It may not make sense or make bindings more troublesome, but can we use something like WallTime/MonotonicTime for these time values?
Chris Dumez
Comment 6 2020-08-10 11:01:45 PDT
Comment on attachment 406242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406242&action=review >> Source/WebCore/ChangeLog:19 >> + - https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc#L513 > > Is this not spec'd? If not, is there bug filed on the spec to add it? Yes, it is specified here: https://webaudio.github.io/web-audio-api/#dfn-automation-event """ If setValueCurveAtTime() is called for time 𝑇 T and duration 𝐷 D and there are any events having a time strictly greater than 𝑇 T , but strictly less than 𝑇+𝐷 T + D , then a NotSupportedError exception MUST be thrown. In other words, it’s not ok to schedule a value curve during a time period containing other events, but it’s ok to schedule a value curve exactly at the time of another event. Similarly a NotSupportedError exception MUST be thrown if any automation method is called at a time which is contained in [𝑇,𝑇+𝐷) [ T , T + D ) , 𝑇 T being the time of the curve and 𝐷 D its duration. """
Chris Dumez
Comment 7 2020-08-10 11:06:31 PDT
Chris Dumez
Comment 8 2020-08-10 11:58:30 PDT
EWS
Comment 9 2020-08-10 12:29:07 PDT
Committed r265440: <https://trac.webkit.org/changeset/265440> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406314 [details].
Radar WebKit Bug Importer
Comment 10 2020-08-10 12:31:36 PDT
Note You need to log in before you can comment on or make changes to this bug.