Changeset 282196 in webkit

Timestamp:

Sep 9, 2021 12:02:21 AM (10 months ago)

Author:

Jean-Yves Avenard

Message:

[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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• LayoutTests/webaudio/decode-audio-data-webm-opus-resample-expected.txt (added)
• LayoutTests/webaudio/decode-audio-data-webm-opus-resample.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-09-09  Jean-Yves Avenard  <jya@apple.com>
+
+        [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
+
+        Reviewed by Eric Carlson.
+
+        * webaudio/decode-audio-data-webm-opus-resample-expected.txt: Added.
+        * webaudio/decode-audio-data-webm-opus-resample.html: Added.
+
 2021-09-07  Tim Nguyen  <ntim@apple.com>
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1713 +1713 @@
 [ Catalina Mojave BigSur ] media/media-source/media-webm-opus-partial-abort.html [ Skip ]
 [ Catalina Mojave BigSur ] webaudio/decode-audio-data-webm-opus.html [ Skip ]
+[ Catalina Mojave BigSur ] webaudio/decode-audio-data-webm-opus-resample.html [ Skip ]
 [ Catalina Mojave BigSur ] webaudio/decode-audio-data-webm-vorbis.html [ Skip ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-09  Jean-Yves Avenard  <jya@apple.com>
+
+        [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
+
+        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):
+
 2021-09-08  Youenn Fablet  <youenn@apple.com>
 

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

@@ -319 +319 @@
         PAL::AudioConverterSetProperty(converter, kAudioConverterDecompressionMagicCookie, magicCookieSize, magicCookie);
 
-    AudioConverterPrimeInfo primeInfo = { UInt32(m_webmData->m_track->codecDelay().value_or(MediaTime()).toDouble() * outFormat.mSampleRate), 0 };
-    INFO_LOG(LOGIDENTIFIER, "Will drop %u leading frames out of %llu", primeInfo.leadingFrames, numberOfFrames);
+    AudioBufferListHolder decodedBufferList(inFormat.mChannelsPerFrame);
+    if (!decodedBufferList) {
+        RELEASE_LOG_FAULT(WebAudio, "Unable to create decoder");
+        return { };
+    }
+
+    // Instruct the decoder to not drop any frames
+    // (by default the Opus decoder assumes that SampleRate / 400 frames are to be dropped.
+    AudioConverterPrimeInfo primeInfo = { 0, 0 };
     PAL::AudioConverterSetProperty(converter, kAudioConverterPrimeInfo, sizeof(primeInfo), &primeInfo);
     UInt32 primeMethod = kConverterPrimeMethod_None;
     PAL::AudioConverterSetProperty(converter, kAudioConverterPrimeMethod, sizeof(primeMethod), &primeMethod);
 
-    AudioBufferListHolder decodedBufferList(inFormat.mChannelsPerFrame);
-    if (!decodedBufferList) {
-        RELEASE_LOG_FAULT(WebAudio, "Unable to create decoder");
-        return { };
-    }
+    uint32_t leadingTrim = m_webmData->m_track->codecDelay().value_or(MediaTime::zeroTime()).toDouble() * outFormat.mSampleRate;
+    // Calculate the number of trailing frames to be trimmed by rounding to nearest integer while minimizing cummulative rounding errors.
+    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;
+    INFO_LOG(LOGIDENTIFIER, "Will drop ", leadingTrim, " leading and ", trailingTrim, " trailing frames out of ", numberOfFrames);
 
     size_t decodedFrames = 0;
+    size_t totalDecodedFrames = 0;
     OSStatus status;
     for (size_t i = 0; i < m_webmData->m_samples.size(); i++) {
@@ -358 +365 @@
         do {
             if (numberOfFrames < decodedFrames) {
-                RELEASE_LOG_FAULT(WebAudio, "Decoded more frames than first calculated");
+                RELEASE_LOG_FAULT(WebAudio, "Decoded more frames than first calculated, no available space left");
                 return { };
             }
             // in: the max number of packets we can handle from the decoder.
             // out: the number of packets the decoder is actually returning.
-            UInt32 numFrames = std::min<uint32_t>(std::numeric_limits<int32_t>::max() / sizeof(float), numberOfFrames - decodedFrames);
+            // The AudioConverter will sometimes pad with trailing silence if we set the free space to what it actually is (numberOfFrames - decodedFrames).
+            // So we set it to what there is left to decode instead.
+            UInt32 numFrames = std::min<uint32_t>(std::numeric_limits<int32_t>::max() / sizeof(float), numberOfFrames - totalDecodedFrames);
 
             for (UInt32 i = 0; i < inFormat.mChannelsPerFrame; i++) {
@@ -375 +384 @@
                 return { };
             }
+            totalDecodedFrames += numFrames;
+            if (leadingTrim > 0) {
+                UInt32 toTrim = std::min(leadingTrim, numFrames);
+                for (UInt32 i = 0; i < outFormat.mChannelsPerFrame; i++)
+                    memcpy(decodedBufferList->mBuffers[i].mData, static_cast<float*>(decodedBufferList->mBuffers[i].mData) + toTrim, (numFrames - toTrim) * sizeof(float));
+                leadingTrim -= toTrim;
+                numFrames -= toTrim;
+            }
             decodedFrames += numFrames;
         } while (status != kNoMoreDataErr && status != noErr);
     }
-    size_t paddingFrames = m_webmData->m_track->discardPadding().value_or(MediaTime()).toDouble() * outFormat.mSampleRate;
-    if (decodedFrames > paddingFrames)
-        return decodedFrames - paddingFrames;
+    if (decodedFrames > trailingTrim)
+        return decodedFrames - trailingTrim;
     return 0;
 }