display:none HTMLMediaElement with a camera track as source is leakin…

…g video frames https://bugs.webkit.org/show_bug.cgi?id=242939 rdar://96677785 Reviewed by Eric Carlson. AVSampleBufferDisplayLayer is keeping CVPixelBuffers when it cannot display them, which happens on iOS in case HTMLMediaElement is display:none. In that case, the camera buffer pool might get exhausted and this will fail camera capture. We fixed this by making sure to not enqueue frames in that case, but this got broken in d126b89. We reintroduce a specific callback renderVideoWillBeDestroyed so that only MediaPlayerPrivateMediaStreamAVFObjC is changed. To make our mock camera closer to actual cameras, we create CVPixelBuffer from a pool with kCVPixelBufferPoolAllocationThresholdKey. This should allow to test CVPixelBuffer camera leaks. Update LayoutTests/fast/mediastream/resources/getUserMedia-to-canvas.js to make sure we do not keep a VideoFrame for each video element as otherwise, we can run out of camera frames. * LayoutTests/fast/mediastream/getUserMedia-media-element-display-none-expected.txt: Added. * LayoutTests/fast/mediastream/getUserMedia-media-element-display-none.html: Added. * LayoutTests/fast/mediastream/resources/getUserMedia-to-canvas.js: (createUserMediaToCanvasTests): * Source/WebCore/platform/cocoa/CoreVideoSoftLink.cpp: * Source/WebCore/platform/cocoa/CoreVideoSoftLink.h: * Source/WebCore/platform/graphics/MediaPlayer.cpp: (WebCore::MediaPlayer::renderVideoWillBeDestroyed): * Source/WebCore/platform/graphics/MediaPlayer.h: * Source/WebCore/platform/graphics/MediaPlayerPrivate.h: (WebCore::MediaPlayerPrivateInterface::renderVideoWillBeDestroyed): * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h: * Source/WebCore/platform/graphics/cv/CVUtilities.h: * Source/WebCore/platform/graphics/cv/CVUtilities.mm: (WebCore::createCVPixelBufferFromPool): * Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h: (WebCore::ImageTransferSessionVT::setMaximumBufferPoolSize): * Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm: (WebCore::ImageTransferSessionVT::convertPixelBuffer): * Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.h: * Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm: (WebCore::MockRealtimeVideoSourceMac::updateSampleBuffer): * Source/WebCore/rendering/RenderVideo.cpp: (WebCore::RenderVideo::willBeDestroyed): Canonical link: https://commits.webkit.org/252871@main
WebKit · Jul 27, 2022 · 3d75fe4 · 3d75fe4
1 parent f5950ae
commit 3d75fe4
Show file tree

Hide file tree

