m_parser can be concurrently accessed by SourceBufferPrivateAVFObjC

https://bugs.webkit.org/show_bug.cgi?id=265503 rdar://118913735 Reviewed by Youenn Fablet. We ensure that m_parser is only ever used on the parser queue. Additionally, remove the non-functional SourceBufferParser::setShouldProvideMediaDataForTrackID API and SourceBufferParser::shouldProvideMediadataForTrackID which was never used. * Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.mm: (WebCore::SourceBufferParserAVFObjC::setShouldProvideMediaDataForTrackID): Deleted. (WebCore::SourceBufferParserAVFObjC::shouldProvideMediadataForTrackID): Deleted. * Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm: (WebCore::SourceBufferPrivateAVFObjC::appendInternal): (WebCore::SourceBufferPrivateAVFObjC::trackDidChangeSelected): (WebCore::SourceBufferPrivateAVFObjC::trackDidChangeEnabled): (WebCore::SourceBufferPrivateAVFObjC::streamDataParser const): Deleted. * Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h: * Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp: (WebCore::SourceBufferParserWebM::setShouldProvideMediaDataForTrackID): Deleted. (WebCore::SourceBufferParserWebM::shouldProvideMediadataForTrackID): Deleted. * Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h: Canonical link: https://commits.webkit.org/271270@main
WebKit · Nov 29, 2023 · 98e1d75 · 98e1d75
1 parent 3991417
commit 98e1d75
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 100 deletions.
diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.h b/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.h
@@ -60,8 +60,6 @@ class SourceBufferParserAVFObjC final
     Type type() const { return Type::AVFObjC; }
     Expected<void, PlatformMediaError> appendData(Segment&&, AppendFlags = AppendFlags::None) final;
     void flushPendingMediaData() final;
-    void setShouldProvideMediaDataForTrackID(bool, uint64_t) final;
-    bool shouldProvideMediadataForTrackID(uint64_t) final;
     void resetParserState() final;
     void invalidate() final;
 #if !RELEASE_LOG_DISABLED

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.mm
@@ -267,20 +267,6 @@ AtomString codec() const override
     [m_parser providePendingMediaData];
 }
 
