Changeset 247211 in webkit

Timestamp:

Jul 8, 2019 10:46:27 AM (5 years ago)

Author:

youenn@apple.com

Message:

MediaStreamTrackPrivate should always call readyStateChanged on the main thread
​https://bugs.webkit.org/show_bug.cgi?id=199538
<rdar://problem/52709106>

Reviewed by Eric Carlson.

MediaStreamTrackPrivate is sometimes calling readyStateChanged in a
background thread inside its audioSamplesAvailable method.
Instead of doing that, we hop to the main thread to call readyStateChanged.
Once the call is made in the main thread, MediaStreamTrackPrivate will
send the audio samples to its observers.

To make mock audio source closer to real capture audio sources,
audioSamplesAvailable is called on a background thread.
RealtimeMediaSource is updated to always be destroyed in the main
run loop since it is WebKit2 only.

Covered by existing tests and making sure the mock audio source calls
the audioSamplesAvailable method on a background thread.

• platform/mediastream/MediaStreamTrackPrivate.cpp:

(WebCore::MediaStreamTrackPrivate::videoSampleAvailable):
(WebCore::MediaStreamTrackPrivate::audioSamplesAvailable):

• platform/mediastream/MediaStreamTrackPrivate.h:
• platform/mediastream/RealtimeMediaSource.cpp:

(WebCore::RealtimeMediaSource::scheduleDeferredTask):
scheduleDeferredTask may be called from a background thread.
It is thus safer to ref the source instead of creating a weak pointer.

• platform/mediastream/RealtimeMediaSource.h:
• platform/mediastream/mac/MockRealtimeAudioSourceMac.mm:

(WebCore::MockRealtimeAudioSourceMac::MockRealtimeAudioSourceMac):
(WebCore::MockRealtimeAudioSourceMac::emitSampleBuffers):
(WebCore::MockRealtimeAudioSourceMac::reconfigure):
(WebCore::MockRealtimeAudioSourceMac::render):
(WebCore::MockRealtimeAudioSourceMac::settingsDidChange):

• platform/mock/MockRealtimeAudioSource.cpp:

(WebCore::MockRealtimeAudioSource::MockRealtimeAudioSource):
(WebCore::MockRealtimeAudioSource::tick):