Showing 16 changed files with 68 additions and 6 deletions.
diff --git a/LayoutTests/fast/mediastream/getUserMedia-media-element-display-none-expected.txt b/LayoutTests/fast/mediastream/getUserMedia-media-element-display-none-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Media element should not store video frames in display=none case
+
diff --git a/LayoutTests/fast/mediastream/getUserMedia-media-element-display-none.html b/LayoutTests/fast/mediastream/getUserMedia-media-element-display-none.html
@@ -0,0 +1,22 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <script src="../../resources/testharness.js"></script>
+        <script src="../../resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <video id="video" autoplay></video>
+        <script>
+promise_test(async (test) => {
+    video.srcObject = await navigator.mediaDevices.getUserMedia({ video: true });
+    await video.play();
+    video.style.display = 'none';
+    return new Promise((resolve, reject) => {
+        video.srcObject.getVideoTracks()[0].onended = () => reject('capture failed');
+        setTimeout(resolve, 3000);
+    });
+}, "Media element should not store video frames in display=none case");
+        </script>
+    </body>
+</html>
diff --git a/LayoutTests/fast/mediastream/resources/getUserMedia-to-canvas.js b/LayoutTests/fast/mediastream/resources/getUserMedia-to-canvas.js
@@ -141,6 +141,7 @@ async function testUserMediaToCanvas(t, subcase) {
         setMockCameraImageOrientation(0);
         await waitForVideoSize(video, realVideoSize[0], realVideoSize[1]);
         debuge.removeChild(video);
+        video.srcObject = null;
     });
 
     if (subcase.angle == 180) {

diff --git a/Source/WebCore/platform/cocoa/CoreVideoSoftLink.cpp b/Source/WebCore/platform/cocoa/CoreVideoSoftLink.cpp
@@ -52,6 +52,7 @@ SOFT_LINK_FUNCTION_FOR_SOURCE(WebCore, CoreVideo, CVPixelBufferLockBaseAddress,
 SOFT_LINK_FUNCTION_FOR_SOURCE(WebCore, CoreVideo, CVPixelBufferUnlockBaseAddress, CVReturn, (CVPixelBufferRef pixelBuffer, CVOptionFlags lockFlags), (pixelBuffer, lockFlags))
 SOFT_LINK_FUNCTION_FOR_SOURCE(WebCore, CoreVideo, CVPixelBufferPoolCreate, CVReturn,(CFAllocatorRef allocator, CFDictionaryRef poolAttributes, CFDictionaryRef pixelBufferAttributes, CVPixelBufferPoolRef* poolOut), (allocator, poolAttributes, pixelBufferAttributes, poolOut))
 SOFT_LINK_FUNCTION_FOR_SOURCE(WebCore, CoreVideo, CVPixelBufferPoolCreatePixelBuffer, CVReturn, (CFAllocatorRef allocator, CVPixelBufferPoolRef pixelBufferPool, CVPixelBufferRef* pixelBufferOut), (allocator, pixelBufferPool, pixelBufferOut))
+SOFT_LINK_FUNCTION_FOR_SOURCE(WebCore, CoreVideo, CVPixelBufferPoolCreatePixelBufferWithAuxAttributes, CVReturn, (CFAllocatorRef allocator, CVPixelBufferPoolRef pixelBufferPool, CFDictionaryRef auxiliaryAttributes, CVPixelBufferRef* pixelBufferOut), (allocator, pixelBufferPool, auxiliaryAttributes, pixelBufferOut))
 SOFT_LINK_FUNCTION_FOR_SOURCE_WITH_EXPORT(WebCore, CoreVideo, CVPixelBufferGetIOSurface, IOSurfaceRef, (CVPixelBufferRef pixelBuffer), (pixelBuffer), WEBCORE_EXPORT)
 SOFT_LINK_FUNCTION_FOR_SOURCE(WebCore, CoreVideo, CVImageBufferCreateColorSpaceFromAttachments, CGColorSpaceRef, (CFDictionaryRef attachments), (attachments))
 
@@ -65,6 +66,7 @@ SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVPixelFormatOpenGLCompatibil
 SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVPixelFormatOpenGLESCompatibility, CFStringRef)
 SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVPixelBufferIOSurfacePropertiesKey, CFStringRef)
 SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVPixelBufferPoolMinimumBufferCountKey, CFStringRef)
+SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVPixelBufferPoolAllocationThresholdKey, CFStringRef)
 SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVImageBufferYCbCrMatrixKey, CFStringRef)
 SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVImageBufferYCbCrMatrix_ITU_R_709_2, CFStringRef)
 SOFT_LINK_CONSTANT_FOR_SOURCE(WebCore, CoreVideo, kCVImageBufferYCbCrMatrix_ITU_R_601_4, CFStringRef)

