Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Captured video frames are leaked if Web process media player is destroyed before message is received #33453

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kkinnunen-apple
Copy link
Contributor

@kkinnunen-apple kkinnunen-apple commented Sep 11, 2024

3da3dca

Captured video frames are leaked if Web process media player is destroyed before message is received
https://bugs.webkit.org/show_bug.cgi?id=279671
rdar://problem/135948989

Reviewed by NOBODY (OOPS!).

GPU process would put the captured VideoFrame to the video frame heap
and send the reference to the WebContent process media player. If
the MediaPlayerPrivateRemote was destroyed while the message was in transit,
the message would be ignored and the video frame would leak in the
heap.

Make the GPUP -> WCP video frame offer messages asynchronous with
replies. Since asynchronous message replies are always delivered, this
fixes the memory leak. Remove the video frame from the heap if
the frame was not adopted.

This concerns following video frame offer messages:
    Messages::MediaPlayerPriveteRemote::PushVideoFrameMetadata
    Messages::LibWebRTCCodecs::CompletedDecoding
    Messages::RemoteCaptureSampleManager::videoFrameAvailable

* Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp:
(WebKit::RemoteVideoFrameObjectHeap::addWithCompletion):
(WebKit::RemoteVideoFrameObjectHeap::add):
* Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h:
* Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm:
(WebKit::RemoteMediaPlayerProxy::mediaPlayerOnNewVideoFrameMetadata):
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:
(WebKit::LibWebRTCCodecsProxy::createDecoderCallback):
* Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::videoFrameForCurrentTime):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in:
* Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.cpp:
(WebKit::RemoteVideoFrameProxy::properties):
(WebKit::RemoteVideoFrameProxy::~RemoteVideoFrameProxy):
(WebKit::releaseRemoteVideoFrameProxy): Deleted.
(WebKit::RemoteVideoFrameProxy::releaseUnused): Deleted.
* Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.h:
* Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxyProperties.h:
* Source/WebKit/WebProcess/GPU/media/cocoa/MediaPlayerPrivateRemoteCocoa.mm:
(WebKit::MediaPlayerPrivateRemote::pushVideoFrameMetadata):
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
(WebKit::LibWebRTCCodecs::completedDecoding):
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in:
* Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp:
(WebKit::RemoteCaptureSampleManager::videoFrameAvailable):
* Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.h:
* Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.messages.in:

3da3dca

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@kkinnunen-apple kkinnunen-apple self-assigned this Sep 11, 2024
@kkinnunen-apple kkinnunen-apple added the Media Bugs related to the HTML 5 Media elements. label Sep 11, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 11, 2024
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 11, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 11, 2024
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 11, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 11, 2024
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 13, 2024
@kkinnunen-apple kkinnunen-apple changed the title VideoFrames cannot be created asynchronously in WebContent process WebXROpaqueFramebuffer doesn't use WebGLFramebuffer correctly. Just use the plain GL object. Also fixes one relase nullptr deref. Sep 13, 2024
…oyed before message is received

https://bugs.webkit.org/show_bug.cgi?id=279671
rdar://problem/135948989

Reviewed by NOBODY (OOPS!).

GPU process would put the captured VideoFrame to the video frame heap
and send the reference to the WebContent process media player. If
the MediaPlayerPrivateRemote was destroyed while the message was in transit,
the message would be ignored and the video frame would leak in the
heap.

Make the GPUP -> WCP video frame offer messages asynchronous with
replies. Since asynchronous message replies are always delivered, this
fixes the memory leak. Remove the video frame from the heap if
the frame was not adopted.

This concerns following video frame offer messages:
    Messages::MediaPlayerPriveteRemote::PushVideoFrameMetadata
    Messages::LibWebRTCCodecs::CompletedDecoding
    Messages::RemoteCaptureSampleManager::videoFrameAvailable

* Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp:
(WebKit::RemoteVideoFrameObjectHeap::addWithCompletion):
(WebKit::RemoteVideoFrameObjectHeap::add):
* Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h:
* Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm:
(WebKit::RemoteMediaPlayerProxy::mediaPlayerOnNewVideoFrameMetadata):
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:
(WebKit::LibWebRTCCodecsProxy::createDecoderCallback):
* Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::videoFrameForCurrentTime):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in:
* Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.cpp:
(WebKit::RemoteVideoFrameProxy::properties):
(WebKit::RemoteVideoFrameProxy::~RemoteVideoFrameProxy):
(WebKit::releaseRemoteVideoFrameProxy): Deleted.
(WebKit::RemoteVideoFrameProxy::releaseUnused): Deleted.
* Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.h:
* Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxyProperties.h:
* Source/WebKit/WebProcess/GPU/media/cocoa/MediaPlayerPrivateRemoteCocoa.mm:
(WebKit::MediaPlayerPrivateRemote::pushVideoFrameMetadata):
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
(WebKit::LibWebRTCCodecs::completedDecoding):
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in:
* Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp:
(WebKit::RemoteCaptureSampleManager::videoFrameAvailable):
* Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.h:
* Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.messages.in:
@kkinnunen-apple kkinnunen-apple changed the title WebXROpaqueFramebuffer doesn't use WebGLFramebuffer correctly. Just use the plain GL object. Also fixes one relase nullptr deref. Captured video frames are leaked if Web process media player is destroyed before message is received Sep 13, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Media Bugs related to the HTML 5 Media elements. merging-blocked Applied to prevent a change from being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants