⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 290653 in webkit


Ignore:
Timestamp:
Mar 1, 2022, 9:56:49 AM (5 years ago)
Author:
youenn@apple.com
Message:

Annotate LibWebRTC with thread safety macros
​https://bugs.webkit.org/show_bug.cgi?id=237321

Reviewed by Eric Carlson.

LibWebRTCCodecs works with 3 threads and we add macros to make it clear where each thread is used:

  • the main thread to get its GPU process connection (isMainRunLoop()).
  • the libwebrtc thread where it gets orders to decode/encode frames (!isMainRunLoop())
  • the work queue thread where it is receiving encode/decode results (assertIsCurrent(workQueue())).

Rename m_encodersLock to m_encodersConnectionLock to make it clear this is about locking the encoder connection and not the encoder map.
Both decoder and encoder maps should only be touched on the workQueue thread.

Introduce encoderConnection/setEncoderConnection and decoderConnection/setDecoderConnection routines.
These methods are guarded by corresponding locks.
This requires adding some additional locks when accessing connections in workQueue thread.
Fix a potential issue when creating the encoder: we lock the encoderConnection lock earlier when setting the connection.

Covered by existing tests.

  • WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:

(WebKit::LibWebRTCCodecs::gpuProcessConnectionMayNoLongerBeNeeded):
(WebKit::LibWebRTCCodecs::createDecoder):
(WebKit::LibWebRTCCodecs::releaseDecoder):
(WebKit::LibWebRTCCodecs::decodeFrame):
(WebKit::LibWebRTCCodecs::registerDecodeFrameCallback):
(WebKit::LibWebRTCCodecs::failedDecoding):
(WebKit::LibWebRTCCodecs::completedDecoding):
(WebKit::LibWebRTCCodecs::completedDecodingCV):
(WebKit::LibWebRTCCodecs::createEncoder):
(WebKit::LibWebRTCCodecs::releaseEncoder):
(WebKit::LibWebRTCCodecs::initializeEncoder):
(WebKit::LibWebRTCCodecs::copySharedVideoFrame):
(WebKit::LibWebRTCCodecs::encodeFrame):
(WebKit::LibWebRTCCodecs::registerEncodeFrameCallback):
(WebKit::LibWebRTCCodecs::setEncodeRates):
(WebKit::LibWebRTCCodecs::completedEncoding):
(WebKit::LibWebRTCCodecs::gpuProcessConnectionDidClose):
(WebKit::LibWebRTCCodecs::encoderConnection):
(WebKit::LibWebRTCCodecs::setEncoderConnection):
(WebKit::LibWebRTCCodecs::decoderConnection):
(WebKit::LibWebRTCCodecs::setDecoderConnection):
(WebKit::copySharedVideoFrame): Deleted.

  • WebProcess/GPU/webrtc/LibWebRTCCodecs.h:

(WebKit::LibWebRTCCodecs::workQueue const):

