Skip to content

Commit ad6bc59

Browse files
jyavenardjyavenard
committed
[WebAudio/WebM] Incorrect number of frames returned if decoding frame rate doesn't match original
https://bugs.webkit.org/show_bug.cgi?id=229251 rdar://problem/82095650 Source/WebCore: Reviewed by Eric Carlson.. We can't rely on CoreMedia to performed the trimming correctly when resampling is also to be done. It gives unexpected results. Let's do it ourselves instead. Test: webaudio/decode-audio-data-webm-opus-resample.html * platform/audio/cocoa/AudioFileReaderCocoa.cpp: (WebCore::AudioFileReader::decodeWebMData const): LayoutTests: Reviewed by Eric Carlson. * webaudio/decode-audio-data-webm-opus-resample-expected.txt: Added. * webaudio/decode-audio-data-webm-opus-resample.html: Added. Canonical link: https://commits.webkit.org/241483@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282196 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent eab80e6 commit ad6bc59

6 files changed

Lines changed: 101 additions & 11 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2021-09-09 Jean-Yves Avenard <jya@apple.com>
2+
3+
[WebAudio/WebM] Incorrect number of frames returned if decoding frame rate doesn't match original
4+
https://bugs.webkit.org/show_bug.cgi?id=229251
5+
rdar://problem/82095650
6+
7+
Reviewed by Eric Carlson.
8+
9+
* webaudio/decode-audio-data-webm-opus-resample-expected.txt: Added.
10+
* webaudio/decode-audio-data-webm-opus-resample.html: Added.
11+
112
2021-09-07 Tim Nguyen <ntim@apple.com>
213

314
Re-import css/css-pseudo WPT

‎LayoutTests/platform/mac/TestExpectations‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,6 +1712,7 @@ webkit.org/b/214155 imported/w3c/web-platform-tests/html/cross-origin-embedder-p
17121712
[ Catalina Mojave BigSur ] media/media-source/media-webm-opus-partial.html [ Skip ]
17131713
[ Catalina Mojave BigSur ] media/media-source/media-webm-opus-partial-abort.html [ Skip ]
17141714
[ Catalina Mojave BigSur ] webaudio/decode-audio-data-webm-opus.html [ Skip ]
1715+
[ Catalina Mojave BigSur ] webaudio/decode-audio-data-webm-opus-resample.html [ Skip ]
17151716
[ Catalina Mojave BigSur ] webaudio/decode-audio-data-webm-vorbis.html [ Skip ]
17161717

17171718
webkit.org/b/214422 imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/suspend-after-construct.html [ Pass Failure ]

‎LayoutTests/webaudio/decode-audio-data-webm-opus-resample-expected.txt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Test that decoding an opus webm file with resampling succeeds
2+
3+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
4+
5+
6+
PASS Successfully decoded content
7+
PASS Decoding returned the right number of frames.
8+
PASS successfullyParsed is true
9+
10+
TEST COMPLETE
11+

‎LayoutTests/webaudio/decode-audio-data-webm-opus-resample.html‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../resources/js-test.js"></script>
5+
<script type="text/javascript" src="resources/audio-testing.js"></script>
6+
</head>
7+
<body>
8+
<script>
9+
description("Test that decoding an opus webm file with resampling succeeds");
10+
11+
window.jsTestIsAsync = true;
12+
13+
var context = new window.AudioContext({ sampleRate: 44100 });
14+
var request = new XMLHttpRequest();
15+
request.open("GET", 'resources/media/opus.webm', true);
16+
request.responseType = "arraybuffer";
17+
18+
request.onload = function() {
19+
context.decodeAudioData(request.response, (buffer) => {
20+
testPassed("Successfully decoded content");
21+
// File is exactly 1-0.0065s long @ 48000Hz, so 47688 frames, after resampling it should be 47688/48000*44100 = 43813.
22+
if (buffer.length === 43813)
23+
testPassed("Decoding returned the right number of frames.");
24+
else
25+
testFailed("Decoding returned the wrong number of frames: " + buffer.length);
26+
finishJSTest();
27+
}, () => {
28+
testFailed("Failed to decode file");
29+
finishJSTest();
30+
});
31+
}
32+
request.send();
33+
34+
</script>
35+
</body>
36+
</html>

‎Source/WebCore/ChangeLog‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
2021-09-09 Jean-Yves Avenard <jya@apple.com>
2+
3+
[WebAudio/WebM] Incorrect number of frames returned if decoding frame rate doesn't match original
4+
https://bugs.webkit.org/show_bug.cgi?id=229251
5+
rdar://problem/82095650
6+
7+
Reviewed by Eric Carlson..
8+
9+
We can't rely on CoreMedia to performed the trimming correctly when resampling is also to be done.
10+
It gives unexpected results. Let's do it ourselves instead.
11+
Test: webaudio/decode-audio-data-webm-opus-resample.html
12+
13+
* platform/audio/cocoa/AudioFileReaderCocoa.cpp:
14+
(WebCore::AudioFileReader::decodeWebMData const):
15+
116
2021-09-08 Youenn Fablet <youenn@apple.com>
217

318
RTCPeerConnection.addIceCandidate takes an optional argument

