MediaRecorder.stop() fires an additional dataavailable event with byt…

…es after MediaRecorder.pause() https://bugs.webkit.org/show_bug.cgi?id=243837 rdar://problem/98565949 Reviewed by Eric Carlson. When fetching recorded data, make sure to flush encoders to get all possible data. This allows to ensure that we get as much data as possible when requestData is called. This aligns with other browsers behavior. Covered by added test. * LayoutTests/http/wpt/mediarecorder/pause-recording-expected.txt: * LayoutTests/http/wpt/mediarecorder/pause-recording.html: * Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h: (WebCore::AudioSampleBufferCompressor::finish): (WebCore::AudioSampleBufferCompressor::flush): * Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm: (WebCore::AudioSampleBufferCompressor::flushInternal): (WebCore::AudioSampleBufferCompressor::finish): Deleted. * Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm: (WebCore::MediaRecorderPrivateWriter::fetchData): * Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.h: (WebCore::VideoSampleBufferCompressor::finish): (WebCore::VideoSampleBufferCompressor::flush): * Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm: (WebCore::VideoSampleBufferCompressor::flushInternal): (WebCore::VideoSampleBufferCompressor::finish): Deleted. Canonical link: https://commits.webkit.org/253529@main
WebKit · Aug 17, 2022 · f296787 · f296787
1 parent f76a7e3
commit f296787
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 15 deletions.
diff --git a/LayoutTests/http/wpt/mediarecorder/pause-recording-expected.txt b/LayoutTests/http/wpt/mediarecorder/pause-recording-expected.txt
@@ -2,4 +2,5 @@
 
 PASS Pausing and resuming the recording should impact the video duration
 PASS Calling requestData once after pausing should lead to more than header data
+PASS Once paused, the second requestData call should lead to a zero size blob
 
diff --git a/LayoutTests/http/wpt/mediarecorder/pause-recording.html b/LayoutTests/http/wpt/mediarecorder/pause-recording.html
@@ -62,7 +62,35 @@
     assert_greater_than(blob.size, 1400);
 }, "Calling requestData once after pausing should lead to more than header data");
 
+promise_test(async (test) => {
+    const stream = await navigator.mediaDevices.getUserMedia({ audio: true, video: true });
+
+    const recorder = new MediaRecorder(stream);
+    let dataPromise = new Promise(resolve => recorder.ondataavailable = (e) => resolve(e.data));
+
+    const startPromise = new Promise(resolve => recorder.onstart = resolve);
+    recorder.start();
+    await startPromise;
+
+    await waitFor(1000);
+
+    const pausePromise = new Promise(resolve => recorder.onpause = resolve);
+    recorder.pause();
+
+    recorder.requestData();
+
+    await pausePromise;
+    const blobWithData = await dataPromise;
+    assert_greater_than(blobWithData.size, 0);
+
+    recorder.requestData();
+    const blobWithNoDataPromise = new Promise(resolve => recorder.ondataavailable = (e) => resolve(e.data));
+    await waitFor(100);
+
+    const blobWithNoData = await blobWithNoDataPromise;
+    assert_equals(blobWithNoData.size, 0);
 
+}, "Once paused, the second requestData call should lead to a zero size blob");
     </script>
 </body>
 </html>
