Bug 216781

Summary: Values returned by FFTFrame::doFFT() are twice as large as they should be
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web AudioAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, darin, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, pnormand, sergio, vjaquez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 216735, 216736    
Bug Blocks: 212611    
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch none

Description Chris Dumez 2020-09-21 08:29:54 PDT
Values returned by FFTFrame::doFFT() are twice as large as they should be.
Comment 1 Chris Dumez 2020-09-21 08:48:33 PDT
Created attachment 409277 [details]
Patch
Comment 2 Chris Dumez 2020-09-21 12:26:40 PDT
Created attachment 409299 [details]
Patch
Comment 3 Chris Dumez 2020-09-21 15:06:01 PDT
Created attachment 409325 [details]
Patch
Comment 4 EWS 2020-09-21 15:25:07 PDT
Found 2 new test failures: webrtc/peer-connection-audio-mute2.html, webrtc/peer-connection-remote-audio-mute2.html
Comment 5 EWS 2020-09-21 16:27:54 PDT
Committed r267383: <https://trac.webkit.org/changeset/267383>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409325 [details].
Comment 6 Radar WebKit Bug Importer 2020-09-21 16:28:18 PDT
<rdar://problem/69335018>
Comment 7 Philippe Normand 2020-09-22 02:37:34 PDT
https://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug%20(Tests)/r267388%20(7283)/webaudio/AudioParam/audioparam-k-rate-crash-log.txt

Thread 1 (Thread 0x7fd6477fe700 (LWP 37517)):
#0  0x00007fd71634ed8e in WTFCrash() () at ../../Source/WTF/wtf/Assertions.cpp:295
#1  0x00007fd7227f086b in WTFCrashWithInfo(int, char const*, char const*, int) () at DerivedSources/ForwardingHeaders/wtf/Assertions.h:671
#2  0x00007fd726ab57d7 in WebCore::HRTFPanner::pan(double, double, WebCore::AudioBus const*, WebCore::AudioBus*, unsigned long) (this=0x7fd6cacffa00, desiredAzimuth=0, elevation=-2.5044781608585254e-06, inputBus=0x7fd6b26cfcb0, outputBus=0x7fd6b26cfce8, framesToProcess=128) at ../../Source/WebCore/platform/audio/HRTFPanner.cpp:248
#3  0x00007fd726ab5f25 in WebCore::HRTFPanner::panWithSampleAccurateValues(double*, double*, WebCore::AudioBus const*, WebCore::AudioBus*, unsigned long) (this=0x7fd6cacffa00, azimuth=0x7fd6477fc870, elevation=0x7fd6477fc470, inputBus=0x7fd6b26cfcb0, outputBus=0x7fd6b26cfce8, framesToProcess=128) at ../../Source/WebCore/platform/audio/HRTFPanner.cpp:327
#4  0x00007fd7255608fd in WebCore::PannerNode::processSampleAccurateValues(WebCore::AudioBus*, WebCore::AudioBus const*, unsigned long) (this=0x7fd6c90009e0, destination=0x7fd6b26cfce8, source=0x7fd6b26cfcb0, framesToProcess=128) at ../../Source/WebCore/Modules/webaudio/PannerNode.cpp:234
#5  0x00007fd725560132 in WebCore::PannerNode::process(unsigned long) (this=0x7fd6c90009e0, framesToProcess=128) at ../../Source/WebCore/Modules/webaudio/PannerNode.cpp:167
#6  0x00007fd7254e70ba in WebCore::AudioNode::processIfNecessary(unsigned long) (this=0x7fd6c90009e0, framesToProcess=128) at ../../Source/WebCore/Modules/webaudio/AudioNode.cpp:485
#7  0x00007fd7254e9317 in WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long) (this=0x7fd6464a3888, inPlaceBus=0x0, framesToProcess=128) at ../../Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp:123
#8  0x00007fd7254e9146 in WebCore::AudioNodeInput::sumAllConnections(WebCore::AudioBus*, unsigned long) (this=0x7fd6464a36e8, summingBus=0x7fd6b26cfb28, framesToProcess=128) at ../../Source/WebCore/Modules/webaudio/AudioNodeInput.cpp:206
#9  0x00007fd7254e7727 in WebCore::AudioNodeInput::pull(WebCore::AudioBus*, unsigned long) (this=0x7fd6464a36e8, inPlaceBus=0x7fd6b26cfb60, framesToProcess=128) at ../../Source/WebCore/Modules/webaudio/AudioNodeInput.cpp:234
#10 0x00007fd7254b8a51 in WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) (this=0x7fd6c9021870, destinationBus=0x7fd6b26cfb60, numberOfFrames=128, outputPosition=...) at ../../Source/WebCore/Modules/webaudio/AudioDestinationNode.cpp:85
#11 0x00007fd72555c40a in WebCore::OfflineAudioDestinationNode::offlineRender() (this=0x7fd6c9021870) at ../../Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:151
#12 0x00007fd7255677bf in WebCore::OfflineAudioDestinationNode::startRendering()::$_0::operator()() const (this=0x7fd6c81795b8) at ../../Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:102
#13 0x00007fd72556778e in WTF::Detail::CallableWrapper<WebCore::OfflineAudioDestinationNode::startRendering()::$_0, void>::call() (this=0x7fd6c81795b0) at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#14 0x00007fd7227ef4a2 in WTF::Function<void ()>::operator()() const (this=0x7fd6477fdc40) at DerivedSources/ForwardingHeaders/wtf/Function.h:83
#15 0x00007fd71638da28 in WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (newThreadContext=0x7fd6b2c1f1e0) at ../../Source/WTF/wtf/Threading.cpp:179
#16 0x00007fd71642a8a8 in WTF::wtfThreadEntryPoint(void*) (context=0x7fd6b2c1f1e0) at ../../Source/WTF/wtf/posix/ThreadingPOSIX.cpp:213
#17 0x00007fd7106714d2 in start_thread (arg=<optimized out>) at pthread_create.c:477
#18 0x00007fd70e1e94d3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