diff --git a/Source/WebCore/platform/cocoa/CoreVideoSoftLink.h b/Source/WebCore/platform/cocoa/CoreVideoSoftLink.h
@@ -70,6 +70,8 @@ SOFT_LINK_FUNCTION_FOR_HEADER(WebCore, CoreVideo, CVPixelBufferPoolCreate, CVRet
 #define CVPixelBufferPoolCreate softLink_CoreVideo_CVPixelBufferPoolCreate
 SOFT_LINK_FUNCTION_FOR_HEADER(WebCore, CoreVideo, CVPixelBufferPoolCreatePixelBuffer, CVReturn, (CFAllocatorRef allocator, CVPixelBufferPoolRef pixelBufferPool, CVPixelBufferRef* pixelBufferOut), (allocator, pixelBufferPool, pixelBufferOut))
 #define CVPixelBufferPoolCreatePixelBuffer softLink_CoreVideo_CVPixelBufferPoolCreatePixelBuffer
+SOFT_LINK_FUNCTION_FOR_HEADER(WebCore, CoreVideo, CVPixelBufferPoolCreatePixelBufferWithAuxAttributes, CVReturn, (CFAllocatorRef allocator, CVPixelBufferPoolRef pixelBufferPool, CFDictionaryRef auxiliaryAttributes, CVPixelBufferRef* pixelBufferOut), (allocator, pixelBufferPool, auxiliaryAttributes, pixelBufferOut))
+#define CVPixelBufferPoolCreatePixelBufferWithAuxAttributes softLink_CoreVideo_CVPixelBufferPoolCreatePixelBufferWithAuxAttributes
 SOFT_LINK_FUNCTION_FOR_HEADER(WebCore, CoreVideo, CVPixelBufferGetIOSurface, IOSurfaceRef, (CVPixelBufferRef pixelBuffer), (pixelBuffer))
 #define CVPixelBufferGetIOSurface softLink_CoreVideo_CVPixelBufferGetIOSurface
 SOFT_LINK_FUNCTION_FOR_HEADER(WebCore, CoreVideo, CVPixelBufferPoolGetPixelBufferAttributes, CFDictionaryRef, (CVPixelBufferPoolRef pool), (pool))
@@ -91,6 +93,8 @@ SOFT_LINK_CONSTANT_FOR_HEADER(WebCore, CoreVideo, kCVPixelBufferIOSurfacePropert
 #define kCVPixelBufferIOSurfacePropertiesKey get_CoreVideo_kCVPixelBufferIOSurfacePropertiesKey()
 SOFT_LINK_CONSTANT_FOR_HEADER(WebCore, CoreVideo, kCVPixelBufferPoolMinimumBufferCountKey, CFStringRef)
 #define kCVPixelBufferPoolMinimumBufferCountKey get_CoreVideo_kCVPixelBufferPoolMinimumBufferCountKey()
+SOFT_LINK_CONSTANT_FOR_HEADER(WebCore, CoreVideo, kCVPixelBufferPoolAllocationThresholdKey, CFStringRef)
+#define kCVPixelBufferPoolAllocationThresholdKey get_CoreVideo_kCVPixelBufferPoolAllocationThresholdKey()
 SOFT_LINK_CONSTANT_FOR_HEADER(WebCore, CoreVideo, kCVImageBufferYCbCrMatrixKey, CFStringRef)
 #define kCVImageBufferYCbCrMatrixKey get_CoreVideo_kCVImageBufferYCbCrMatrixKey()
 SOFT_LINK_CONSTANT_FOR_HEADER(WebCore, CoreVideo, kCVImageBufferYCbCrMatrix_ITU_R_709_2, CFStringRef)

diff --git a/Source/WebCore/platform/graphics/MediaPlayer.cpp b/Source/WebCore/platform/graphics/MediaPlayer.cpp
@@ -1817,6 +1817,11 @@ void MediaPlayer::stopVideoFrameMetadataGathering()
     m_private->stopVideoFrameMetadataGathering();
 }
 
+void MediaPlayer::renderVideoWillBeDestroyed()
+{
+    m_private->renderVideoWillBeDestroyed();
+}
+
 void MediaPlayer::playerContentBoxRectChanged(const LayoutRect& rect)
 {
     m_private->playerContentBoxRectChanged(rect);

diff --git a/Source/WebCore/platform/graphics/MediaPlayer.h b/Source/WebCore/platform/graphics/MediaPlayer.h
@@ -710,6 +710,8 @@ class WEBCORE_EXPORT MediaPlayer : public MediaPlayerEnums, public ThreadSafeRef
 
     bool prefersSandboxedParsing() const { return client().mediaPlayerPrefersSandboxedParsing(); }
 
+    void renderVideoWillBeDestroyed();
+
 private:
     MediaPlayer(MediaPlayerClient&);
     MediaPlayer(MediaPlayerClient&, MediaPlayerEnums::MediaEngineIdentifier);

diff --git a/Source/WebCore/platform/graphics/MediaPlayerPrivate.h b/Source/WebCore/platform/graphics/MediaPlayerPrivate.h
@@ -336,6 +336,8 @@ class MediaPlayerPrivateInterface {
     virtual void setResourceOwner(const ProcessIdentity&) { }
 
     virtual String errorMessage() const { return { }; }
+
+    virtual void renderVideoWillBeDestroyed() { }
 };
 
 }

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h
@@ -168,6 +168,7 @@ class MediaPlayerPrivateMediaStreamAVFObjC final
     void audioOutputDeviceChanged() final;
     std::optional<VideoFrameMetadata> videoFrameMetadata() final;
     void setResourceOwner(const ProcessIdentity&) final { ASSERT_NOT_REACHED(); }
+    void renderVideoWillBeDestroyed() final { destroyLayers(); }
 
     MediaPlayer::ReadyState currentReadyState();
     void updateReadyState();

diff --git a/Source/WebCore/platform/graphics/cv/CVUtilities.h b/Source/WebCore/platform/graphics/cv/CVUtilities.h
@@ -45,7 +45,7 @@ WEBCORE_EXPORT Expected<RetainPtr<CVPixelBufferPoolRef>, CVReturn> createInMemor
 
 WEBCORE_EXPORT Expected<RetainPtr<CVPixelBufferPoolRef>, CVReturn> createCVPixelBufferPool(size_t width, size_t height, OSType pixelFormat, unsigned minimumBufferCount = 0u, bool isCGImageCompatible = false, bool shouldUseIOSUrface = true);
 
-WEBCORE_EXPORT Expected<RetainPtr<CVPixelBufferRef>, CVReturn> createCVPixelBufferFromPool(CVPixelBufferPoolRef);
+WEBCORE_EXPORT Expected<RetainPtr<CVPixelBufferRef>, CVReturn> createCVPixelBufferFromPool(CVPixelBufferPoolRef, unsigned maximumBufferCount = 0u);
 
 WEBCORE_EXPORT Expected<RetainPtr<CVPixelBufferRef>, CVReturn> createCVPixelBuffer(IOSurfaceRef);
 

diff --git a/Source/WebCore/platform/graphics/cv/CVUtilities.mm b/Source/WebCore/platform/graphics/cv/CVUtilities.mm
@@ -83,10 +83,16 @@
     return shouldUseIOSurfacePool ? createIOSurfaceCVPixelBufferPool(width, height, format, minimumBufferCount, isCGImageCompatible) : createInMemoryCVPixelBufferPool(width, height, format, minimumBufferCount, isCGImageCompatible);
 }
 
