Changeset 267521 in webkit

Timestamp:

Sep 24, 2020, 12:10:26 AM (6 years ago)

Author:

youenn@apple.com

Message:

REGRESSION (iOS/Safari 14): MediaRecorder produces invalid video files
​https://bugs.webkit.org/show_bug.cgi?id=216832
<rdar://problem/69377550>

Reviewed by Eric Carlson.

Start the audio and video timestamps at zero.
Compute the audio timestamp based on the sample count and the video timestamp based
on the time at which the video sample is received.

Covered by manually testing Safari generated videos on VLC, Chrome and Firefox.

• platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h:
• platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:

(WebCore::MediaRecorderPrivateWriter::initialize):
(WebCore::MediaRecorderPrivateWriter::startAssetWriter):
(WebCore::copySampleBufferWithCurrentTimeStamp):
(WebCore::MediaRecorderPrivateWriter::appendVideoSampleBuffer):
(WebCore::createAudioSampleBuffer):
(WebCore::MediaRecorderPrivateWriter::appendAudioSampleBuffer):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h (modified) (1 diff)
• platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-09-24  Youenn Fablet  <youenn@apple.com>
+
+        REGRESSION (iOS/Safari 14): MediaRecorder produces invalid video files
+        https://bugs.webkit.org/show_bug.cgi?id=216832
+        <rdar://problem/69377550>
+
+        Reviewed by Eric Carlson.
+
+        Start the audio and video timestamps at zero.
+        Compute the audio timestamp based on the sample count and the video timestamp based
+        on the time at which the video sample is received.
+
+        Covered by manually testing Safari generated videos on VLC, Chrome and Firefox.
+
+        * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h:
+        * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
+        (WebCore::MediaRecorderPrivateWriter::initialize):
+        (WebCore::MediaRecorderPrivateWriter::startAssetWriter):
+        (WebCore::copySampleBufferWithCurrentTimeStamp):
+        (WebCore::MediaRecorderPrivateWriter::appendVideoSampleBuffer):
+        (WebCore::createAudioSampleBuffer):
+        (WebCore::MediaRecorderPrivateWriter::appendAudioSampleBuffer):
+
 2020-09-23  Peng Liu  <peng.liu6@apple.com>
 

trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.h

@@ -132 +132 @@
     bool m_shouldStopAfterFlushingSamples { false };
     bool m_firstVideoFrame { false };
+    CMTime m_firstVideoSampleTime { kCMTimeZero };
+    CMTime m_currentAudioSampleTime { kCMTimeZero };
 };
 

trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm

@@ -400 +400 @@
 
 
-static inline RetainPtr<CMSampleBufferRef> copySampleBufferWithCurrentTimeStamp(CMSampleBufferRef originalBuffer)
-{
-    CMTime startTime = CMClockGetTime(CMClockGetHostTimeClock());
+static inline RetainPtr<CMSampleBufferRef> copySampleBufferWithCurrentTimeStamp(CMSampleBufferRef originalBuffer, CMTime startTime)
+{
     CMItemCount count = 0;
     CMSampleBufferGetSampleTimingInfoArray(originalBuffer, 0, nil, &count);
@@ -426 +425 @@
     if (!m_firstVideoFrame) {
         m_firstVideoFrame = true;
+        m_firstVideoSampleTime = CMClockGetTime(CMClockGetHostTimeClock());
         if (sample.videoRotation() != MediaSample::VideoRotation::None || sample.videoMirrored()) {
             auto videoTransform = CGAffineTransformMakeRotation(static_cast<int>(sample.videoRotation()) * M_PI / 180);
@@ -433 +433 @@
         }
     }
-    // FIXME: We should not set the timestamps if they are already set.
-    if (auto bufferWithCurrentTime = copySampleBufferWithCurrentTimeStamp(sample.platformSample().sample.cmSampleBuffer))
+
+    CMTime sampleTime = CMTimeSubtract(CMClockGetTime(CMClockGetHostTimeClock()), m_firstVideoSampleTime);
+    if (auto bufferWithCurrentTime = copySampleBufferWithCurrentTimeStamp(sample.platformSample().sample.cmSampleBuffer, sampleTime))
         m_videoCompressor->addSampleBuffer(bufferWithCurrentTime.get());
 }
@@ -450 +451 @@
 }
 
-static inline RetainPtr<CMSampleBufferRef> createAudioSampleBuffer(const PlatformAudioData& data, const AudioStreamDescription& description, const WTF::MediaTime& time, size_t sampleCount)
+static inline RetainPtr<CMSampleBufferRef> createAudioSampleBuffer(const PlatformAudioData& data, const AudioStreamDescription& description, CMTime time, size_t sampleCount)
 {
     auto format = createAudioFormatDescription(description);
@@ -457 +458 @@
 
     CMSampleBufferRef sampleBuffer = nullptr;
-    auto error = CMAudioSampleBufferCreateWithPacketDescriptions(kCFAllocatorDefault, NULL, false, NULL, NULL, format.get(), sampleCount, toCMTime(time), NULL, &sampleBuffer);
+    auto error = CMAudioSampleBufferCreateWithPacketDescriptions(kCFAllocatorDefault, NULL, false, NULL, NULL, format.get(), sampleCount, time, NULL, &sampleBuffer);
     if (error) {
         RELEASE_LOG_ERROR(MediaStream, "MediaRecorderPrivateWriter createAudioSampleBufferWithPacketDescriptions failed with %d", error);
@@ -472 +473 @@
 }
 
-void MediaRecorderPrivateWriter::appendAudioSampleBuffer(const PlatformAudioData& data, const AudioStreamDescription& description, const WTF::MediaTime& time, size_t sampleCount)
-{
-    if (auto sampleBuffer = createAudioSampleBuffer(data, description, time, sampleCount))
+void MediaRecorderPrivateWriter::appendAudioSampleBuffer(const PlatformAudioData& data, const AudioStreamDescription& description, const WTF::MediaTime&, size_t sampleCount)
+{
+    if (auto sampleBuffer = createAudioSampleBuffer(data, description, m_currentAudioSampleTime, sampleCount))
         m_audioCompressor->addSampleBuffer(sampleBuffer.get());
+    m_currentAudioSampleTime = CMTimeAdd(m_currentAudioSampleTime, toCMTime(MediaTime(sampleCount, description.sampleRate())));
 }