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

Description Chris Dumez 2020-08-26 14:53:31 PDT 
Add support for sub-sample accurate start for AudioBufferSourceNode.
Comment 1 Chris Dumez 2020-08-26 15:22:44 PDT 
Created attachment 407344 [details]
Patch
Comment 2 Chris Dumez 2020-08-26 16:12:27 PDT 
Created attachment 407350 [details]
Patch
Comment 3 Chris Dumez 2020-08-26 16:30:21 PDT 
Created attachment 407354 [details]
Patch
Comment 4 Darin Adler 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.
Comment 5 EWS 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].
Comment 6 Radar WebKit Bug Importer 2020-08-26 21:29:20 PDT 
<rdar://problem/67849532>
Comment 7 Chris Dumez 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>.