-Expected<RetainPtr<CVPixelBufferRef>, CVReturn> createCVPixelBufferFromPool(CVPixelBufferPoolRef pixelBufferPool)
+Expected<RetainPtr<CVPixelBufferRef>, CVReturn> createCVPixelBufferFromPool(CVPixelBufferPoolRef pixelBufferPool, unsigned maxBufferSize)
 {
     CVPixelBufferRef pixelBuffer = nullptr;
-    auto status = CVPixelBufferPoolCreatePixelBuffer(kCFAllocatorDefault, pixelBufferPool, &pixelBuffer);
+    CVReturn status;
+    if (!maxBufferSize)
+        status = CVPixelBufferPoolCreatePixelBuffer(kCFAllocatorDefault, pixelBufferPool, &pixelBuffer);
+    else {
+        auto *auxiliaryAttributes = @{ (__bridge NSString *)kCVPixelBufferPoolAllocationThresholdKey : @(maxBufferSize) };
+        status = CVPixelBufferPoolCreatePixelBufferWithAuxAttributes(kCFAllocatorDefault, pixelBufferPool, (__bridge CFDictionaryRef)auxiliaryAttributes, &pixelBuffer);
+    }
     if (status != kCVReturnSuccess || !pixelBuffer)
         return makeUnexpected(status);
     return adoptCF(pixelBuffer);

diff --git a/Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h b/Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h
@@ -54,6 +54,7 @@ class ImageTransferSessionVT {
 #endif
 
     uint32_t pixelFormat() const { return m_pixelFormat; }
+    void setMaximumBufferPoolSize(size_t maxBufferPoolSize) { m_maxBufferPoolSize = maxBufferPoolSize; }
 
 private:
     WEBCORE_EXPORT ImageTransferSessionVT(uint32_t pixelFormat, bool shouldUseIOSurface);
@@ -77,6 +78,7 @@ class ImageTransferSessionVT {
     bool m_shouldUseIOSurface { true };
     uint32_t m_pixelFormat;
     IntSize m_size;
+    size_t m_maxBufferPoolSize { 0 };
 };
 
 }
diff --git a/Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm b/Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm
@@ -93,7 +93,7 @@
     if (!sourceBuffer || !setSize(size))
         return nullptr;
 
-    auto result = createCVPixelBufferFromPool(m_outputBufferPool.get());
+    auto result = createCVPixelBufferFromPool(m_outputBufferPool.get(), m_maxBufferPoolSize);
     if (!result) {
         RELEASE_LOG(Media, "ImageTransferSessionVT::convertPixelBuffer, createCVPixelBufferFromPool failed with error %d", static_cast<int>(result.error()));
         return nullptr;

diff --git a/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.h b/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.h
@@ -62,6 +62,7 @@ class MockRealtimeVideoSourceMac final : public MockRealtimeVideoSource {
     std::unique_ptr<ImageTransferSessionVT> m_imageTransferSession;
     IntSize m_presetSize;
     Ref<WorkQueue> m_workQueue;
+    size_t m_pixelGenerationFailureCount { 0 };
 };
 
 } // namespace WebCore

diff --git a/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm b/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm
@@ -86,18 +86,25 @@
     if (!imageBuffer)
         return;
 
-    if (!m_imageTransferSession)
+    if (!m_imageTransferSession) {
         m_imageTransferSession = ImageTransferSessionVT::create(preferedPixelBufferFormat());
+        m_imageTransferSession->setMaximumBufferPoolSize(10);
+    }
 
     PlatformImagePtr platformImage;
     if (auto nativeImage = imageBuffer->copyImage()->nativeImage())
         platformImage = nativeImage->platformImage();
 
     auto presentationTime = MediaTime::createWithDouble((elapsedTime() + 100_ms).seconds());
     auto videoFrame = m_imageTransferSession->createVideoFrame(platformImage.get(), presentationTime, size(), videoFrameRotation());
-    if (!videoFrame)
+    if (!videoFrame) {
+        static const size_t MaxPixelGenerationFailureCount = 30;
+        if (++m_pixelGenerationFailureCount > MaxPixelGenerationFailureCount)
+            captureFailed();
         return;
+    }
 
+    m_pixelGenerationFailureCount = 0;
     auto captureTime = MonotonicTime::now().secondsSinceEpoch();
     m_workQueue->dispatch([this, protectedThis = Ref { *this }, videoFrame = WTFMove(videoFrame), captureTime]() mutable {
         VideoFrameTimeMetadata metadata;

diff --git a/Source/WebCore/rendering/RenderVideo.cpp b/Source/WebCore/rendering/RenderVideo.cpp
@@ -65,6 +65,10 @@ RenderVideo::~RenderVideo()
 void RenderVideo::willBeDestroyed()
 {
     visibleInViewportStateChanged();
+
+    if (auto player = videoElement().player())
+        player->renderVideoWillBeDestroyed();
+
     RenderMedia::willBeDestroyed();
 }