Location:
trunk/Source/WebKit
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r290646 r290653  
     12022-03-01  Youenn Fablet  <youenn@apple.com>
     2
     3        Annotate LibWebRTC with thread safety macros
     4        https://bugs.webkit.org/show_bug.cgi?id=237321
     5
     6        Reviewed by Eric Carlson.
     7
     8        LibWebRTCCodecs works with 3 threads and we add macros to make it clear where each thread is used:
     9        - the main thread to get its GPU process connection (isMainRunLoop()).
     10        - the libwebrtc thread where it gets orders to decode/encode frames (!isMainRunLoop())
     11        - the work queue thread where it is receiving encode/decode results (assertIsCurrent(workQueue())).
     12
     13        Rename m_encodersLock to m_encodersConnectionLock to make it clear this is about locking the encoder connection and not the encoder map.
     14        Both decoder and encoder maps should only be touched on the workQueue thread.
     15
     16        Introduce encoderConnection/setEncoderConnection and decoderConnection/setDecoderConnection routines.
     17        These methods are guarded by corresponding locks.
     18        This requires adding some additional locks when accessing connections in workQueue thread.
     19        Fix a potential issue when creating the encoder: we lock the encoderConnection lock earlier when setting the connection.
     20
     21        Covered by existing tests.
     22
     23        * WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
     24        (WebKit::LibWebRTCCodecs::gpuProcessConnectionMayNoLongerBeNeeded):
     25        (WebKit::LibWebRTCCodecs::createDecoder):
     26        (WebKit::LibWebRTCCodecs::releaseDecoder):
     27        (WebKit::LibWebRTCCodecs::decodeFrame):
     28        (WebKit::LibWebRTCCodecs::registerDecodeFrameCallback):
     29        (WebKit::LibWebRTCCodecs::failedDecoding):
     30        (WebKit::LibWebRTCCodecs::completedDecoding):
     31        (WebKit::LibWebRTCCodecs::completedDecodingCV):
     32        (WebKit::LibWebRTCCodecs::createEncoder):
     33        (WebKit::LibWebRTCCodecs::releaseEncoder):
     34        (WebKit::LibWebRTCCodecs::initializeEncoder):
     35        (WebKit::LibWebRTCCodecs::copySharedVideoFrame):
     36        (WebKit::LibWebRTCCodecs::encodeFrame):
     37        (WebKit::LibWebRTCCodecs::registerEncodeFrameCallback):
     38        (WebKit::LibWebRTCCodecs::setEncodeRates):
     39        (WebKit::LibWebRTCCodecs::completedEncoding):
     40        (WebKit::LibWebRTCCodecs::gpuProcessConnectionDidClose):
     41        (WebKit::LibWebRTCCodecs::encoderConnection):
     42        (WebKit::LibWebRTCCodecs::setEncoderConnection):
     43        (WebKit::LibWebRTCCodecs::decoderConnection):
     44        (WebKit::LibWebRTCCodecs::setDecoderConnection):
     45        (WebKit::copySharedVideoFrame): Deleted.
     46        * WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
     47        (WebKit::LibWebRTCCodecs::workQueue const):
     48
    1492022-03-01  Wenson Hsieh  <wenson_hsieh@apple.com>
    250

  • trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp

    r290558 r290653  
    222222void LibWebRTCCodecs::gpuProcessConnectionMayNoLongerBeNeeded()
    223223{
    224     ASSERT(!isMainRunLoop());
     224    assertIsCurrent(workQueue());
     225
    225226    if (m_encoders.isEmpty() && m_decoders.isEmpty())
    226227        m_needsGPUProcessConnection = false;
    … …  
    255256LibWebRTCCodecs::Decoder* LibWebRTCCodecs::createDecoder(Type type)
    256257{
     258    ASSERT(!isMainRunLoop());
     259
    257260    auto decoder = makeUnique<Decoder>();
    258261    auto* result = decoder.get();
    … …  
    261264
    262265    ensureGPUProcessConnectionAndDispatchToThread([this, decoder = WTFMove(decoder)]() mutable {
     266        assertIsCurrent(workQueue());
     267
    263268        Locker locker { m_connectionLock };
    264         decoder->connection = m_connection;
    265269        createRemoteDecoder(*decoder, *m_connection, m_useRemoteFrames);
     270        setDecoderConnection(*decoder, m_connection.get());
    266271
    267272        auto decoderIdentifier = decoder->identifier;
    … …  
    274279int32_t LibWebRTCCodecs::releaseDecoder(Decoder& decoder)
    275280{
     281    ASSERT(!isMainRunLoop());
     282
    276283#if ASSERT_ENABLED
    277284    {
    … …  
    281288#endif
    282289    ensureGPUProcessConnectionAndDispatchToThread([this, decoderIdentifier = decoder.identifier] {
     290        assertIsCurrent(workQueue());
     291
    283292        ASSERT(m_decoders.contains(decoderIdentifier));
    284293        if (auto decoder = m_decoders.take(decoderIdentifier)) {
    285             decoder->connection->send(Messages::LibWebRTCCodecsProxy::ReleaseDecoder { decoderIdentifier }, 0);
     294            Locker locker { m_connectionLock };
     295            decoderConnection(*decoder)->send(Messages::LibWebRTCCodecsProxy::ReleaseDecoder { decoderIdentifier }, 0);
    286296            gpuProcessConnectionMayNoLongerBeNeeded();
    287297        }
    … …  
    292302int32_t LibWebRTCCodecs::decodeFrame(Decoder& decoder, uint32_t timeStamp, const uint8_t* data, size_t size, uint16_t width, uint16_t height)
    293303{
     304    ASSERT(!isMainRunLoop());
     305
    294306    Locker locker { m_connectionLock };
    295307    if (!decoder.connection || decoder.hasError) {
    … …  
    307319void LibWebRTCCodecs::registerDecodeFrameCallback(Decoder& decoder, void* decodedImageCallback)
    308320{
     321    ASSERT(!isMainRunLoop());
     322
    309323    Locker locker { decoder.decodedImageCallbackLock };
    310324    decoder.decodedImageCallback = decodedImageCallback;
    … …  
    313327void LibWebRTCCodecs::failedDecoding(RTCDecoderIdentifier decoderIdentifier)
    314328{
    315     ASSERT(!isMainRunLoop());
     329    assertIsCurrent(workQueue());
    316330
    317331    if (auto* decoder = m_decoders.get(decoderIdentifier))
    … …  
    321335void LibWebRTCCodecs::completedDecoding(RTCDecoderIdentifier decoderIdentifier, uint32_t timeStamp, RemoteVideoFrameProxy::Properties&& properties)
    322336{
    323     ASSERT(!isMainRunLoop());
     337    assertIsCurrent(workQueue());
     338
    324339    // Adopt RemoteVideoFrameProxy::Properties to RemoteVideoFrameProxy instance before the early outs, so that the reference gets adopted.
    325340    // Typically RemoteVideoFrameProxy::Properties&& sent to destinations that are already removed need to be handled separately.
    … …  
    348363void LibWebRTCCodecs::completedDecodingCV(RTCDecoderIdentifier decoderIdentifier, uint32_t timeStamp, WebCore::RemoteVideoSample&& remoteSample)
    349364{
    350     ASSERT(!isMainRunLoop());
     365    assertIsCurrent(workQueue());
     366
    351367    // FIXME: Do error logging.
    352368    auto* decoder = m_decoders.get(decoderIdentifier);
    … …  
    394410LibWebRTCCodecs::Encoder* LibWebRTCCodecs::createEncoder(Type type, const std::map<std::string, std::string>& formatParameters)
    395411{
     412    ASSERT(!isMainRunLoop());
     413
    396414    auto encoder = makeUnique<Encoder>();
    397415    auto* result = encoder.get();
    … …  
    404422
    405423    ensureGPUProcessConnectionAndDispatchToThread([this, encoder = WTFMove(encoder), type, parameters = WTFMove(parameters)]() mutable {
     424        assertIsCurrent(workQueue());
     425
     426        auto connection = [&]() -> Ref<IPC::Connection> {
     427            Locker locker { m_connectionLock };
     428            return *m_connection;
     429        }();
     430
    406431        {
    407             Locker locker { m_connectionLock };
    408             encoder->connection = m_connection;
     432            Locker locker { m_encodersConnectionLock };
     433            connection->send(Messages::LibWebRTCCodecsProxy::CreateEncoder { encoder->identifier, formatNameFromCodecType(type), parameters, RuntimeEnabledFeatures::sharedFeatures().webRTCH264LowLatencyEncoderEnabled() }, 0);
     434            setEncoderConnection(*encoder, connection.ptr());
    409435        }
    410436
    411         encoder->connection->send(Messages::LibWebRTCCodecsProxy::CreateEncoder { encoder->identifier, formatNameFromCodecType(type), parameters, RuntimeEnabledFeatures::sharedFeatures().webRTCH264LowLatencyEncoderEnabled() }, 0);
    412437        encoder->parameters = WTFMove(parameters);
    413 
    414         Locker locker { m_encodersLock };
    415438        auto encoderIdentifier = encoder->identifier;
    416439        ASSERT(!m_encoders.contains(encoderIdentifier));
    … …  
    422445int32_t LibWebRTCCodecs::releaseEncoder(Encoder& encoder)
    423446{
     447    ASSERT(!isMainRunLoop());
     448
    424449#if ASSERT_ENABLED
    425450    {
    … …  
    429454#endif
    430455    ensureGPUProcessConnectionAndDispatchToThread([this, encoderIdentifier = encoder.identifier] {
    431         Locker locker { m_encodersLock };
     456        assertIsCurrent(workQueue());
     457
    432458        ASSERT(m_encoders.contains(encoderIdentifier));
    433459        auto encoder = m_encoders.take(encoderIdentifier);
    434         encoder->connection->send(Messages::LibWebRTCCodecsProxy::ReleaseEncoder { encoderIdentifier }, 0);
     460
     461        Locker locker { m_encodersConnectionLock };
     462        encoderConnection(*encoder)->send(Messages::LibWebRTCCodecsProxy::ReleaseEncoder { encoderIdentifier }, 0);
     463
    435464        gpuProcessConnectionMayNoLongerBeNeeded();
    436465    });
    … …  
    440469int32_t LibWebRTCCodecs::initializeEncoder(Encoder& encoder, uint16_t width, uint16_t height, unsigned startBitRate, unsigned maxBitRate, unsigned minBitRate, uint32_t maxFrameRate)
    441470{
     471    ASSERT(!isMainRunLoop());
     472
    442473    ensureGPUProcessConnectionAndDispatchToThread([this, encoderIdentifier = encoder.identifier, width, height, startBitRate, maxBitRate, minBitRate, maxFrameRate]() mutable {
     474        assertIsCurrent(workQueue());
     475
    443476        auto* encoder = m_encoders.get(encoderIdentifier);
    444477        encoder->initializationData = EncoderInitializationData { width, height, startBitRate, maxBitRate, minBitRate, maxFrameRate };
    445         encoder->connection->send(Messages::LibWebRTCCodecsProxy::InitializeEncoder { encoderIdentifier, width, height, startBitRate, maxBitRate, minBitRate, maxFrameRate }, 0);
     478
     479        Locker locker { m_encodersConnectionLock };
     480        encoderConnection(*encoder)->send(Messages::LibWebRTCCodecsProxy::InitializeEncoder { encoderIdentifier, width, height, startBitRate, maxBitRate, minBitRate, maxFrameRate }, 0);
    446481    });
    447482    return 0;
    … …  
    449484
    450485template<typename Buffer>
    451 bool copySharedVideoFrame(LibWebRTCCodecs::Encoder& encoder, Buffer&& frameBuffer)
     486bool LibWebRTCCodecs::copySharedVideoFrame(LibWebRTCCodecs::Encoder& encoder, IPC::Connection& connection, Buffer&& frameBuffer)
    452487{
    453488    return encoder.sharedVideoFrameWriter.write(frameBuffer,
    454         [&](auto& semaphore) { encoder.connection->send(Messages::LibWebRTCCodecsProxy::SetSharedVideoFrameSemaphore { encoder.identifier, semaphore }, 0); },
    455         [&](auto& handle) { encoder.connection->send(Messages::LibWebRTCCodecsProxy::SetSharedVideoFrameMemory { encoder.identifier, handle }, 0); }
     489        [&](auto& semaphore) { connection.send(Messages::LibWebRTCCodecsProxy::SetSharedVideoFrameSemaphore { encoder.identifier, semaphore }, 0); },
     490        [&](auto& handle) { connection.send(Messages::LibWebRTCCodecsProxy::SetSharedVideoFrameMemory { encoder.identifier, handle }, 0); }
    456491    );
    457492}
    … …  
    459494int32_t LibWebRTCCodecs::encodeFrame(Encoder& encoder, const webrtc::VideoFrame& frame, bool shouldEncodeAsKeyFrame)
    460495{
    461     Locker locker { m_encodersLock };
    462     if (!encoder.connection)
     496    ASSERT(!isMainRunLoop());
     497
     498    Locker locker { m_encodersConnectionLock };
     499    auto* connection = encoderConnection(encoder);
     500    if (!connection)
    463501        return WEBRTC_VIDEO_CODEC_ERROR;
    464502
    … …  
    475513        if (!buffer) {
    476514            // buffer is not native, we need to copy to shared video frame.
    477             if (!copySharedVideoFrame(encoder, frame))
     515            if (!copySharedVideoFrame(encoder, *connection, frame))
    478516                return WEBRTC_VIDEO_CODEC_ERROR;
    479517        }
    … …  
    483521    if (buffer && !sample->surface()) {
    484522        // buffer is not IOSurface, we need to copy to shared video frame.
    485         if (!copySharedVideoFrame(encoder, buffer.get()))
     523        if (!copySharedVideoFrame(encoder, *connection, buffer.get()))
    486524            return WEBRTC_VIDEO_CODEC_ERROR;
    487525    }
    488526
    489     encoder.connection->send(Messages::LibWebRTCCodecsProxy::EncodeFrame { encoder.identifier, *sample, frame.timestamp(), shouldEncodeAsKeyFrame, remoteVideoFrameReadReference }, 0);
     527    connection->send(Messages::LibWebRTCCodecsProxy::EncodeFrame { encoder.identifier, *sample, frame.timestamp(), shouldEncodeAsKeyFrame, remoteVideoFrameReadReference }, 0);
    490528    return WEBRTC_VIDEO_CODEC_OK;
    491529}
    … …  
    493531void LibWebRTCCodecs::registerEncodeFrameCallback(Encoder& encoder, void* encodedImageCallback)
    494532{
     533    ASSERT(!isMainRunLoop());
     534
    495535    Locker locker { encoder.encodedImageCallbackLock };
    496536
    … …  
    500540void LibWebRTCCodecs::setEncodeRates(Encoder& encoder, uint32_t bitRate, uint32_t frameRate)
    501541{
    502     Locker locker { m_encodersLock };
    503 
    504     if (!encoder.connection) {
     542    ASSERT(!isMainRunLoop());
     543
     544    Locker locker { m_encodersConnectionLock };
     545
     546    auto* connection = encoderConnection(encoder);
     547    if (!connection) {
    505548        callOnMainRunLoop([encoderIdentifier = encoder.identifier, bitRate, frameRate] {
    506549            WebProcess::singleton().ensureGPUProcessConnection().connection().send(Messages::LibWebRTCCodecsProxy::SetEncodeRates { encoderIdentifier, bitRate, frameRate }, 0);
    … …  
    508551        return;
    509552    }
    510     encoder.connection->send(Messages::LibWebRTCCodecsProxy::SetEncodeRates { encoder.identifier, bitRate, frameRate }, 0);
     553    connection->send(Messages::LibWebRTCCodecsProxy::SetEncodeRates { encoder.identifier, bitRate, frameRate }, 0);
    511554}
    512555
    513556void LibWebRTCCodecs::completedEncoding(RTCEncoderIdentifier identifier, IPC::DataReference&& data, const webrtc::WebKitEncodedFrameInfo& info)
    514557{
    515     ASSERT(!isMainRunLoop());
     558    assertIsCurrent(workQueue());
    516559
    517560    // FIXME: Do error logging.
    … …  
    554597{
    555598    ASSERT(isMainRunLoop());
     599
    556600    Locker locker { m_connectionLock };
    557601    std::exchange(m_connection, nullptr)->removeThreadMessageReceiver(Messages::LibWebRTCCodecs::messageReceiverName());
    … …  
    561605    ensureGPUProcessConnectionOnMainThreadWithLock();
    562606    dispatchToThread([this, connection = m_connection]() {
    563         for (auto& decoder : m_decoders.values()) {
    564             createRemoteDecoder(*decoder, *connection, m_useRemoteFrames);
    565             decoder->connection = connection.get();
     607        assertIsCurrent(workQueue());
     608        {
     609            Locker locker { m_connectionLock };
     610            for (auto& decoder : m_decoders.values()) {
     611                createRemoteDecoder(*decoder, *connection, m_useRemoteFrames);
     612                setDecoderConnection(*decoder, connection.get());
     613            }
    566614        }
    567615
    … …  
    570618            encoder->sharedVideoFrameWriter.disable();
    571619
    572         Locker locker { m_encodersLock };
     620        Locker locker { m_encodersConnectionLock };
    573621        for (auto& encoder : m_encoders.values()) {
    574622            connection->send(Messages::LibWebRTCCodecsProxy::CreateEncoder { encoder->identifier, formatNameFromWebRTCCodecType(encoder->codecType), encoder->parameters, RuntimeEnabledFeatures::sharedFeatures().webRTCH264LowLatencyEncoderEnabled() }, 0);
    575623            if (encoder->initializationData)
    576624                connection->send(Messages::LibWebRTCCodecsProxy::InitializeEncoder { encoder->identifier, encoder->initializationData->width, encoder->initializationData->height, encoder->initializationData->startBitRate, encoder->initializationData->maxBitRate, encoder->initializationData->minBitRate, encoder->initializationData->maxFrameRate }, 0);
    577             encoder->connection = connection.get();
     625            setEncoderConnection(*encoder, connection.get());
    578626            encoder->sharedVideoFrameWriter = { };
    579627        }
    … …  
    590638}
    591639
     640IPC::Connection* LibWebRTCCodecs::encoderConnection(Encoder& encoder)
     641{
     642    return encoder.connection.get();
     643}
     644
     645void LibWebRTCCodecs::setEncoderConnection(Encoder& encoder, RefPtr<IPC::Connection>&& connection)
     646{
     647    encoder.connection = WTFMove(connection);
     648}
     649
     650IPC::Connection* LibWebRTCCodecs::decoderConnection(Decoder& decoder)
     651{
     652    return decoder.connection.get();
     653}
     654
     655void LibWebRTCCodecs::setDecoderConnection(Decoder& decoder, RefPtr<IPC::Connection>&& connection)
     656{
     657    decoder.connection = WTFMove(connection);
     658}
     659
    592660}
    593661

  • trunk/Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h

    r290555 r290653  
    145145    void gpuProcessConnectionDidClose(GPUProcessConnection&);
    146146
     147    IPC::Connection* encoderConnection(Encoder&) WTF_REQUIRES_LOCK(m_encodersConnectionLock);
     148    void setEncoderConnection(Encoder&, RefPtr<IPC::Connection>&&)  WTF_REQUIRES_LOCK(m_encodersConnectionLock);
     149    IPC::Connection* decoderConnection(Decoder&) WTF_REQUIRES_LOCK(m_connectionLock);
     150    void setDecoderConnection(Decoder&, RefPtr<IPC::Connection>&&) WTF_REQUIRES_LOCK(m_connectionLock);
     151
     152    template<typename Buffer> bool copySharedVideoFrame(LibWebRTCCodecs::Encoder&, IPC::Connection&, Buffer&&);
     153    WorkQueue& workQueue() const { return m_queue; }
     154
    147155private:
    148     HashMap<RTCDecoderIdentifier, std::unique_ptr<Decoder>> m_decoders;
    149     HashSet<RTCDecoderIdentifier> m_decodingErrors;
     156    HashMap<RTCDecoderIdentifier, std::unique_ptr<Decoder>> m_decoders WTF_GUARDED_BY_CAPABILITY(workQueue());
    150157
    151     Lock m_encodersLock;
    152     HashMap<RTCEncoderIdentifier, std::unique_ptr<Encoder>> m_encoders;
     158    Lock m_encodersConnectionLock;
     159    HashMap<RTCEncoderIdentifier, std::unique_ptr<Encoder>> m_encoders WTF_GUARDED_BY_CAPABILITY(workQueue());
    153160
    154161    std::atomic<bool> m_needsGPUProcessConnection;
Note: See TracChangeset for help on using the changeset viewer.