Allow MediaPlayerPrivateMediaStreamAVFObjC to avoid pixel conformers if IOSurfaces are not allowed
<rdar://92629845>
Created attachment 458862 [details] Patch
Created attachment 458863 [details] Patch
Created attachment 458864 [details] Patch
Created attachment 458865 [details] Patch
Comment on attachment 458865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458865&action=review r=me once the bots are happy > Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp:142 > + m_connection->send(Messages::RemoteVideoFrameObjectHeapProxyProcessor::NewConvertedVideoFrameBuffer { { } }, 0); It looks like SharedVideoFrameReader doesn't log errors, so it might be worth logging an error here to help debug unexpected failures. > Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp:149 > + m_connection->send(Messages::RemoteVideoFrameObjectHeapProxyProcessor::NewConvertedVideoFrameBuffer { { } }, 0); Ditto, PixelBufferConformerCV also doesn't log failures. > Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.mm:32 > + Nit: unneeded white space
> It looks like SharedVideoFrameReader doesn't log errors, so it might be > worth logging an error here to help debug unexpected failures. Let's fix this in https://bugs.webkit.org/show_bug.cgi?id=236066, I just uploaded a patch.
> > Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp:149 > > + m_connection->send(Messages::RemoteVideoFrameObjectHeapProxyProcessor::NewConvertedVideoFrameBuffer { { } }, 0); > > Ditto, PixelBufferConformerCV also doesn't log failures. Will add logging for this one.
Comment on attachment 458865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458865&action=review > Source/WebKit/WebProcess/GPU/webrtc/RemoteVideoFrameObjectHeapProxyProcessor.cpp:145 > + connection.send(Messages::RemoteVideoFrameObjectHeap::ConvertBuffer { WTFMove(*buffer) }, 0); > + m_conversionSemaphore.wait(); Why this and not just a sync message? Sync IPC has logic to avoid deadlocking, but this doesn't.
Created attachment 458882 [details] Patch for landing
Created attachment 458888 [details] Patch for landing
(In reply to Simon Fraser (smfr) from comment #9) > Comment on attachment 458865 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=458865&action=review > > > Source/WebKit/WebProcess/GPU/webrtc/RemoteVideoFrameObjectHeapProxyProcessor.cpp:145 > > + connection.send(Messages::RemoteVideoFrameObjectHeap::ConvertBuffer { WTFMove(*buffer) }, 0); > > + m_conversionSemaphore.wait(); > > Why this and not just a sync message? Sync IPC has logic to avoid > deadlocking, but this doesn't. We send a message from main thread but we receive the answer from a work queue.
Created attachment 458941 [details] Patch for landing
(In reply to youenn fablet from comment #12) > (In reply to Simon Fraser (smfr) from comment #9) > > Comment on attachment 458865 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=458865&action=review > > > > > Source/WebKit/WebProcess/GPU/webrtc/RemoteVideoFrameObjectHeapProxyProcessor.cpp:145 > > > + connection.send(Messages::RemoteVideoFrameObjectHeap::ConvertBuffer { WTFMove(*buffer) }, 0); > > > + m_conversionSemaphore.wait(); > > > > Why this and not just a sync message? Sync IPC has logic to avoid > > deadlocking, but this doesn't. > > We send a message from main thread but we receive the answer from a work > queue. Thinking more about it, this will create a deadlock in case of GPUProcess crash, so we might need to use sync IPC, though I am unsure how sync IPC and work queue message receivers do things consistently. I'll file a bug to keep track of this.
I filed https://bugs.webkit.org/show_bug.cgi?id=240161.
Committed r293886 (250344@main): <https://commits.webkit.org/250344@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458941 [details].