Modified: trunk/Source/WebKit/ChangeLog (276213 => 276214)
--- trunk/Source/WebKit/ChangeLog 2021-04-17 23:55:13 UTC (rev 276213)
+++ trunk/Source/WebKit/ChangeLog 2021-04-18 00:10:13 UTC (rev 276214)
@@ -1,5 +1,82 @@
2021-04-17 Chris Dumez <[email protected]>
+ LibWebRTCCodecs eagerly launches the GPUProcess and always relaunches it on exit
+ https://bugs.webkit.org/show_bug.cgi?id=224704
+
+ Reviewed by Darin Adler.
+
+ LibWebRTCCodecs eagerly launches the GPUProcess and always relaunches it on exit. The GPUProcess
+ should only be (re-)launched when needed. In the case of the LibWebRTCCodecs, it seems it only
+ needs a GPUProcess connection if it has m_decoders / m_encoders are non-empty.
+
+ * WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
+ (WebKit::LibWebRTCCodecs::ensureGPUProcessConnectionOnMainThread):
+ Renamed startListeningForIPC() to ensureGPUProcessConnectionOnMainThread(). Only do the
+ connection initialization if m_connection is not null.
+
+ (WebKit::LibWebRTCCodecs::ensureGPUProcessConnectionAndDispatchToThread):
+ Version of dispatchToThread() which makes sure that the GPUProcessConnection is initialized
+ before dispatching. It is used when constructing a decoder / encoder. It sets the
+ m_needsGPUProcessConnection flag to true to indicate someone needed the connection
+ (and that we should re-initiate it in case it is severed). If the connection is already
+ initialized, then it does a simple dispatchToThread(). If the connection is not initialized
+ yet, then we have to hop to the main thread (if not already on it) to initialize the
+ GPUProcessConnection.
+
+ (WebKit::LibWebRTCCodecs::gpuProcessConnectionMayNoLongerBeNeeded):
+ Function that gets called on the background thread every time a encoder / decoder is
+ removed. Its purpose is to set m_needsGPUProcessConnection back to false once we no
+ longer have any encoder / decoder, so that gpuProcessConnectionDidClose() does not
+ attempt to relaunch the GPUProcess if it goes away.
+
+ LibWebRTCCodecs::setCallbacks():
+ Check if VP9Support is enabled via PlatformMediaSessionManager instead of from the
+ GPUProcessConnection. This avoids eagerly launching the GPUProcess. The
+ GPUProcessConnection constructor gets its VPx support information from
+ PlatformMediaSessionManager anyway. The WebPage constructor is where the VPx support
+ information comes from and it updates the VPx flags on the PlatformMediaSessionManager.
+ The WebPage constructor only updates the VPx flags on the GPUProcessConnection if this
+ connection already exists to avoid eagerly launching the GPUProcess.
+
+ (WebKit::LibWebRTCCodecs::createDecoder):
+ - Call ensureGPUProcessConnectionAndDispatchToThread() instead of dispatchToThread()
+ to make sure we have a GPUProcessConnection before creating the decoder.
+ - Add a missing locker for m_connectionLock on the background thread since it is using
+ m_connection (pre-existing bug).
+
+ (WebKit::LibWebRTCCodecs::releaseDecoder):
+ Call gpuProcessConnectionMayNoLongerBeNeeded() to reset the m_needsGPUProcessConnection
+ flag to false if necessary.
+
+ (WebKit::LibWebRTCCodecs::createEncoder):
+ - Call ensureGPUProcessConnectionAndDispatchToThread() instead of dispatchToThread()
+ to make sure we have a GPUProcessConnection before creating the encoder.
+
+ (WebKit::LibWebRTCCodecs::releaseEncoder):
+ Call gpuProcessConnectionMayNoLongerBeNeeded() to reset the m_needsGPUProcessConnection
+ flag to false if necessary.
+
+ (WebKit::LibWebRTCCodecs::gpuProcessConnectionDidClose):
+ - Clear m_connection when the GPUProcess connection is severed (note that this does not
+ necessarily indicate a crash since the GPUProcess exits when idle and under memory
+ pressure).
+ - Only re-initiate the GPUProcess connection if m_needsGPUProcessConnection is true,
+ meaning that we have encoders/decoders. I use this flag instead of checking m_encoders
+ & m_decoders since those containers are modified on the background thread and this
+ function is called on the main thread.
+
+ * WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
+ (WebKit::LibWebRTCCodecs::create):
+ - Stop calling startListeningForIPC() on construction as we don't want to launch the GPUProcess
+ until an encoder / decoder is created.
+ - Fix a pre-existing issue where the class subclasses ThreadSafeRefCounted (via
+ ThreadMessageReceiverRefCounted) and yet was using std::unique_ptr<> instead of
+ RefPtr<>.
+
+ * WebProcess/WebProcess.h:
+
+2021-04-17 Chris Dumez <[email protected]>
+
RemoteImageDecoderAVFManager should never re-launch the GPUProcess on destruction
https://bugs.webkit.org/show_bug.cgi?id=224723
Modified: trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp (276213 => 276214)
--- trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp 2021-04-17 23:55:13 UTC (rev 276213)
+++ trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp 2021-04-18 00:10:13 UTC (rev 276214)
@@ -34,6 +34,7 @@
#include "WebCoreArgumentCoders.h"
#include "WebProcess.h"
#include <WebCore/LibWebRTCMacros.h>
+#include <WebCore/PlatformMediaSessionManager.h>
#include <WebCore/RealtimeVideoUtilities.h>
#include <WebCore/RemoteVideoSample.h>
#include <WebCore/RuntimeEnabledFeatures.h>
@@ -163,14 +164,23 @@
WebProcess::singleton().libWebRTCCodecs().setEncodeRates(*static_cast<LibWebRTCCodecs::Encoder*>(encoder), bitRate, frameRate);
}
+Ref<LibWebRTCCodecs> LibWebRTCCodecs::create()
+{
+ return adoptRef(*new LibWebRTCCodecs);
+}
+
LibWebRTCCodecs::LibWebRTCCodecs()
: m_queue(WorkQueue::create("LibWebRTCCodecs", WorkQueue::Type::Serial, WorkQueue::QOS::UserInteractive))
{
}
-void LibWebRTCCodecs::startListeningForIPC()
+void LibWebRTCCodecs::ensureGPUProcessConnectionOnMainThread(Locker<Lock>& locker)
{
- ASSERT(!m_connection);
+ ASSERT(isMainRunLoop());
+ ASSERT_UNUSED(locker, locker.lockable() == &m_connectionLock);
+ if (m_connection)
+ return;
+
auto& gpuConnection = WebProcess::singleton().ensureGPUProcessConnection();
gpuConnection.addClient(*this);
m_connection = makeRef(gpuConnection.connection());
@@ -177,6 +187,29 @@
m_connection->addThreadMessageReceiver(Messages::LibWebRTCCodecs::messageReceiverName(), this);
}
+// May be called on any thread.
+void LibWebRTCCodecs::ensureGPUProcessConnectionAndDispatchToThread(Function<void()>&& task)
+{
+ m_needsGPUProcessConnection = true;
+ {
+ auto locker = holdLock(m_connectionLock);
+ if (m_connection)
+ return dispatchToThread(WTFMove(task));
+ }
+ ensureOnMainRunLoop([this, task = WTFMove(task)]() mutable {
+ auto locker = holdLock(m_connectionLock);
+ ensureGPUProcessConnectionOnMainThread(locker);
+ dispatchToThread(WTFMove(task));
+ });
+}
+
+void LibWebRTCCodecs::gpuProcessConnectionMayNoLongerBeNeeded()
+{
+ ASSERT(!isMainRunLoop());
+ if (m_encoders.isEmpty() && m_decoders.isEmpty())
+ m_needsGPUProcessConnection = false;
+}
+
LibWebRTCCodecs::~LibWebRTCCodecs()
{
ASSERT_NOT_REACHED();
@@ -196,9 +229,8 @@
WebProcess::singleton().libWebRTCCodecs();
#if ENABLE(VP9)
- auto& gpuConnection = WebProcess::singleton().ensureGPUProcessConnection();
// FIMXE: We should disable VP9VTB if VP9 hardware decoding is enabled but there is no support for it.
- WebProcess::singleton().libWebRTCCodecs().setVP9VTBSupport(gpuConnection.isVP9DecoderEnabled() || gpuConnection.isVPSWDecoderEnabled());
+ WebProcess::singleton().libWebRTCCodecs().setVP9VTBSupport(PlatformMediaSessionManager::shouldEnableVP9Decoder() || PlatformMediaSessionManager::shouldEnableVP9SWDecoder());
#endif
webrtc::setVideoDecoderCallbacks(createVideoDecoder, releaseVideoDecoder, decodeVideoFrame, registerDecodeCompleteCallback);
@@ -212,7 +244,8 @@
decoder->identifier = RTCDecoderIdentifier::generateThreadSafe();
decoder->type = type;
- dispatchToThread([this, decoder = WTFMove(decoder)]() mutable {
+ ensureGPUProcessConnectionAndDispatchToThread([this, decoder = WTFMove(decoder)]() mutable {
+ auto locker = holdLock(m_connectionLock);
decoder->connection = m_connection;
createRemoteDecoder(*decoder, *m_connection);
@@ -228,8 +261,10 @@
ASSERT(!decoder.decodedImageCallback);
dispatchToThread([this, decoderIdentifier = decoder.identifier] {
ASSERT(m_decoders.contains(decoderIdentifier));
- if (auto decoder = m_decoders.take(decoderIdentifier))
+ if (auto decoder = m_decoders.take(decoderIdentifier)) {
decoder->connection->send(Messages::LibWebRTCCodecsProxy::ReleaseDecoder { decoderIdentifier }, 0);
+ gpuProcessConnectionMayNoLongerBeNeeded();
+ }
});
return 0;
}
@@ -331,7 +366,7 @@
for (auto& keyValue : formatParameters)
parameters.append(std::make_pair(String::fromUTF8(keyValue.first.data(), keyValue.first.length()), String::fromUTF8(keyValue.second.data(), keyValue.second.length())));
- dispatchToThread([this, encoder = WTFMove(encoder), type, parameters = WTFMove(parameters)]() mutable {
+ ensureGPUProcessConnectionAndDispatchToThread([this, encoder = WTFMove(encoder), type, parameters = WTFMove(parameters)]() mutable {
LockHolder holder(m_connectionLock);
encoder->connection = m_connection;
encoder->connection->send(Messages::LibWebRTCCodecsProxy::CreateEncoder { encoder->identifier, formatNameFromCodecType(type), parameters, RuntimeEnabledFeatures::sharedFeatures().webRTCH264LowLatencyEncoderEnabled() }, 0);
@@ -351,6 +386,7 @@
ASSERT(m_encoders.contains(encoderIdentifier));
auto encoder = m_encoders.take(encoderIdentifier);
encoder->connection->send(Messages::LibWebRTCCodecsProxy::ReleaseEncoder { encoderIdentifier }, 0);
+ gpuProcessConnectionMayNoLongerBeNeeded();
});
return 0;
}
@@ -460,15 +496,13 @@
void LibWebRTCCodecs::gpuProcessConnectionDidClose(GPUProcessConnection&)
{
- auto& gpuConnection = WebProcess::singleton().ensureGPUProcessConnection();
- gpuConnection.addClient(*this);
- {
- auto lock = holdLock(m_connectionLock);
- m_connection->removeThreadMessageReceiver(Messages::LibWebRTCCodecs::messageReceiverName());
- m_connection = makeRef(gpuConnection.connection());
- m_connection->addThreadMessageReceiver(Messages::LibWebRTCCodecs::messageReceiverName(), this);
- }
+ ASSERT(isMainRunLoop());
+ auto locker = holdLock(m_connectionLock);
+ std::exchange(m_connection, nullptr)->removeThreadMessageReceiver(Messages::LibWebRTCCodecs::messageReceiverName());
+ if (!m_needsGPUProcessConnection)
+ return;
+ ensureGPUProcessConnectionOnMainThread(locker);
dispatchToThread([this]() {
// Lock everything so that we can update encoder/decoder connection.
LockHolder holder(m_connectionLock);
Modified: trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h (276213 => 276214)
--- trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h 2021-04-17 23:55:13 UTC (rev 276213)
+++ trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h 2021-04-18 00:10:13 UTC (rev 276214)
@@ -62,12 +62,7 @@
class LibWebRTCCodecs : public IPC::Connection::ThreadMessageReceiverRefCounted, public GPUProcessConnection::Client {
WTF_MAKE_FAST_ALLOCATED;
public:
- static std::unique_ptr<LibWebRTCCodecs> create()
- {
- auto instance = std::unique_ptr<LibWebRTCCodecs>(new LibWebRTCCodecs);
- instance->startListeningForIPC();
- return instance;
- }
+ static Ref<LibWebRTCCodecs> create();
~LibWebRTCCodecs();
static void setCallbacks(bool useGPUProcess);
@@ -125,7 +120,9 @@
private:
LibWebRTCCodecs();
- void startListeningForIPC();
+ void ensureGPUProcessConnectionAndDispatchToThread(Function<void()>&&);
+ void ensureGPUProcessConnectionOnMainThread(Locker<Lock>&);
+ void gpuProcessConnectionMayNoLongerBeNeeded();
void failedDecoding(RTCDecoderIdentifier);
void completedDecoding(RTCDecoderIdentifier, uint32_t timeStamp, WebCore::RemoteVideoSample&&);
@@ -144,6 +141,8 @@
HashMap<RTCEncoderIdentifier, std::unique_ptr<Encoder>> m_encoders;
+ std::atomic<bool> m_needsGPUProcessConnection;
+
Lock m_connectionLock;
RefPtr<IPC::Connection> m_connection;
Ref<WorkQueue> m_queue;
Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (276213 => 276214)
--- trunk/Source/WebKit/WebProcess/WebProcess.h 2021-04-17 23:55:13 UTC (rev 276213)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h 2021-04-18 00:10:13 UTC (rev 276214)
@@ -621,7 +621,7 @@
#if ENABLE(GPU_PROCESS)
RefPtr<GPUProcessConnection> m_gpuProcessConnection;
#if PLATFORM(COCOA) && USE(LIBWEBRTC)
- std::unique_ptr<LibWebRTCCodecs> m_libWebRTCCodecs;
+ RefPtr<LibWebRTCCodecs> m_libWebRTCCodecs;
#endif
#endif
