Diff
Modified: trunk/Source/WebCore/ChangeLog (290422 => 290423)
--- trunk/Source/WebCore/ChangeLog 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebCore/ChangeLog 2022-02-24 12:11:45 UTC (rev 290423)
@@ -1,3 +1,22 @@
+2022-02-24 Kimmo Kinnunen <kkinnu...@apple.com>
+
+ LibWebRTCCodecs, -Proxy create and communicate the RemoteVideoFrameProxy incorrectly
+ https://bugs.webkit.org/show_bug.cgi?id=237083
+
+ Reviewed by Youenn Fablet.
+
+ Add functions to set media sample ownership identity if the underlying object
+ supports the feature.
+
+ * platform/MediaSample.h:
+ (WebCore::MediaSample::setOwnershipIdentity):
+ * platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h:
+ * platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:
+ (WebCore::MediaSampleAVFObjC::setOwnershipIdentity):
+ * platform/graphics/cv/VideoFrameCV.h:
+ * platform/graphics/cv/VideoFrameCV.mm:
+ (WebCore::VideoFrameCV::setOwnershipIdentity):
+
2022-02-24 Martin Robinson <mrobin...@webkit.org>
Implement logical properties for CSS overscroll-behavior
Modified: trunk/Source/WebCore/platform/MediaSample.h (290422 => 290423)
--- trunk/Source/WebCore/platform/MediaSample.h 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebCore/platform/MediaSample.h 2022-02-24 12:11:45 UTC (rev 290423)
@@ -41,6 +41,7 @@
namespace WebCore {
class MockSampleBox;
+class ProcessIdentity;
struct PlatformSample {
enum Type {
@@ -111,6 +112,7 @@
#if PLATFORM(COCOA)
virtual CVPixelBufferRef pixelBuffer() const { return nullptr; };
#endif
+ virtual void setOwnershipIdentity(const ProcessIdentity&) { }
bool isSync() const { return flags() & IsSync; }
bool isNonDisplaying() const { return flags() & IsNonDisplaying; }
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h (290422 => 290423)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h 2022-02-24 12:11:45 UTC (rev 290423)
@@ -76,6 +76,8 @@
bool videoMirrored() const override { return m_mirrored; }
WEBCORE_EXPORT uint32_t videoPixelFormat() const final;
WEBCORE_EXPORT CVPixelBufferRef pixelBuffer() const final;
+ WEBCORE_EXPORT void setOwnershipIdentity(const ProcessIdentity&) final;
+
CMSampleBufferRef sampleBuffer() const { return m_sample.get(); }
bool isHomogeneous() const;
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm (290422 => 290423)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm 2022-02-24 12:11:45 UTC (rev 290423)
@@ -26,8 +26,10 @@
#import "config.h"
#import "MediaSampleAVFObjC.h"
+#import "CVUtilities.h"
#import "PixelBuffer.h"
#import "PixelBufferConformerCV.h"
+#import "ProcessIdentity.h"
#import "VideoFrameCV.h"
#import <_javascript_Core/JSCInlines.h>
#import <_javascript_Core/TypedArrayInlines.h>
@@ -551,4 +553,12 @@
return static_cast<CVPixelBufferRef>(PAL::CMSampleBufferGetImageBuffer(m_sample.get()));
}
+void MediaSampleAVFObjC::setOwnershipIdentity(const ProcessIdentity& resourceOwner)
+{
+ ASSERT(resourceOwner);
+ auto buffer = pixelBuffer();
+ ASSERT(buffer);
+ setOwnershipIdentityForCVPixelBuffer(buffer, resourceOwner);
}
+
+}
Modified: trunk/Source/WebCore/platform/graphics/cv/VideoFrameCV.h (290422 => 290423)
--- trunk/Source/WebCore/platform/graphics/cv/VideoFrameCV.h 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebCore/platform/graphics/cv/VideoFrameCV.h 2022-02-24 12:11:45 UTC (rev 290423)
@@ -49,6 +49,7 @@
// VideoFrame overrides.
WEBCORE_EXPORT WebCore::FloatSize presentationSize() const final;
WEBCORE_EXPORT uint32_t videoPixelFormat() const final;
+ WEBCORE_EXPORT void setOwnershipIdentity(const ProcessIdentity&) final;
bool isCV() const final { return true; }
WEBCORE_EXPORT RefPtr<WebCore::VideoFrameCV> asVideoFrameCV() final;
Modified: trunk/Source/WebCore/platform/graphics/cv/VideoFrameCV.mm (290422 => 290423)
--- trunk/Source/WebCore/platform/graphics/cv/VideoFrameCV.mm 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebCore/platform/graphics/cv/VideoFrameCV.mm 2022-02-24 12:11:45 UTC (rev 290423)
@@ -27,6 +27,8 @@
#include "VideoFrameCV.h"
#if ENABLE(VIDEO) && USE(AVFOUNDATION)
+#include "CVUtilities.h"
+#include "ProcessIdentity.h"
#include "CoreVideoSoftLink.h"
namespace WebCore {
@@ -54,12 +56,20 @@
{
return CVPixelBufferGetPixelFormatType(m_pixelBuffer.get());
}
+
+void VideoFrameCV::setOwnershipIdentity(const ProcessIdentity& resourceOwner)
+{
+ ASSERT(resourceOwner);
+ auto buffer = pixelBuffer();
+ ASSERT(buffer);
+ setOwnershipIdentityForCVPixelBuffer(buffer, resourceOwner);
+}
+
RefPtr<WebCore::VideoFrameCV> VideoFrameCV::asVideoFrameCV()
{
return this;
}
-
ImageOrientation VideoFrameCV::orientation() const
{
// Sample transform first flips x-coordinates, then rotates.
Modified: trunk/Source/WebKit/ChangeLog (290422 => 290423)
--- trunk/Source/WebKit/ChangeLog 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/ChangeLog 2022-02-24 12:11:45 UTC (rev 290423)
@@ -1,5 +1,33 @@
2022-02-24 Kimmo Kinnunen <kkinnu...@apple.com>
+ LibWebRTCCodecs, -Proxy create and communicate the RemoteVideoFrameProxy incorrectly
+ https://bugs.webkit.org/show_bug.cgi?id=237083
+
+ Reviewed by Youenn Fablet.
+ LibWebRTCCodecsProxy should not call RemoteVideoFrameObjectHeap::createVideoFrame,
+ the function should be removed. Instead, LibWebRTCCodecsProxy 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). This works towards being able
+ to remove RemoteVideoFrameObjectHeap::createVideoFrame.
+
+ Split the completedDecoding to two:
+ - completedDecoding used to send new RemoteVideoFrames
+ - completedDecodingCV to send CoreVideo specific RemoteVideoSample instances (which will
+ removed eventually).
+
+ * GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:
+ (WebKit::Function<void):
+ * WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
+ (WebKit::LibWebRTCCodecs::completedDecoding):
+ (WebKit::LibWebRTCCodecs::completedDecodingCV):
+ * WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
+ * WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in:
+
+2022-02-24 Kimmo Kinnunen <kkinnu...@apple.com>
+
Thread safety analysis to assert "code is run sequentially" is not useful when code is mainly run with WorkQueues
https://bugs.webkit.org/show_bug.cgi?id=236832
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp (290422 => 290423)
--- trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.cpp 2022-02-24 12:11:45 UTC (rev 290423)
@@ -74,6 +74,7 @@
}
}
+// FIXME: This will be removed once call sites use addVideoFrame().
RemoteVideoFrameIdentifier RemoteVideoFrameObjectHeap::createRemoteVideoFrame(Ref<WebCore::MediaSample>&& frame)
{
auto identifier = RemoteVideoFrameIdentifier::generateThreadSafe();
@@ -81,6 +82,15 @@
return identifier;
}
+RemoteVideoFrameProxy::Properties RemoteVideoFrameObjectHeap::addVideoFrame(Ref<WebCore::MediaSample>&& frame)
+{
+ auto write = RemoteVideoFrameWriteReference::generateForAdd();
+ auto newFrameReference = write.retiredReference();
+ auto properties = RemoteVideoFrameProxy::properties(WTFMove(newFrameReference), frame);
+ retire(WTFMove(write), WTFMove(frame), std::nullopt);
+ return properties;
+}
+
void RemoteVideoFrameObjectHeap::releaseVideoFrame(RemoteVideoFrameWriteReference&& write)
{
assertIsMainThread();
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h (290422 => 290423)
--- trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteVideoFrameObjectHeap.h 2022-02-24 12:11:45 UTC (rev 290423)
@@ -27,6 +27,7 @@
#if ENABLE(GPU_PROCESS) && ENABLE(VIDEO)
#include "RemoteVideoFrameIdentifier.h"
+#include "RemoteVideoFrameProxy.h"
#include "ThreadSafeObjectHeap.h"
#include <WebCore/MediaSample.h>
#include <wtf/ThreadAssertions.h>
@@ -48,7 +49,9 @@
static Ref<RemoteVideoFrameObjectHeap> create(GPUConnectionToWebProcess&);
~RemoteVideoFrameObjectHeap();
+ // FIXME: This will be removed once call sites use addVideoFrame().
RemoteVideoFrameIdentifier createRemoteVideoFrame(Ref<WebCore::MediaSample>&&);
+ RemoteVideoFrameProxy::Properties addVideoFrame(Ref<WebCore::MediaSample>&&);
void stopListeningForIPC(Ref<RemoteVideoFrameObjectHeap>&& refFromConnection);
Modified: trunk/Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm (290422 => 290423)
--- trunk/Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm 2022-02-24 12:11:45 UTC (rev 290423)
@@ -87,21 +87,17 @@
auto sample = WebCore::MediaSampleAVFObjC::createImageSample(pixelBuffer, WebCore::MediaSample::VideoRotation::None, false, MediaTime(timeStampNs, 1), { });
if (!sample)
return;
-
+ if (resourceOwner)
+ sample->setOwnershipIdentity(resourceOwner);
+ if (useRemoteFrames) {
+ auto properties = videoFrameObjectHeap->addVideoFrame(sample.releaseNonNull());
+ connection->send(Messages::LibWebRTCCodecs::CompletedDecoding { identifier, timeStamp, WTFMove(properties) }, 0);
+ return;
+ }
auto remoteSample = WebCore::RemoteVideoSample::create(*sample);
if (!remoteSample)
return;
-
- if (resourceOwner)
- remoteSample->setOwnershipIdentity(resourceOwner);
-
- std::optional<RemoteVideoFrameIdentifier> remoteIdentifier;
- if (useRemoteFrames) {
- remoteIdentifier = videoFrameObjectHeap->createRemoteVideoFrame(sample.releaseNonNull());
- remoteSample->clearIOSurface();
- }
-
- connection->send(Messages::LibWebRTCCodecs::CompletedDecoding { identifier, timeStamp, *remoteSample, remoteIdentifier }, 0);
+ connection->send(Messages::LibWebRTCCodecs::CompletedDecodingCV { identifier, timeStamp, *remoteSample }, 0);
};
}
Modified: trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp (290422 => 290423)
--- trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp 2022-02-24 12:11:45 UTC (rev 290423)
@@ -318,39 +318,45 @@
decoder->hasError = true;
}
-void LibWebRTCCodecs::completedDecoding(RTCDecoderIdentifier decoderIdentifier, uint32_t timeStamp, WebCore::RemoteVideoSample&& remoteSample, std::optional<RemoteVideoFrameIdentifier> remoteFrameIdentifier)
+void LibWebRTCCodecs::completedDecoding(RTCDecoderIdentifier decoderIdentifier, uint32_t timeStamp, RemoteVideoFrameProxy::Properties&& properties)
{
ASSERT(!isMainRunLoop());
-
- RefPtr<RemoteVideoFrameProxy> remoteVideoFrame;
- // We always create RemoteVideoFrameProxy so that we can release the corresponding GPUProcess IOSurface right away if there is no video source.
- if (remoteFrameIdentifier) {
+ // Adopt RemoteVideoFrameProxy::Properties to RemoteVideoFrameProxy instance before the early outs, so that the reference gets adopted.
+ // Typically RemoteVideoFrameProxy::Properties&& sent to destinations that are already removed need to be handled separately.
+ // LibWebRTCCodecs is not ever removed, so we do not do this. However, if it ever is, LibWebRTCCodecs::handleMessageToRemovedDestination()
+ // needs to be implemented.
+ Ref<RemoteVideoFrameProxy> remoteVideoFrame = [&] {
Locker locker { m_connectionLock };
- RemoteVideoFrameProxy::Properties properties { { *remoteFrameIdentifier, 0 }, remoteSample.time(), remoteSample.mirrored(), remoteSample.rotation(), remoteSample.size(), remoteSample.videoFormat() };
- remoteVideoFrame = RemoteVideoFrameProxy::create(*m_connection, *m_videoFrameObjectHeapProxy, WTFMove(properties));
- }
+ return RemoteVideoFrameProxy::create(*m_connection, *m_videoFrameObjectHeapProxy, WTFMove(properties));
+ }();
// FIXME: Do error logging.
auto* decoder = m_decoders.get(decoderIdentifier);
if (!decoder)
return;
-
if (!decoder->decodedImageCallbackLock.tryLock())
return;
-
Locker locker { AdoptLock, decoder->decodedImageCallbackLock };
-
if (!decoder->decodedImageCallback)
return;
+ auto& frame = remoteVideoFrame.leakRef(); // Balanced by the release callback of videoDecoderTaskComplete.
+ webrtc::videoDecoderTaskComplete(decoder->decodedImageCallback, timeStamp, frame.presentationTime().toDouble(), &frame,
+ [](auto* pointer) { return static_cast<RemoteVideoFrameProxy*>(pointer)->pixelBuffer(); },
+ [](auto* pointer) { static_cast<RemoteVideoFrameProxy*>(pointer)->deref(); },
+ frame.size().width(), frame.size().height());
+}
- if (remoteVideoFrame) {
- Ref videoFrame { static_cast<VideoFrame&>(*remoteVideoFrame) };
- webrtc::videoDecoderTaskComplete(decoder->decodedImageCallback, timeStamp, remoteSample.time().toDouble(), &videoFrame.leakRef(),
- [](auto* pointer) { return static_cast<VideoFrame*>(pointer)->pixelBuffer(); },
- [](auto* pointer) { static_cast<VideoFrame*>(pointer)->deref(); },
- remoteSample.size().width(), remoteSample.size().height());
+void LibWebRTCCodecs::completedDecodingCV(RTCDecoderIdentifier decoderIdentifier, uint32_t timeStamp, WebCore::RemoteVideoSample&& remoteSample)
+{
+ ASSERT(!isMainRunLoop());
+ // FIXME: Do error logging.
+ auto* decoder = m_decoders.get(decoderIdentifier);
+ if (!decoder)
return;
- }
-
+ if (!decoder->decodedImageCallbackLock.tryLock())
+ return;
+ Locker locker { AdoptLock, decoder->decodedImageCallbackLock };
+ if (!decoder->decodedImageCallback)
+ return;
if (!remoteSample.surface())
return;
auto pixelBuffer = createCVPixelBuffer(remoteSample.surface()).value_or(nullptr);
@@ -358,7 +364,6 @@
ASSERT_NOT_REACHED();
return;
}
-
webrtc::videoDecoderTaskComplete(decoder->decodedImageCallback, timeStamp, remoteSample.time().toDouble(), pixelBuffer.get());
}
Modified: trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h (290422 => 290423)
--- trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h 2022-02-24 12:11:45 UTC (rev 290423)
@@ -35,6 +35,7 @@
#include "RTCDecoderIdentifier.h"
#include "RTCEncoderIdentifier.h"
#include "RemoteVideoFrameIdentifier.h"
+#include "RemoteVideoFrameProxy.h"
#include "SharedVideoFrame.h"
#include <WebCore/PixelBufferConformerCV.h>
#include <map>
@@ -131,7 +132,8 @@
void gpuProcessConnectionMayNoLongerBeNeeded();
void failedDecoding(RTCDecoderIdentifier);
- void completedDecoding(RTCDecoderIdentifier, uint32_t timeStamp, WebCore::RemoteVideoSample&&, std::optional<RemoteVideoFrameIdentifier>);
+ void completedDecoding(RTCDecoderIdentifier, uint32_t timeStamp, RemoteVideoFrameProxy::Properties&&);
+ 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/GPU/webrtc/LibWebRTCCodecs.messages.in (290422 => 290423)
--- trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in 2022-02-24 11:57:47 UTC (rev 290422)
+++ trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in 2022-02-24 12:11:45 UTC (rev 290423)
@@ -24,8 +24,8 @@
messages -> LibWebRTCCodecs NotRefCounted {
FailedDecoding(WebKit::RTCDecoderIdentifier identifier)
- CompletedDecoding(WebKit::RTCDecoderIdentifier identifier, uint32_t timeStamp, WebCore::RemoteVideoSample sample, std::optional<WebKit::RemoteVideoFrameIdentifier> remoteFrameIdentifier)
-
+ CompletedDecoding(WebKit::RTCDecoderIdentifier identifier, uint32_t timeStamp, WebKit::RemoteVideoFrameProxy::Properties frame)
+ CompletedDecodingCV(WebKit::RTCDecoderIdentifier identifier, uint32_t timeStamp, WebCore::RemoteVideoSample sample)
CompletedEncoding(WebKit::RTCEncoderIdentifier identifier, IPC::DataReference data, struct webrtc::WebKitEncodedFrameInfo info);
}