-void SourceBufferParserAVFObjC::setShouldProvideMediaDataForTrackID(bool should, uint64_t trackID)
-{
-    INFO_LOG_IF_POSSIBLE(LOGIDENTIFIER, "should = ", should, ", trackID = ", trackID);
-    BEGIN_BLOCK_OBJC_EXCEPTIONS
-    [m_parser setShouldProvideMediaData:should forTrackID:trackID];
-    END_BLOCK_OBJC_EXCEPTIONS
-}
-
-bool SourceBufferParserAVFObjC::shouldProvideMediadataForTrackID(uint64_t trackID)
-{
-    INFO_LOG_IF_POSSIBLE(LOGIDENTIFIER, "trackID = ", trackID);
-    return [m_parser shouldProvideMediaDataForTrackID:trackID];
-}
-
 void SourceBufferParserAVFObjC::resetParserState()
 {
     INFO_LOG_IF_POSSIBLE(LOGIDENTIFIER);

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h b/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h
@@ -112,11 +112,13 @@ class SourceBufferPrivateAVFObjC final
     uint64_t protectedTrackID() const { return m_protectedTrackID; }
     bool needsVideoLayer() const;
 
-    AVStreamDataParser* streamDataParser() const;
+#if (ENABLE(ENCRYPTED_MEDIA) && HAVE(AVCONTENTKEYSESSION)) || ENABLE(LEGACY_ENCRYPTED_MEDIA)
+    AVStreamDataParser* streamDataParser() const { return m_streamDataParser.get(); }
     void setCDMSession(LegacyCDMSession*) final;
     void setCDMInstance(CDMInstance*) final;
     void attemptToDecrypt() final;
     bool waitingForKey() const final { return m_waitingForKey; }
+#endif
 
     void flush();
     void flushIfNeeded();
@@ -205,7 +207,7 @@ ALLOW_NEW_API_WITHOUT_GUARDS_END
     HashMap<AtomString, RefPtr<AudioTrackPrivate>> m_audioTracks;
     Vector<SourceBufferPrivateAVFObjCErrorClient*> m_errorClients;
 
-    Ref<SourceBufferParser> m_parser;
+    const Ref<SourceBufferParser> m_parser;
     Vector<Function<void()>> m_pendingTrackChangeTasks;
     Deque<std::pair<uint64_t, Ref<MediaSampleAVFObjC>>> m_blockedSamples;
 
@@ -253,7 +255,9 @@ ALLOW_NEW_API_WITHOUT_GUARDS_END
     uint64_t m_enabledVideoTrackID { notFound };
     uint64_t m_protectedTrackID { notFound };
     uint64_t m_mapID;
-    uint32_t m_abortCalled { 0 };
+#if ENABLE(ENCRYPTED_MEDIA) && HAVE(AVCONTENTKEYSESSION)
+    RetainPtr<AVStreamDataParser> m_streamDataParser;
+#endif
 
 #if !RELEASE_LOG_DISABLED
     Ref<const Logger> m_logger;

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm
@@ -353,6 +353,7 @@ static bool sampleBufferRenderersSupportKeySession()
     , m_appendQueue(WorkQueue::create("SourceBufferPrivateAVFObjC data parser queue"))
 #if ENABLE(ENCRYPTED_MEDIA) && HAVE(AVCONTENTKEYSESSION)
     , m_keyStatusesChangedObserver(makeUniqueRef<Observer<void()>>([this] { keyStatusesChanged(); }))
+    , m_streamDataParser(is<SourceBufferParserAVFObjC>(m_parser) ? downcast<SourceBufferParserAVFObjC>(m_parser)->streamDataParser() : nil)
 #endif
 #if !RELEASE_LOG_DISABLED
     , m_logger(parent.logger())
@@ -583,70 +584,71 @@ static bool sampleBufferRenderersSupportKeySession()
     ASSERT(!m_hasSessionSemaphore);
     ASSERT(!m_abortSemaphore);
 
-    m_parser->setDidParseInitializationDataCallback([weakThis = ThreadSafeWeakPtr { *this }] (InitializationSegment&& segment) {
-        ASSERT(isMainThread());
-        if (RefPtr protectedThis = weakThis.get())
-            protectedThis->didReceiveInitializationSegment(WTFMove(segment));
-    });
-
-    m_parser->setDidProvideMediaDataCallback([weakThis = ThreadSafeWeakPtr { *this }] (Ref<MediaSampleAVFObjC>&& sample, uint64_t trackId, const String& mediaType) {
-        ASSERT(isMainThread());
-        if (RefPtr protectedThis = weakThis.get())
-            protectedThis->didProvideMediaDataForTrackId(WTFMove(sample), trackId, mediaType);
-    });
+    m_abortSemaphore = Box<Semaphore>::create(0);
+    return invokeAsync(m_appendQueue, [data = WTFMove(data), parser = m_parser, weakThis = ThreadSafeWeakPtr { *this }, abortSemaphore = m_abortSemaphore]() mutable {
+        parser->setDidParseInitializationDataCallback([weakThis] (InitializationSegment&& segment) {
+            ASSERT(isMainThread());
+            if (RefPtr protectedThis = weakThis.get())
+                protectedThis->didReceiveInitializationSegment(WTFMove(segment));
+        });
 
-    m_parser->setDidUpdateFormatDescriptionForTrackIDCallback([weakThis = ThreadSafeWeakPtr { *this }, this, abortCalled = m_abortCalled] (Ref<TrackInfo>&& formatDescription, uint64_t trackId) {
-        ASSERT(isMainThread());
-        if (RefPtr protectedThis = weakThis.get(); protectedThis && abortCalled == m_abortCalled)
-            protectedThis->didUpdateFormatDescriptionForTrackId(WTFMove(formatDescription), trackId);
-    });
+        parser->setDidProvideMediaDataCallback([weakThis] (Ref<MediaSampleAVFObjC>&& sample, uint64_t trackId, const String& mediaType) {
+            ASSERT(isMainThread());
+            if (RefPtr protectedThis = weakThis.get())
+                protectedThis->didProvideMediaDataForTrackId(WTFMove(sample), trackId, mediaType);
+        });
 
-    m_abortSemaphore = Box<Semaphore>::create(0);
-    m_parser->setWillProvideContentKeyRequestInitializationDataForTrackIDCallback([abortSemaphore = m_abortSemaphore] (uint64_t) mutable {
-        // We must call synchronously to the main thread, as the AVStreamSession must be associated
-        // with the streamDataParser before the delegate method returns.
-        Box<BinarySemaphore> respondedSemaphore = Box<BinarySemaphore>::create();
-        callOnMainThread([respondedSemaphore]() {
-            respondedSemaphore->signal();
+        parser->setDidUpdateFormatDescriptionForTrackIDCallback([weakThis] (Ref<TrackInfo>&& formatDescription, uint64_t trackId) {
+            ASSERT(isMainThread());
+            if (RefPtr protectedThis = weakThis.get(); protectedThis)
+                protectedThis->didUpdateFormatDescriptionForTrackId(WTFMove(formatDescription), trackId);
         });
 
-        while (true) {
-            if (respondedSemaphore->waitFor(100_ms))
-                return;
 
-            if (abortSemaphore->waitFor(100_ms)) {
-                abortSemaphore->signal();
-                return;
-            }
-        }
-    });
+        parser->setWillProvideContentKeyRequestInitializationDataForTrackIDCallback([abortSemaphore] (uint64_t) mutable {
+            // We must call synchronously to the main thread, as the AVStreamSession must be associated
+            // with the streamDataParser before the delegate method returns.
+            Box<BinarySemaphore> respondedSemaphore = Box<BinarySemaphore>::create();
+            callOnMainThread([respondedSemaphore]() {
+                respondedSemaphore->signal();
+            });
 
-    m_parser->setDidProvideContentKeyRequestInitializationDataForTrackIDCallback([weakThis = ThreadSafeWeakPtr { *this }, abortSemaphore = m_abortSemaphore](Ref<SharedBuffer>&& initData, uint64_t trackID) mutable {
-        // Called on the data parser queue.
-        Box<BinarySemaphore> hasSessionSemaphore = Box<BinarySemaphore>::create();
-        callOnMainThread([weakThis, initData = WTFMove(initData), trackID, hasSessionSemaphore] () mutable {
-            if (RefPtr protectedThis = weakThis.get())
-                protectedThis->didProvideContentKeyRequestInitializationDataForTrackID(WTFMove(initData), trackID, hasSessionSemaphore);
+            while (true) {
+                if (respondedSemaphore->waitFor(100_ms))
+                    return;
+
+                if (abortSemaphore->waitFor(100_ms)) {
+                    abortSemaphore->signal();
+                    return;
+                }
+            }
         });
 
-        while (true) {
-            if (hasSessionSemaphore->waitFor(100_ms))
-                return;
+        parser->setDidProvideContentKeyRequestInitializationDataForTrackIDCallback([weakThis, abortSemaphore](Ref<SharedBuffer>&& initData, uint64_t trackID) mutable {
+            // Called on the data parser queue.
+            Box<BinarySemaphore> hasSessionSemaphore = Box<BinarySemaphore>::create();
+            callOnMainThread([weakThis, initData = WTFMove(initData), trackID, hasSessionSemaphore] () mutable {
+                if (RefPtr protectedThis = weakThis.get())
+                    protectedThis->didProvideContentKeyRequestInitializationDataForTrackID(WTFMove(initData), trackID, hasSessionSemaphore);
+            });
 
-            if (abortSemaphore->waitFor(100_ms)) {
-                abortSemaphore->signal();
-                return;
+            while (true) {
+                if (hasSessionSemaphore->waitFor(100_ms))
+                    return;
+
+                if (abortSemaphore->waitFor(100_ms)) {
+                    abortSemaphore->signal();
+                    return;
+                }
             }
-        }
-    });
+        });
 
-    m_parser->setDidProvideContentKeyRequestIdentifierForTrackIDCallback([weakThis = ThreadSafeWeakPtr { *this }] (Ref<SharedBuffer>&& initData, uint64_t trackID) {
-        ASSERT(isMainThread());
-        if (RefPtr protectedThis = weakThis.get())
-            protectedThis->didProvideContentKeyRequestInitializationDataForTrackID(WTFMove(initData), trackID, nullptr);
-    });
+        parser->setDidProvideContentKeyRequestIdentifierForTrackIDCallback([weakThis] (Ref<SharedBuffer>&& initData, uint64_t trackID) {
+            ASSERT(isMainThread());
+            if (RefPtr protectedThis = weakThis.get())
+                protectedThis->didProvideContentKeyRequestInitializationDataForTrackID(WTFMove(initData), trackID, nullptr);
+        });
 
-    return invokeAsync(m_appendQueue, [data = WTFMove(data), parser = m_parser]() mutable {
         return MediaPromise::createAndSettle(parser->appendData(WTFMove(data)));
     })->whenSettled(RunLoop::current(), [weakThis = ThreadSafeWeakPtr { *this }](auto&& result) {
         if (RefPtr protectedThis = weakThis.get())
@@ -783,14 +785,10 @@ static bool sampleBufferRenderersSupportKeySession()
 
     if (!selected && m_enabledVideoTrackID == trackID) {
         m_enabledVideoTrackID = -1;
-        m_parser->setShouldProvideMediaDataForTrackID(false, trackID);
-
         if (m_decompressionSession)
             m_decompressionSession->stopRequestingMediaData();
     } else if (selected) {
         m_enabledVideoTrackID = trackID;
-        m_parser->setShouldProvideMediaDataForTrackID(true, trackID);
-
         if (m_decompressionSession) {
             m_decompressionSession->requestMediaDataWhenReady([weakThis = ThreadSafeWeakPtr { *this }, trackID] {
                 if (RefPtr protectedThis = weakThis.get())
@@ -816,11 +814,9 @@ static bool sampleBufferRenderersSupportKeySession()
 ALLOW_NEW_API_WITHOUT_GUARDS_BEGIN
         RetainPtr<AVSampleBufferAudioRenderer> renderer = m_audioRenderers.get(trackID);
 ALLOW_NEW_API_WITHOUT_GUARDS_END
-        m_parser->setShouldProvideMediaDataForTrackID(false, trackID);
         if (RefPtr player = this->player())
             player->removeAudioRenderer(renderer.get());
     } else {
-        m_parser->setShouldProvideMediaDataForTrackID(true, trackID);
 ALLOW_NEW_API_WITHOUT_GUARDS_BEGIN
         RetainPtr<AVSampleBufferAudioRenderer> renderer;
 ALLOW_NEW_API_WITHOUT_GUARDS_END
@@ -855,14 +851,7 @@ static bool sampleBufferRenderersSupportKeySession()
             player->addAudioRenderer(renderer.get());
     }
 }
-
-AVStreamDataParser* SourceBufferPrivateAVFObjC::streamDataParser() const
-{
-    if (auto avfParser = dynamicDowncast<SourceBufferParserAVFObjC>(m_parser))
-        return avfParser->streamDataParser();
-    return nil;
-}
-
+#if (ENABLE(ENCRYPTED_MEDIA) && HAVE(AVCONTENTKEYSESSION)) || ENABLE(LEGACY_ENCRYPTED_MEDIA)
 void SourceBufferPrivateAVFObjC::setCDMSession(LegacyCDMSession* session)
 {
 #if ENABLE(LEGACY_ENCRYPTED_MEDIA)
@@ -952,6 +941,7 @@ static bool sampleBufferRenderersSupportKeySession()
     m_waitingForKey = false;
 #endif
 }
+#endif // (ENABLE(ENCRYPTED_MEDIA) && HAVE(AVCONTENTKEYSESSION)) || ENABLE(LEGACY_ENCRYPTED_MEDIA)
 
 bool SourceBufferPrivateAVFObjC::requiresFlush() const
 {

diff --git a/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h b/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h
@@ -98,8 +98,6 @@ class WEBCORE_EXPORT SourceBufferParser : public ThreadSafeRefCounted<SourceBuff
     // Other methods will be called on the main thread, but only once appendData has returned.
     virtual Expected<void, PlatformMediaError> appendData(Segment&&, AppendFlags = AppendFlags::None) = 0;
     virtual void flushPendingMediaData() = 0;
-    virtual void setShouldProvideMediaDataForTrackID(bool, uint64_t) = 0;
-    virtual bool shouldProvideMediadataForTrackID(uint64_t) = 0;
     virtual void resetParserState() = 0;
     virtual void invalidate() = 0;
     virtual void setMinimumAudioSampleDuration(float);

diff --git a/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp b/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp
@@ -1608,17 +1608,6 @@ void SourceBufferParserWebM::flushPendingMediaData()
 {
 }
 
-void SourceBufferParserWebM::setShouldProvideMediaDataForTrackID(bool, uint64_t)
-{
-    notImplemented();
-}
-
-bool SourceBufferParserWebM::shouldProvideMediadataForTrackID(uint64_t)
-{
-    notImplemented();
-    return false;
-}
-
 void SourceBufferParserWebM::invalidate()
 {
     INFO_LOG_IF_POSSIBLE(LOGIDENTIFIER);

diff --git a/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h b/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h
@@ -331,8 +331,6 @@ class SourceBufferParserWebM
     Type type() const { return Type::WebM; }
     WEBCORE_EXPORT Expected<void, PlatformMediaError> appendData(Segment&&, AppendFlags = AppendFlags::None) final;
     void flushPendingMediaData() final;
-    void setShouldProvideMediaDataForTrackID(bool, uint64_t) final;
-    bool shouldProvideMediadataForTrackID(uint64_t) final;
     void resetParserState() final { m_parser.resetState(); }
     void invalidate() final;