Diff
Modified: trunk/Source/WebKit/ChangeLog (290554 => 290555)
--- trunk/Source/WebKit/ChangeLog 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/ChangeLog 2022-02-27 07:27:45 UTC (rev 290555)
@@ -1,3 +1,26 @@
+2022-02-26 Kimmo Kinnunen <[email protected]>
+
+ RemoteCaptureSampleManager, UserMediaSampleCaptureManagerProxy create the RemoteVideoFrame in incorrectly
+ https://bugs.webkit.org/show_bug.cgi?id=237134
+
+ Reviewed by Youenn Fablet.
+
+ UserMediaSampleCaptureManagerProxy should not call RemoteVideoFrameObjectHeap::createVideoFrame,
+ the function should be removed. Instead, UserMediaSampleCaptureManagerProxy should construct a
+ write reference to insert the media sample -> remote proxy mapping. Then
+ the result of the insert, a new reference, should be sent as part of the
+ RemoteVideoFrameProxy::Properties to the WP. This way the sent reference is
+ constructed as expected. Previously the reference was correct but matched just
+ by selecting the constants currently used (0).
+
+ * UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:
+ * WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
+ * WebProcess/cocoa/RemoteCaptureSampleManager.cpp:
+ (WebKit::RemoteCaptureSampleManager::videoSampleAvailable):
+ (WebKit::RemoteCaptureSampleManager::videoSampleAvailableCV):
+ * WebProcess/cocoa/RemoteCaptureSampleManager.h:
+ * WebProcess/cocoa/RemoteCaptureSampleManager.messages.in:
+
2022-02-26 Chris Dumez <[email protected]>
Drop Ref<>'s operator==() as it is a bit ambiguous / confusing
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp (290554 => 290555)
--- trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp 2022-02-27 07:27:45 UTC (rev 290555)
@@ -74,13 +74,6 @@
}
}
-// FIXME: This will be removed once call sites use addVideoFrame().
-RemoteVideoFrameIdentifier RemoteVideoFrameObjectHeap::createRemoteVideoFrame(Ref<WebCore::MediaSample>&& frame)
-{
- auto identifier = RemoteVideoFrameIdentifier::generateThreadSafe();
- m_heap.add(identifier, WTFMove(frame));
- return identifier;
-}
RemoteVideoFrameProxy::Properties RemoteVideoFrameObjectHeap::add(Ref<WebCore::MediaSample>&& frame)
{
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h (290554 => 290555)
--- trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h 2022-02-27 07:27:45 UTC (rev 290555)
@@ -44,7 +44,6 @@
static Ref<RemoteVideoFrameObjectHeap> create(GPUConnectionToWebProcess&);
~RemoteVideoFrameObjectHeap();
- RemoteVideoFrameIdentifier createRemoteVideoFrame(Ref<WebCore::MediaSample>&&);
RemoteVideoFrameProxy::Properties add(Ref<WebCore::MediaSample>&&);
RefPtr<WebCore::MediaSample> get(RemoteVideoFrameReadReference&& read) { return m_heap.retire(WTFMove(read), 0_s); }
Modified: trunk/Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp (290554 => 290555)
--- trunk/Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp 2022-02-27 07:27:45 UTC (rev 290555)
@@ -200,21 +200,17 @@
auto videoSample = rotateVideoFrameIfNeeded(sample);
if (!videoSample)
return;
-
+ if (m_resourceOwner)
+ videoSample->setOwnershipIdentity(m_resourceOwner);
+ if (m_videoFrameObjectHeap) {
+ auto properties = m_videoFrameObjectHeap->add(sample);
+ m_connection->send(Messages::RemoteCaptureSampleManager::VideoSampleAvailable(m_id, properties, metadata), 0);
+ return;
+ }
auto remoteSample = RemoteVideoSample::create(*videoSample);
if (!remoteSample)
return;
-
- if (m_resourceOwner)
- remoteSample->setOwnershipIdentity(m_resourceOwner);
-
- std::optional<RemoteVideoFrameIdentifier> remoteIdentifier;
- if (m_videoFrameObjectHeap) {
- remoteIdentifier = m_videoFrameObjectHeap->createRemoteVideoFrame(sample);
- remoteSample->clearIOSurface();
- }
-
- m_connection->send(Messages::RemoteCaptureSampleManager::VideoSampleAvailable(m_id, WTFMove(*remoteSample), remoteIdentifier, metadata), 0);
+ m_connection->send(Messages::RemoteCaptureSampleManager::VideoSampleAvailableCV(m_id, *remoteSample, metadata), 0);
}
RetainPtr<CVPixelBufferRef> rotatePixelBuffer(MediaSample& sample)
Modified: trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h (290554 => 290555)
--- trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h 2022-02-27 07:27:45 UTC (rev 290555)
@@ -133,6 +133,7 @@
void failedDecoding(RTCDecoderIdentifier);
void completedDecoding(RTCDecoderIdentifier, uint32_t timeStamp, RemoteVideoFrameProxy::Properties&&);
+ // FIXME: Will be removed once RemoteVideoFrameProxy providers are the only ones sending data.
void completedDecodingCV(RTCDecoderIdentifier, uint32_t timeStamp, WebCore::RemoteVideoSample&&);
void completedEncoding(RTCEncoderIdentifier, IPC::DataReference&&, const webrtc::WebKitEncodedFrameInfo&);
RetainPtr<CVPixelBufferRef> convertToBGRA(CVPixelBufferRef);
Modified: trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp (290554 => 290555)
--- trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp 2022-02-27 07:27:45 UTC (rev 290555)
@@ -158,28 +158,32 @@
iterator->value->setStorage(ipcHandle.handle, description, numberOfFrames, WTFMove(semaphore), mediaTime, frameChunkSize);
}
-void RemoteCaptureSampleManager::videoSampleAvailable(RealtimeMediaSourceIdentifier identifier, RemoteVideoSample&& sample, std::optional<RemoteVideoFrameIdentifier> remoteIdentifier, VideoSampleMetadata metadata)
+void RemoteCaptureSampleManager::videoSampleAvailable(RealtimeMediaSourceIdentifier identifier, RemoteVideoFrameProxy::Properties&& properties, VideoSampleMetadata metadata)
{
ASSERT(!WTF::isMainRunLoop());
-
- RefPtr<MediaSample> videoFrame;
- // We always create RemoteVideoFrameProxy so that we can release the corresponding GPUProcess IOSurface right away if there is no video source.
- if (remoteIdentifier) {
- RemoteVideoFrameProxy::Properties properties { { *remoteIdentifier, 0 }, sample.time(), sample.mirrored(), sample.rotation(), sample.size(), sample.videoFormat() };
+ // Create videoFrame before early outs so that the reference in `properties` is adopted.
+ Ref<RemoteVideoFrameProxy> videoFrame = [&] {
// FIXME: We need to either get GPUProcess or UIProcess object heap proxy. For now we always go to GPUProcess.
Locker lock(m_videoFrameObjectHeapProxyLock);
- videoFrame = RemoteVideoFrameProxy::create(*m_connection, *m_videoFrameObjectHeapProxy, WTFMove(properties));
+ return RemoteVideoFrameProxy::create(*m_connection, *m_videoFrameObjectHeapProxy, WTFMove(properties));
+ }();
+ auto iterator = m_videoSources.find(identifier);
+ if (iterator == m_videoSources.end()) {
+ RELEASE_LOG_ERROR(WebRTC, "Unable to find source %llu for remoteVideoSampleAvailable", identifier.toUInt64());
+ return;
}
+ auto videoFrameSize = videoFrame->size();
+ iterator->value->videoFrameAvailable(WTFMove(videoFrame), videoFrameSize, metadata);
+}
+void RemoteCaptureSampleManager::videoSampleAvailableCV(RealtimeMediaSourceIdentifier identifier, RemoteVideoSample&& sample, VideoSampleMetadata metadata)
+{
+ ASSERT(!WTF::isMainRunLoop());
auto iterator = m_videoSources.find(identifier);
if (iterator == m_videoSources.end()) {
RELEASE_LOG_ERROR(WebRTC, "Unable to find source %llu for remoteVideoSampleAvailable", identifier.toUInt64());
return;
}
- if (videoFrame) {
- iterator->value->videoFrameAvailable(videoFrame.releaseNonNull(), sample.size(), metadata);
- return;
- }
iterator->value->videoSampleAvailable(WTFMove(sample), metadata);
}
Modified: trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.h (290554 => 290555)
--- trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.h 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.h 2022-02-27 07:27:45 UTC (rev 290555)
@@ -34,6 +34,7 @@
#include "RemoteRealtimeDisplaySource.h"
#include "RemoteRealtimeVideoSource.h"
#include "RemoteVideoFrameIdentifier.h"
+#include "RemoteVideoFrameProxy.h"
#include "SharedMemory.h"
#include <WebCore/CAAudioStreamDescription.h>
#include <WebCore/CARingBuffer.h>
@@ -74,7 +75,9 @@
// Messages
void audioStorageChanged(WebCore::RealtimeMediaSourceIdentifier, const SharedMemory::IPCHandle&, const WebCore::CAAudioStreamDescription&, uint64_t numberOfFrames, IPC::Semaphore&&, const MediaTime&, size_t frameSampleSize);
void audioSamplesAvailable(WebCore::RealtimeMediaSourceIdentifier, MediaTime, uint64_t numberOfFrames);
- void videoSampleAvailable(WebCore::RealtimeMediaSourceIdentifier, WebCore::RemoteVideoSample&&, std::optional<RemoteVideoFrameIdentifier>, WebCore::VideoSampleMetadata);
+ void videoSampleAvailable(WebCore::RealtimeMediaSourceIdentifier, RemoteVideoFrameProxy::Properties&&, WebCore::VideoSampleMetadata);
+ // FIXME: Will be removed once RemoteVideoFrameProxy providers are the only ones sending data.
+ void videoSampleAvailableCV(WebCore::RealtimeMediaSourceIdentifier, WebCore::RemoteVideoSample&&, WebCore::VideoSampleMetadata);
void setConnection(IPC::Connection*);
Modified: trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.messages.in (290554 => 290555)
--- trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.messages.in 2022-02-27 07:06:29 UTC (rev 290554)
+++ trunk/Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.messages.in 2022-02-27 07:27:45 UTC (rev 290555)
@@ -25,7 +25,9 @@
messages -> RemoteCaptureSampleManager NotRefCounted {
AudioStorageChanged(WebCore::RealtimeMediaSourceIdentifier id, WebKit::SharedMemory::IPCHandle storageHandle, WebCore::CAAudioStreamDescription description, uint64_t numberOfFrames, IPC::Semaphore captureSemaphore, MediaTime mediaTime, size_t frameChunkSize);
- VideoSampleAvailable(WebCore::RealtimeMediaSourceIdentifier id, WebCore::RemoteVideoSample sample, std::optional<WebKit::RemoteVideoFrameIdentifier> remoteIdentifier, struct WebCore::VideoSampleMetadata metadata)
+ VideoSampleAvailable(WebCore::RealtimeMediaSourceIdentifier id, WebKit::RemoteVideoFrameProxy::Properties sample, struct WebCore::VideoSampleMetadata metadata)
+ VideoSampleAvailableCV(WebCore::RealtimeMediaSourceIdentifier id, WebCore::RemoteVideoSample sample, struct WebCore::VideoSampleMetadata metadata)
+
}
#endif