STDERR: ASSERTION FAILED: frameDelayL1 / sampleRate() < MaxDelayTimeSeconds && frameDelayR1 / sampleRate() < MaxDelayTimeSeconds
STDERR: ../../Source/WebCore/platform/audio/HRTFPanner.cpp(248) : virtual void WebCore::HRTFPanner::pan(double, double, const WebCore::AudioBus *, WebCore::AudioBus *, size_t)
STDERR: 1   0x7fd71634ed89 WTFCrash
STDERR: 2   0x7fd7227f086b /app/webkit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0xae3386b) [0x7fd7227f086b]
STDERR: 3   0x7fd726ab57d7 WebCore::HRTFPanner::pan(double, double, WebCore::AudioBus const*, WebCore::AudioBus*, unsigned long)
STDERR: 4   0x7fd726ab5f25 WebCore::HRTFPanner::panWithSampleAccurateValues(double*, double*, WebCore::AudioBus const*, WebCore::AudioBus*, unsigned long)
STDERR: 5   0x7fd7255608fd WebCore::PannerNode::processSampleAccurateValues(WebCore::AudioBus*, WebCore::AudioBus const*, unsigned long)
STDERR: 6   0x7fd725560132 WebCore::PannerNode::process(unsigned long)
STDERR: 7   0x7fd7254e70ba WebCore::AudioNode::processIfNecessary(unsigned long)
STDERR: 8   0x7fd7254e9317 WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long)
STDERR: 9   0x7fd7254e9146 WebCore::AudioNodeInput::sumAllConnections(WebCore::AudioBus*, unsigned long)
STDERR: 10  0x7fd7254e7727 WebCore::AudioNodeInput::pull(WebCore::AudioBus*, unsigned long)
STDERR: 11  0x7fd7254b8a51 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&)
STDERR: 12  0x7fd72555c40a WebCore::OfflineAudioDestinationNode::offlineRender()
STDERR: 13  0x7fd7255677bf /app/webkit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0xdbaa7bf) [0x7fd7255677bf]
STDERR: 14  0x7fd72556778e /app/webkit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0xdbaa78e) [0x7fd72556778e]
STDERR: 15  0x7fd7227ef4a2 WTF::Function<void ()>::operator()() const
STDERR: 16  0x7fd71638da28 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)
STDERR: 17  0x7fd71642a8a8 /app/webkit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x3d6f8a8) [0x7fd71642a8a8]
STDERR: 18  0x7fd7106714d2 /usr/lib/x86_64-linux-gnu/libpthread.so.0(+0x84d2) [0x7fd7106714d2]
STDERR: 19  0x7fd70e1e94d3 clone
Comment 8 Philippe Normand 2020-09-22 02:39:58 PDT
Also in GTK 2 new failures:

