Bug 215868

Summary: Add support for sub-sample accurate start for AudioBufferSourceNode
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web AudioAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://chromium-review.googlesource.com/c/chromium/src/+/1212270
See Also: https://bugs.webkit.org/show_bug.cgi?id=215886
Bug Depends on:    
Bug Blocks: 212611    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2020-08-26 14:53:31 PDT
Add support for sub-sample accurate start for AudioBufferSourceNode.
Attachments
Patch (975.06 KB, patch)
2020-08-26 15:22 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (994.33 KB, patch)
2020-08-26 16:12 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (994.34 KB, patch)
2020-08-26 16:30 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2020-08-26 15:22:44 PDT
Created attachment 407344 [details] Patch
Chris Dumez
Comment 2 2020-08-26 16:12:27 PDT
Created attachment 407350 [details] Patch
Chris Dumez
Comment 3 2020-08-26 16:30:21 PDT
Created attachment 407354 [details] Patch
Darin Adler
Comment 4 2020-08-26 18:29:44 PDT
Comment on attachment 407354 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407354&action=review Crash looks like it is related? ASSERTION FAILED: m_finishedNodes.isEmpty() ./Modules/webaudio/BaseAudioContext.cpp(188) : virtual WebCore::BaseAudioContext::~BaseAudioContext() > Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp:82 > + size_t endFrame = 0; Stray space after the "=" here. > Source/WebCore/platform/audio/AudioUtilities.cpp:85 > + double frame = round(time * sampleRate * oversampleFactor) / oversampleFactor; > + > + switch (rounding) { > + case SampleFrameRounding::Nearest: > + frame = round(frame); > + break; > + case SampleFrameRounding::Down: > + frame = floor(frame); > + break; > + case SampleFrameRounding::Up: > + frame = ceil(frame); > + break; > + } std::round/floor/ceil? Should not matter here since it’s all double.
EWS
Comment 5 2020-08-26 21:28:35 PDT
Committed r266221: <https://trac.webkit.org/changeset/266221> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407354 [details].
Radar WebKit Bug Importer
Comment 6 2020-08-26 21:29:20 PDT
<rdar://problem/67849532>
Chris Dumez
Comment 7 2020-08-27 08:54:55 PDT
(In reply to Darin Adler from comment #4) > Comment on attachment 407354 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407354&action=review > > Crash looks like it is related? > > ASSERTION FAILED: m_finishedNodes.isEmpty() > ./Modules/webaudio/BaseAudioContext.cpp(188) : virtual > WebCore::BaseAudioContext::~BaseAudioContext() > > > Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp:82 > > + size_t endFrame = 0; > > Stray space after the "=" here. > > > Source/WebCore/platform/audio/AudioUtilities.cpp:85 > > + double frame = round(time * sampleRate * oversampleFactor) / oversampleFactor; > > + > > + switch (rounding) { > > + case SampleFrameRounding::Nearest: > > + frame = round(frame); > > + break; > > + case SampleFrameRounding::Down: > > + frame = floor(frame); > > + break; > > + case SampleFrameRounding::Up: > > + frame = ceil(frame); > > + break; > > + } > > std::round/floor/ceil? Should not matter here since it’s all double. Sorry, I missed those comments before landing. I applied those fixes in <https://trac.webkit.org/changeset/266234>.
Note You need to log in before you can comment on or make changes to this bug.