• platform/mock/MockRealtimeAudioSource.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mediastream/MediaStreamTrackPrivate.cpp (modified) (3 diffs)
• platform/mediastream/MediaStreamTrackPrivate.h (modified) (2 diffs)
• platform/mediastream/RealtimeMediaSource.cpp (modified) (1 diff)
• platform/mediastream/RealtimeMediaSource.h (modified) (1 diff)
• platform/mediastream/mac/MockRealtimeAudioSourceMac.mm (modified) (4 diffs)
• platform/mock/MockRealtimeAudioSource.cpp (modified) (2 diffs)
• platform/mock/MockRealtimeAudioSource.h (modified) (5 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-08  Youenn Fablet  <youenn@apple.com>
+
+        MediaStreamTrackPrivate should always call readyStateChanged on the main thread
+        https://bugs.webkit.org/show_bug.cgi?id=199538
+        <rdar://problem/52709106>
+
+        Reviewed by Eric Carlson.
+
+        MediaStreamTrackPrivate is sometimes calling readyStateChanged in a
+        background thread inside its audioSamplesAvailable method.
+        Instead of doing that, we hop to the main thread to call readyStateChanged.
+        Once the call is made in the main thread, MediaStreamTrackPrivate will
+        send the audio samples to its observers.
+
+        To make mock audio source closer to real capture audio sources,
+        audioSamplesAvailable is called on a background thread.
+        RealtimeMediaSource is updated to always be destroyed in the main
+        run loop since it is WebKit2 only.
+
+        Covered by existing tests and making sure the mock audio source calls
+        the audioSamplesAvailable method on a background thread.
+
+        * platform/mediastream/MediaStreamTrackPrivate.cpp:
+        (WebCore::MediaStreamTrackPrivate::videoSampleAvailable):
+        (WebCore::MediaStreamTrackPrivate::audioSamplesAvailable):
+        * platform/mediastream/MediaStreamTrackPrivate.h:
+        * platform/mediastream/RealtimeMediaSource.cpp:
+        (WebCore::RealtimeMediaSource::scheduleDeferredTask):
+        scheduleDeferredTask may be called from a background thread.
+        It is thus safer to ref the source instead of creating a weak pointer.
+        * platform/mediastream/RealtimeMediaSource.h:
+        * platform/mediastream/mac/MockRealtimeAudioSourceMac.mm:
+        (WebCore::MockRealtimeAudioSourceMac::MockRealtimeAudioSourceMac):
+        (WebCore::MockRealtimeAudioSourceMac::emitSampleBuffers):
+        (WebCore::MockRealtimeAudioSourceMac::reconfigure):
+        (WebCore::MockRealtimeAudioSourceMac::render):
+        (WebCore::MockRealtimeAudioSourceMac::settingsDidChange):
+        * platform/mock/MockRealtimeAudioSource.cpp:
+        (WebCore::MockRealtimeAudioSource::MockRealtimeAudioSource):
+        (WebCore::MockRealtimeAudioSource::tick):
+        * platform/mock/MockRealtimeAudioSource.h:
+
 2019-07-08  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp

@@ -243 +243 @@
 void MediaStreamTrackPrivate::videoSampleAvailable(MediaSample& mediaSample)
 {
+    ASSERT(isMainThread());
     if (!m_haveProducedData) {
         m_haveProducedData = true;
@@ -260 +261 @@
 void MediaStreamTrackPrivate::audioSamplesAvailable(const MediaTime& mediaTime, const PlatformAudioData& data, const AudioStreamDescription& description, size_t sampleCount)
 {
-    if (!m_haveProducedData) {
-        m_haveProducedData = true;
-        updateReadyState();
+    if (!m_hasSentStartProducedData) {
+        callOnMainThread([this, protectedThis = makeRef(*this)] {
+            if (!m_haveProducedData) {
+                m_haveProducedData = true;
+                updateReadyState();
+            }
+            m_hasSentStartProducedData = true;
+        });
+        return;
     }
 
@@ -269 +276 @@
     });
 }
-
 
 void MediaStreamTrackPrivate::updateReadyState()

trunk/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h

@@ -42 +42 @@
 
 class MediaStreamTrackPrivate final
-    : public RefCounted<MediaStreamTrackPrivate>
+    : public ThreadSafeRefCounted<MediaStreamTrackPrivate, WTF::DestructionThread::Main>
     , public RealtimeMediaSource::Observer
 #if !RELEASE_LOG_DISABLED
@@ -152 +152 @@
     bool m_isEnded { false };
     bool m_haveProducedData { false };
+    bool m_hasSentStartProducedData { false };
     HintValue m_contentHint { HintValue::Empty };
     RefPtr<WebAudioSourceProvider> m_audioSourceProvider;

trunk/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp

@@ -1036 +1036 @@
 }
 
-void RealtimeMediaSource::scheduleDeferredTask(WTF::Function<void()>&& function)
+void RealtimeMediaSource::scheduleDeferredTask(Function<void()>&& function)
 {
     ASSERT(function);
-    callOnMainThread([weakThis = makeWeakPtr(*this), function = WTFMove(function)] {
-        if (!weakThis)
-            return;
-
+    callOnMainThread([protectedThis = makeRef(*this), function = WTFMove(function)] {
         function();
     });

trunk/Source/WebCore/platform/mediastream/RealtimeMediaSource.h

@@ -68 +68 @@
 
 class WEBCORE_EXPORT RealtimeMediaSource
-    : public ThreadSafeRefCounted<RealtimeMediaSource>
+    : public ThreadSafeRefCounted<RealtimeMediaSource, WTF::DestructionThread::MainRunLoop>
     , public CanMakeWeakPtr<RealtimeMediaSource>
 #if !RELEASE_LOG_DISABLED

trunk/Source/WebCore/platform/mediastream/mac/MockRealtimeAudioSourceMac.mm

@@ -107 +107 @@
     : MockRealtimeAudioSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))
 {
+    ASSERT(isMainThread());
 }
 
 void MockRealtimeAudioSourceMac::emitSampleBuffers(uint32_t frameCount)
 {
+    ASSERT(!isMainThread());
     ASSERT(m_formatDescription);
 
@@ -121 +123 @@
 void MockRealtimeAudioSourceMac::reconfigure()
 {
+    ASSERT(!isMainThread());
     m_maximiumFrameCount = WTF::roundUpToPowerOfTwo(renderInterval().seconds() * sampleRate() * 2);
     ASSERT(m_maximiumFrameCount);
@@ -141 +144 @@
 void MockRealtimeAudioSourceMac::render(Seconds delta)
 {
+    ASSERT(!isMainThread());
     if (!m_audioBufferList)
         reconfigure();
@@ -169 +173 @@
 {
     if (settings.contains(RealtimeMediaSourceSettings::Flag::SampleRate)) {
-        m_formatDescription = nullptr;
-        m_audioBufferList = nullptr;
+        m_workQueue->dispatch([this, protectedThis = makeRef(*this)] {
+            m_formatDescription = nullptr;
+            m_audioBufferList = nullptr;
 
-        auto rate = sampleRate();
-        size_t sampleCount = 2 * rate;
+            auto rate = sampleRate();
+            size_t sampleCount = 2 * rate;
 
-        m_bipBopBuffer.grow(sampleCount);
-        m_bipBopBuffer.fill(0);
+            m_bipBopBuffer.grow(sampleCount);
+            m_bipBopBuffer.fill(0);
 
-        size_t bipBopSampleCount = ceil(BipBopDuration * rate);
-        size_t bipStart = 0;
-        size_t bopStart = rate;
+            size_t bipBopSampleCount = ceil(BipBopDuration * rate);
+            size_t bipStart = 0;
+            size_t bopStart = rate;
 
-        addHum(BipBopVolume, BipFrequency, rate, 0, m_bipBopBuffer.data() + bipStart, bipBopSampleCount);
-        addHum(BipBopVolume, BopFrequency, rate, 0, m_bipBopBuffer.data() + bopStart, bipBopSampleCount);
+            addHum(BipBopVolume, BipFrequency, rate, 0, m_bipBopBuffer.data() + bipStart, bipBopSampleCount);
+            addHum(BipBopVolume, BopFrequency, rate, 0, m_bipBopBuffer.data() + bopStart, bipBopSampleCount);
+        });
     }
 

trunk/Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp

@@ -63 +63 @@
 MockRealtimeAudioSource::MockRealtimeAudioSource(String&& deviceID, String&& name, String&& hashSalt)
     : RealtimeMediaSource(RealtimeMediaSource::Type::Audio, WTFMove(name), WTFMove(deviceID), WTFMove(hashSalt))
+    , m_workQueue(WorkQueue::create("MockRealtimeAudioSource Render Queue"))
     , m_timer(RunLoop::current(), this, &MockRealtimeAudioSource::tick)
 {
@@ -152 +153 @@
     Seconds delta = now - m_lastRenderTime;
     m_lastRenderTime = now;
-    render(delta);
+
+    m_workQueue->dispatch([this, delta, protectedThis = makeRef(*this)] {
+        render(delta);
+    });
 }
 

trunk/Source/WebCore/platform/mock/MockRealtimeAudioSource.h

@@ -37 +37 @@
 #include "RealtimeMediaSourceFactory.h"
 #include <wtf/RunLoop.h>
+#include <wtf/WorkQueue.h>
 
 namespace WebCore {
@@ -42 +43 @@
 class MockRealtimeAudioSource : public RealtimeMediaSource {
 public:
-
     static CaptureSourceOrError create(String&& deviceID, String&& name, String&& hashSalt, const MediaConstraints*);
-
     virtual ~MockRealtimeAudioSource();
 
@@ -50 +49 @@
     MockRealtimeAudioSource(String&& deviceID, String&& name, String&& hashSalt);
 
-    void startProducingData() final;
-    void stopProducingData() final;
-
-    virtual void render(Seconds) { }
+    virtual void render(Seconds) = 0;
     void settingsDidChange(OptionSet<RealtimeMediaSourceSettings::Flag>) override;
 
@@ -62 +58 @@
     const RealtimeMediaSourceSettings& settings() final;
 
-    void tick();
+    void startProducingData() final;
+    void stopProducingData() final;
 
     bool isCaptureSource() const final { return true; }
@@ -69 +66 @@
     void delaySamples(Seconds) final;
 
+    void tick();
+
+protected:
+    Ref<WorkQueue> m_workQueue;
+
+private:
     Optional<RealtimeMediaSourceCapabilities> m_capabilities;
     Optional<RealtimeMediaSourceSettings> m_currentSettings;