‎Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -318,19 +318,26 @@ std::optional<size_t> AudioFileReader::decodeWebMData(AudioBufferList& bufferLis
318318
if (magicCookie && magicCookieSize)
319319
PAL::AudioConverterSetProperty(converter, kAudioConverterDecompressionMagicCookie, magicCookieSize, magicCookie);
320320

321-
AudioConverterPrimeInfo primeInfo = { UInt32(m_webmData->m_track->codecDelay().value_or(MediaTime()).toDouble() * outFormat.mSampleRate), 0 };
322-
INFO_LOG(LOGIDENTIFIER, "Will drop %u leading frames out of %llu", primeInfo.leadingFrames, numberOfFrames);
323-
PAL::AudioConverterSetProperty(converter, kAudioConverterPrimeInfo, sizeof(primeInfo), &primeInfo);
324-
UInt32 primeMethod = kConverterPrimeMethod_None;
325-
PAL::AudioConverterSetProperty(converter, kAudioConverterPrimeMethod, sizeof(primeMethod), &primeMethod);
326-
327321
AudioBufferListHolder decodedBufferList(inFormat.mChannelsPerFrame);
328322
if (!decodedBufferList) {
329323
RELEASE_LOG_FAULT(WebAudio, "Unable to create decoder");
330324
return { };
331325
}
332326

327+
// Instruct the decoder to not drop any frames
328+
// (by default the Opus decoder assumes that SampleRate / 400 frames are to be dropped.
329+
AudioConverterPrimeInfo primeInfo = { 0, 0 };
330+
PAL::AudioConverterSetProperty(converter, kAudioConverterPrimeInfo, sizeof(primeInfo), &primeInfo);
331+
UInt32 primeMethod = kConverterPrimeMethod_None;
332+
PAL::AudioConverterSetProperty(converter, kAudioConverterPrimeMethod, sizeof(primeMethod), &primeMethod);
333+
334+
uint32_t leadingTrim = m_webmData->m_track->codecDelay().value_or(MediaTime::zeroTime()).toDouble() * outFormat.mSampleRate;
335+
// Calculate the number of trailing frames to be trimmed by rounding to nearest integer while minimizing cummulative rounding errors.
336+
uint32_t trailingTrim = (m_webmData->m_track->codecDelay().value_or(MediaTime::zeroTime()) + m_webmData->m_track->discardPadding().value_or(MediaTime::zeroTime())).toDouble() * outFormat.mSampleRate - leadingTrim + 0.5;
337+
INFO_LOG(LOGIDENTIFIER, "Will drop ", leadingTrim, " leading and ", trailingTrim, " trailing frames out of ", numberOfFrames);
338+
333339
size_t decodedFrames = 0;
340+
size_t totalDecodedFrames = 0;
334341
OSStatus status;
335342
for (size_t i = 0; i < m_webmData->m_samples.size(); i++) {
336343
auto& sample = m_webmData->m_samples[i];
@@ -357,12 +364,14 @@ std::optional<size_t> AudioFileReader::decodeWebMData(AudioBufferList& bufferLis
357364

358365
do {
359366
if (numberOfFrames < decodedFrames) {
360-
RELEASE_LOG_FAULT(WebAudio, "Decoded more frames than first calculated");
367+
RELEASE_LOG_FAULT(WebAudio, "Decoded more frames than first calculated, no available space left");
361368
return { };
362369
}
363370
// in: the max number of packets we can handle from the decoder.
364371
// out: the number of packets the decoder is actually returning.
365-
UInt32 numFrames = std::min<uint32_t>(std::numeric_limits<int32_t>::max() / sizeof(float), numberOfFrames - decodedFrames);
372+
// The AudioConverter will sometimes pad with trailing silence if we set the free space to what it actually is (numberOfFrames - decodedFrames).
373+
// So we set it to what there is left to decode instead.
374+
UInt32 numFrames = std::min<uint32_t>(std::numeric_limits<int32_t>::max() / sizeof(float), numberOfFrames - totalDecodedFrames);
366375

367376
for (UInt32 i = 0; i < inFormat.mChannelsPerFrame; i++) {
368377
decodedBufferList->mBuffers[i].mNumberChannels = 1;
@@ -374,12 +383,19 @@ std::optional<size_t> AudioFileReader::decodeWebMData(AudioBufferList& bufferLis
374383
RELEASE_LOG_FAULT(WebAudio, "Error decoding data");
375384
return { };
376385
}
386+
totalDecodedFrames += numFrames;
387+
if (leadingTrim > 0) {
388+
UInt32 toTrim = std::min(leadingTrim, numFrames);
389+
for (UInt32 i = 0; i < outFormat.mChannelsPerFrame; i++)
390+
memcpy(decodedBufferList->mBuffers[i].mData, static_cast<float*>(decodedBufferList->mBuffers[i].mData) + toTrim, (numFrames - toTrim) * sizeof(float));
391+
leadingTrim -= toTrim;
392+
numFrames -= toTrim;
393+
}
377394
decodedFrames += numFrames;
378395
} while (status != kNoMoreDataErr && status != noErr);
379396
}
380-
size_t paddingFrames = m_webmData->m_track->discardPadding().value_or(MediaTime()).toDouble() * outFormat.mSampleRate;
381-
if (decodedFrames > paddingFrames)
382-
return decodedFrames - paddingFrames;
397+
if (decodedFrames > trailingTrim)
398+
return decodedFrames - trailingTrim;
383399
return 0;
384400
}
385401
#endif

0 commit comments

Comments
 (0)