diff --git a/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h b/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h
@@ -41,7 +41,8 @@ class AudioSampleBufferCompressor {
     ~AudioSampleBufferCompressor();
 
     void setBitsPerSecond(unsigned);
-    void finish();
+    void finish() { flushInternal(true); }
+    void flush() { flushInternal(false); }
     void addSampleBuffer(CMSampleBufferRef);
     CMSampleBufferRef getOutputSampleBuffer();
     RetainPtr<CMSampleBufferRef> takeOutputSampleBuffer();
@@ -63,6 +64,7 @@ class AudioSampleBufferCompressor {
     RetainPtr<CMSampleBufferRef> sampleBufferWithNumPackets(UInt32 numPackets, AudioBufferList);
     void processSampleBuffersUntilLowWaterTime(CMTime);
     OSStatus provideSourceDataNumOutputPackets(UInt32*, AudioBufferList*, AudioStreamPacketDescription**);
+    void flushInternal(bool isFinished);
 
     Ref<WorkQueue> m_serialDispatchQueue;
     CMTime m_lowWaterTime { kCMTimeInvalid };

diff --git a/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm b/Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm
@@ -85,10 +85,14 @@
     return true;
 }
 
-void AudioSampleBufferCompressor::finish()
+void AudioSampleBufferCompressor::flushInternal(bool isFinished)
 {
-    m_serialDispatchQueue->dispatchSync([this] {
+    m_serialDispatchQueue->dispatchSync([this, isFinished] {
         processSampleBuffersUntilLowWaterTime(PAL::kCMTimeInvalid);
+
+        if (!isFinished)
+            return;
+
         auto error = PAL::CMBufferQueueMarkEndOfData(m_outputBufferQueue.get());
         RELEASE_LOG_ERROR_IF(error, MediaStream, "AudioSampleBufferCompressor CMBufferQueueMarkEndOfData failed %d", error);
         m_isEncoding = false;

diff --git a/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm b/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm
@@ -485,17 +485,25 @@ static inline void appendEndsPreviousSampleDurationMarker(AVAssetWriterInput *as
         return;
     }
 
-    flushCompressedSampleBuffers([weakThis = WeakPtr { *this }]() mutable {
-        if (!weakThis)
-            return;
+    if (m_videoCompressor)
+        m_videoCompressor->flush();
+    if (m_audioCompressor)
+        m_audioCompressor->flush();
+
+    // We hop to the main thread since flushing the video compressor might trigger starting the writer asynchronously.
+    callOnMainThread([this, weakThis = WeakPtr { *this }]() mutable {
+        flushCompressedSampleBuffers([weakThis = WTFMove(weakThis)]() mutable {
+            if (!weakThis)
+                return;
 
-        ALLOW_DEPRECATED_DECLARATIONS_BEGIN
-        [weakThis->m_writer flush];
-        ALLOW_DEPRECATED_DECLARATIONS_END
+            ALLOW_DEPRECATED_DECLARATIONS_BEGIN
+            [weakThis->m_writer flush];
+            ALLOW_DEPRECATED_DECLARATIONS_END
 
-        callOnMainThread([weakThis = WTFMove(weakThis)] {
-            if (weakThis)
-                weakThis->completeFetchData();
+            callOnMainThread([weakThis = WTFMove(weakThis)] {
+                if (weakThis)
+                    weakThis->completeFetchData();
+            });
         });
     });
 }

diff --git a/Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.h b/Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.h
@@ -42,7 +42,8 @@ class VideoSampleBufferCompressor {
     ~VideoSampleBufferCompressor();
 
     void setBitsPerSecond(unsigned);
-    void finish();
+    void finish() { flushInternal(true); }
+    void flush() { flushInternal(false); }
     void addSampleBuffer(CMSampleBufferRef);
     CMSampleBufferRef getOutputSampleBuffer();
     RetainPtr<CMSampleBufferRef> takeOutputSampleBuffer();
@@ -58,6 +59,7 @@ class VideoSampleBufferCompressor {
     void processSampleBuffer(CMSampleBufferRef);
     bool initCompressionSession(CMVideoFormatDescriptionRef);
     CFStringRef vtProfileLevel() const;
+    void flushInternal(bool isFinished);
 
     static void videoCompressionCallback(void *refCon, void*, OSStatus, VTEncodeInfoFlags, CMSampleBufferRef);
 

diff --git a/Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm b/Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm
@@ -92,12 +92,15 @@
     m_outputBitRate = bitRate;
 }
 
-void VideoSampleBufferCompressor::finish()
+void VideoSampleBufferCompressor::flushInternal(bool isFinished)
 {
-    m_serialDispatchQueue->dispatchSync([this] {
+    m_serialDispatchQueue->dispatchSync([this, isFinished] {
         auto error = PAL::VTCompressionSessionCompleteFrames(m_vtSession.get(), PAL::kCMTimeInvalid);
         RELEASE_LOG_ERROR_IF(error, MediaStream, "VideoSampleBufferCompressor VTCompressionSessionCompleteFrames failed with %d", error);
 
+        if (!isFinished)
+            return;
+
         error = PAL::CMBufferQueueMarkEndOfData(m_outputBufferQueue.get());
         RELEASE_LOG_ERROR_IF(error, MediaStream, "VideoSampleBufferCompressor CMBufferQueueMarkEndOfData failed with %d", error);