webaudio/Analyser/realtimeanalyser-freq-data-smoothing.html [ Failure ]
webaudio/Analyser/realtimeanalyser-freq-data.html [ Failure ]

And these 2 just need a rebaseline i think:

webaudio/Analyser/realtimeanalyser-fftsize-reset.html
webaudio/Analyser/realtimeanalyser-multiple-calls.html
Comment 9 Chris Dumez 2020-09-22 12:05:15 PDT
(In reply to Philippe Normand from comment #8)
> Also in GTK 2 new failures:
> 
> webaudio/Analyser/realtimeanalyser-freq-data-smoothing.html [ Failure ]
> webaudio/Analyser/realtimeanalyser-freq-data.html [ Failure ]
> 
> And these 2 just need a rebaseline i think:
> 
> webaudio/Analyser/realtimeanalyser-fftsize-reset.html
> webaudio/Analyser/realtimeanalyser-multiple-calls.html

I re-introduced the GTK-specific implementation of FFTFrame::multiply() in <https://trac.webkit.org/changeset/267428> to try and be more conservative.

I could use help from a GTK person to figure out these issues if they persist since I am unable to reproduce the issue.
Comment 10 Chris Dumez 2020-09-22 12:08:11 PDT
(In reply to Philippe Normand from comment #8)
> Also in GTK 2 new failures:
> 
> webaudio/Analyser/realtimeanalyser-freq-data-smoothing.html [ Failure ]
> webaudio/Analyser/realtimeanalyser-freq-data.html [ Failure ]

Those are the tests that started passing on cocoa ports after my fix. Looks like they are still failing on GTK for some reason. This is not a regression per-say since those tests were already failing before my change. It'd be good to figure out why they are passing for Mac but not GTK. Seems likely related to the GStreamer-specific implementation of FFTFrame.

> And these 2 just need a rebaseline i think:
> 
> webaudio/Analyser/realtimeanalyser-fftsize-reset.html
> webaudio/Analyser/realtimeanalyser-multiple-calls.html

Agreed, those look like they merely need new baselines.
Comment 11 Philippe Normand 2020-09-23 04:18:29 PDT
Comment on attachment 409325 [details]
Patch

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

> Source/WebCore/platform/audio/gstreamer/FFTFrameGStreamer.cpp:-145
> -    float* imagData = m_imagData.data();
> -    float* realData = m_realData.data();
> -    for (unsigned i = 0; i < unpackedFFTDataSize(m_FFTSize); ++i) {
> -        imagData[i] = m_complexData[i].i * scaleFactor;
> -        realData[i] = m_complexData[i].r * scaleFactor;
> -    }

Removing this entirely was not a great idea. The m_imagData and m_realData still have to be updated after applying the FFT.
Comment 12 Chris Dumez 2020-09-23 08:19:37 PDT
Comment on attachment 409325 [details]
Patch

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

>> Source/WebCore/platform/audio/gstreamer/FFTFrameGStreamer.cpp:-145
>> -    }
> 
> Removing this entirely was not a great idea. The m_imagData and m_realData still have to be updated after applying the FFT.

Yep, I totally missed it. Sorry about that.