Changeset 287249 in webkit

Timestamp:

Dec 19, 2021 4:03:00 PM (7 months ago)

Author:

Jean-Yves Avenard

Message:

Don't pack audio samples with discontinuity together
​https://bugs.webkit.org/show_bug.cgi?id=234458
rdar://86659914

Reviewed by Eric Carlson.

Some webm content may have a data gap between frames. Normally audio frames
are packed in 2s block. When we pack the samples with discontinuities, those
discontinuities would all be accumulated at the 2s boundary which makes them
much more audible.
The CMSampleBufferCreateReady API should allow us to pack samples with
discontinuities as we can give a vector of CMSampleTimingInfo with the
exact information for all packets.
However, this data appears to be ignored and the discontinuities is still
heard at the 2s boundary.
So we no longer pack samples with discontinuities so that the frame
timestamps will be more accurate and no audible artefacts are heard on
small gaps.

Manually tested and verified manually. This is getting around an issue
in CoreMedia that inserts very audible artifacts when there's a gap between
samples.

• platform/graphics/cocoa/SourceBufferParserWebM.cpp:

(WebCore::SourceBufferParserWebM::AudioTrackData::resetCompleted):
(WebCore::SourceBufferParserWebM::AudioTrackData::consumeFrameData):
(WebCore::SourceBufferParserWebM::AudioTrackData::createSampleBuffer):

• platform/graphics/cocoa/SourceBufferParserWebM.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/cocoa/SourceBufferParserWebM.cpp (modified) (5 diffs)
• platform/graphics/cocoa/SourceBufferParserWebM.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-19  Jean-Yves Avenard  <jya@apple.com>
+
+        Don't pack audio samples with discontinuity together
+        https://bugs.webkit.org/show_bug.cgi?id=234458
+        rdar://86659914
+
+        Reviewed by Eric Carlson.
+
+        Some webm content may have a data gap between frames. Normally audio frames
+        are packed in 2s block. When we pack the samples with discontinuities, those
+        discontinuities would all be accumulated at the 2s boundary which makes them
+        much more audible.
+        The CMSampleBufferCreateReady API should allow us to pack samples with
+        discontinuities as we can give a vector of CMSampleTimingInfo with the
+        exact information for all packets.
+        However, this data appears to be ignored and the discontinuities is still
+        heard at the 2s boundary.
+        So we no longer pack samples with discontinuities so that the frame
+        timestamps will be more accurate and no audible artefacts are heard on
+        small gaps.
+
+        Manually tested and verified manually. This is getting around an issue
+        in CoreMedia that inserts very audible artifacts when there's a gap between
+        samples.
+
+        * platform/graphics/cocoa/SourceBufferParserWebM.cpp:
+        (WebCore::SourceBufferParserWebM::AudioTrackData::resetCompleted):
+        (WebCore::SourceBufferParserWebM::AudioTrackData::consumeFrameData):
+        (WebCore::SourceBufferParserWebM::AudioTrackData::createSampleBuffer):
+        * platform/graphics/cocoa/SourceBufferParserWebM.h:
+
 2021-12-19  Alan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp

@@ -1374 +1374 @@
 {
     mNumFramesInCompleteBlock = 0;
-    m_packetDescriptions.clear();
+    m_packetSizes.clear();
+    m_packetTimings.clear();
     m_currentPacketByteOffset = 0;
     TrackData::resetCompleted();
@@ -1381 +1382 @@
 webm::Status SourceBufferParserWebM::AudioTrackData::consumeFrameData(webm::Reader& reader, const FrameMetadata& metadata, uint64_t* bytesRemaining, const CMTime& presentationTime, int sampleCount)
 {
+    if (m_packetTimings.size()) {
+        auto& lastTiming = m_packetTimings.last();
+        if (PAL::CMTimeCompare(PAL::CMTimeAdd(lastTiming.duration, lastTiming.presentationTimeStamp), presentationTime)) {
+            // Discontinuity encountered, emit the previously demuxed samples.
+            createSampleBuffer(metadata.position);
+            reset();
+        }
+    }
+
     auto status = readFrameData(reader, metadata, bytesRemaining);
     if (!status.completed_ok())
@@ -1390 +1400 @@
         mMaxBlockBufferCapacity = mNumFramesInCompleteBlock;
 
-    if (m_packetDescriptions.isEmpty())
+    if (m_packetSizes.isEmpty())
         m_samplePresentationTime = presentationTime;
 
@@ -1467 +1477 @@
     }
 
-    m_packetDescriptions.append({ static_cast<int64_t>(m_currentPacketByteOffset), 0, static_cast<UInt32>(*m_completePacketSize) });
+    m_packetSizes.append(*m_completePacketSize);
+    m_packetTimings.append({ m_packetDuration, presentationTime, PAL::kCMTimeInvalid });
     m_currentPacketByteOffset += *m_completePacketSize;
     m_completePacketSize = std::nullopt;
 
     auto sampleDuration = PAL::CMTimeGetSeconds(PAL::CMTimeSubtract(presentationTime, m_samplePresentationTime)) + PAL::CMTimeGetSeconds(m_packetDuration) * sampleCount;
+
     if (sampleDuration >= m_minimumSampleDuration) {
         createSampleBuffer(metadata.position);
@@ -1483 +1495 @@
 void SourceBufferParserWebM::AudioTrackData::createSampleBuffer(std::optional<size_t> latestByteRangeOffset)
 {
-    if (m_packetDescriptions.isEmpty())
+    if (m_packetSizes.isEmpty())
         return;
 
     CMSampleBufferRef rawSampleBuffer = nullptr;
-    auto err = PAL::CMAudioSampleBufferCreateReadyWithPacketDescriptions(kCFAllocatorDefault, m_completeBlockBuffer.get(), formatDescription().get(), m_packetDescriptions.size(), m_samplePresentationTime, m_packetDescriptions.data(), &rawSampleBuffer);
+    auto err = PAL::CMSampleBufferCreateReady(kCFAllocatorDefault, m_completeBlockBuffer.get(), formatDescription().get(), m_packetSizes.size(), m_packetTimings.size(), m_packetTimings.data(), m_packetSizes.size(), m_packetSizes.data(), &rawSampleBuffer);
     if (err) {
         PARSER_LOG_ERROR_IF_POSSIBLE("CMAudioSampleBufferCreateWithPacketDescriptions failed with %d", err);

trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h

@@ -235 +235 @@
         uint8_t m_framesPerPacket { 0 };
         Seconds m_frameDuration { 0_s };
-        Vector<AudioStreamPacketDescription> m_packetDescriptions;
+        Vector<size_t> m_packetSizes;
+        Vector<CMSampleTimingInfo> m_packetTimings;
         size_t mNumFramesInCompleteBlock { 0 };
         // FIXME: 0.5 - 1.0 seconds is a better duration per sample buffer, but use 2 seconds so at least the first