Bug 237027

Created attachment 452844 [details] Patch

Created attachment 452971 [details] Patch

Comment on attachment 452971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452971&action=review > Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp:101 > + m_blackFrameSize = WebCore::IntSize { static_cast<int>(size.width()), static_cast<int>(size.height()) }; Just curious, is it possible that we need to change the black frame size?

Comment on attachment 452971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452971&action=review with some suggestions > Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp:103 > + SharedVideoFrame sharedVideoFrame { sample.presentationTime(), sample.videoMirrored(), sample.videoRotation(), *m_blackFrameSize }; From this part it would seem you can just remove m_blackFrameSize and keep sending the real sample size? > Source/WebKit/WebProcess/GPU/webrtc/SharedVideoFrame.cpp:190 > + }, [](IntSize size) -> RetainPtr<CVPixelBufferRef> { I think for this use-case, you could; - cache the last black frame - update the last black frame only if the size differs - set the ownership to the caller (so black frames don't consume GPUP memory)

(In reply to Kimmo Kinnunen from comment #4) > Comment on attachment 452971 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452971&action=review > > with some suggestions > > > Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp:103 > > + SharedVideoFrame sharedVideoFrame { sample.presentationTime(), sample.videoMirrored(), sample.videoRotation(), *m_blackFrameSize }; > > From this part it would seem you can just remove m_blackFrameSize and keep > sending the real sample size? This is done to keep things in sync with webrtc encoding where we stick with the last available frame size. > > Source/WebKit/WebProcess/GPU/webrtc/SharedVideoFrame.cpp:190 > > + }, [](IntSize size) -> RetainPtr<CVPixelBufferRef> { > > I think for this use-case, you could; > - cache the last black frame > - update the last black frame only if the size differs > - set the ownership to the caller (so black frames don't consume GPUP memory) I hesitated to do this (we are storing a buffer that might not be reused nor removed), I'll add it.

Created attachment 453097 [details] Patch for landing

Committed r290501 (247788@main): <https://commits.webkit.org/247788@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453097 [details].

<rdar://problem/89463204>

This appears to fix fast/mediastream/video-rotation-gpu-process-crash.html on the standard bots, but it is still intermittently crashing on the GPU Process bot https://results.webkit.org/?suite=layout-tests&test=fast%2Fmediastream%2Fvideo-rotation-gpu-process-crash.html

(In reply to Jon Lee from comment #9) > This appears to fix fast/mediastream/video-rotation-gpu-process-crash.html > on the standard bots, but it is still intermittently crashing on the GPU > Process bot > > https://results.webkit.org/?suite=layout- > tests&test=fast%2Fmediastream%2Fvideo-rotation-gpu-process-crash.html This is tracked in https://bugs.webkit.org/show_bug.cgi?id=237295