Attachment 373605 Details for Bug 199538 – Patch

[patch] Patch

bug-199538-20190707155229.patch (text/plain), 12.99 KB, created by youenn fablet on 2019-07-07 15:52:30 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-07-07 15:52:30 PDT

Size: 12.99 KB

patch

obsolete

>Subversion Revision: 247200
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7b8f6a802ddbb804ed7015c8783704f7158d8d63..f8b8ea6866a24c5ee497110ac0eb50698b0c0261 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,45 @@
>+2019-07-07  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 NOBODY (OOPS!).
>+
>+        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-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
>diff --git a/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp b/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp
>index 4da133bd1c75dd3e557293ac1116d91a66181aca..adef680a4e260eddee58a6eb0249d1f184506170 100644
>--- a/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp
>+++ b/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp
>@@ -242,6 +242,7 @@ bool MediaStreamTrackPrivate::preventSourceFromStopping()
> 
> void MediaStreamTrackPrivate::videoSampleAvailable(MediaSample& mediaSample)
> {
>+    ASSERT(isMainThread());
>     if (!m_haveProducedData) {
>         m_haveProducedData = true;
>         updateReadyState();
>@@ -259,9 +260,15 @@ void MediaStreamTrackPrivate::videoSampleAvailable(MediaSample& mediaSample)
> // May get called on a background thread.
> 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;
>     }
> 
>     forEachObserver([&](auto& observer) {
>@@ -269,7 +276,6 @@ void MediaStreamTrackPrivate::audioSamplesAvailable(const MediaTime& mediaTime,
>     });
> }
> 
>-
> void MediaStreamTrackPrivate::updateReadyState()
> {
>     ReadyState state = ReadyState::None;
>diff --git a/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h b/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h
>index efa673cf09ac0bfc95868b6d3a4d872e49194caf..375905862d83ef9ed42c4d7de23656476fe6f70f 100644
>--- a/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h
>+++ b/Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h
>@@ -41,7 +41,7 @@ class RealtimeMediaSourceCapabilities;
> class WebAudioSourceProvider;
> 
> class MediaStreamTrackPrivate final
>-    : public RefCounted<MediaStreamTrackPrivate>
>+    : public ThreadSafeRefCounted<MediaStreamTrackPrivate, WTF::DestructionThread::Main>
>     , public RealtimeMediaSource::Observer
> #if !RELEASE_LOG_DISABLED
>     , private LoggerHelper
>@@ -151,6 +151,7 @@ private:
>     bool m_isEnabled { true };
>     bool m_isEnded { false };
>     bool m_haveProducedData { false };
>+    bool m_hasSentStartProducedData { false };
>     HintValue m_contentHint { HintValue::Empty };
>     RefPtr<WebAudioSourceProvider> m_audioSourceProvider;
>     Ref<const Logger> m_logger;
>diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp b/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
>index e9757e2d092c6db57fa3cd3f026a99bbc21e9e1e..6260e37e111adf7d2e3a2ac800ae22001a79fc57 100644
>--- a/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
>+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
>@@ -1035,13 +1035,10 @@ void RealtimeMediaSource::setEchoCancellation(bool echoCancellation)
>     notifySettingsDidChangeObservers(RealtimeMediaSourceSettings::Flag::EchoCancellation);
> }
> 
>-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();
>     });
> }
>diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSource.h b/Source/WebCore/platform/mediastream/RealtimeMediaSource.h
>index 226491f8d24e719f44978aa065a459229c32631d..7ab4ab22efa4a5fd7c758cc7000ad02c407d7dbe 100644
>--- a/Source/WebCore/platform/mediastream/RealtimeMediaSource.h
>+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSource.h
>@@ -67,7 +67,7 @@ class RemoteVideoSample;
> struct CaptureSourceOrError;
> 
> class WEBCORE_EXPORT RealtimeMediaSource
>-    : public ThreadSafeRefCounted<RealtimeMediaSource>
>+    : public ThreadSafeRefCounted<RealtimeMediaSource, WTF::DestructionThread::MainRunLoop>
>     , public CanMakeWeakPtr<RealtimeMediaSource>
> #if !RELEASE_LOG_DISABLED
>     , private LoggerHelper
>diff --git a/Source/WebCore/platform/mediastream/mac/MockRealtimeAudioSourceMac.mm b/Source/WebCore/platform/mediastream/mac/MockRealtimeAudioSourceMac.mm
>index 76766067e66c140997d0fee37c59f67d57ba2db0..b2968d3c6b217592231b604b4fefb921e28ff4e0 100644
>--- a/Source/WebCore/platform/mediastream/mac/MockRealtimeAudioSourceMac.mm
>+++ b/Source/WebCore/platform/mediastream/mac/MockRealtimeAudioSourceMac.mm
>@@ -106,10 +106,12 @@ CaptureSourceOrError MockRealtimeAudioSource::create(String&& deviceID, String&&
> MockRealtimeAudioSourceMac::MockRealtimeAudioSourceMac(String&& deviceID, String&& name, String&& hashSalt)
>     : MockRealtimeAudioSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))
> {
>+    ASSERT(isMainThread());
> }
> 
> void MockRealtimeAudioSourceMac::emitSampleBuffers(uint32_t frameCount)
> {
>+    ASSERT(!isMainThread());
>     ASSERT(m_formatDescription);
> 
>     CMTime startTime = CMTimeMake(m_samplesEmitted, sampleRate());
>@@ -120,6 +122,7 @@ void MockRealtimeAudioSourceMac::emitSampleBuffers(uint32_t frameCount)
> 
> void MockRealtimeAudioSourceMac::reconfigure()
> {
>+    ASSERT(!isMainThread());
>     m_maximiumFrameCount = WTF::roundUpToPowerOfTwo(renderInterval().seconds() * sampleRate() * 2);
>     ASSERT(m_maximiumFrameCount);
> 
>@@ -140,6 +143,7 @@ void MockRealtimeAudioSourceMac::reconfigure()
> 
> void MockRealtimeAudioSourceMac::render(Seconds delta)
> {
>+    ASSERT(!isMainThread());
>     if (!m_audioBufferList)
>         reconfigure();
> 
>@@ -168,21 +172,23 @@ void MockRealtimeAudioSourceMac::render(Seconds delta)
> void MockRealtimeAudioSourceMac::settingsDidChange(OptionSet<RealtimeMediaSourceSettings::Flag> settings)
> {
>     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);
>+        });
>     }
> 
>     MockRealtimeAudioSource::settingsDidChange(settings);
>diff --git a/Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp b/Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp
>index a14aee21e3172a6b326e2c1987fc81acf487d080..8d9ed876e3a9c955fac55e2398f79b6e6da40cb1 100644
>--- a/Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp
>+++ b/Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp
>@@ -62,6 +62,7 @@ CaptureSourceOrError MockRealtimeAudioSource::create(String&& deviceID, String&&
> 
> 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)
> {
>     auto device = MockRealtimeMediaSourceCenter::mockDeviceWithPersistentID(persistentID());
>@@ -151,7 +152,10 @@ void MockRealtimeAudioSource::tick()
> 
>     Seconds delta = now - m_lastRenderTime;
>     m_lastRenderTime = now;
>-    render(delta);
>+
>+    m_workQueue->dispatch([this, delta, protectedThis = makeRef(*this)] {
>+        render(delta);
>+    });
> }
> 
> void MockRealtimeAudioSource::delaySamples(Seconds delta)
>diff --git a/Source/WebCore/platform/mock/MockRealtimeAudioSource.h b/Source/WebCore/platform/mock/MockRealtimeAudioSource.h
>index a2f94b7ce7eeb66111239a526a79d60db5725682..90e3baab6a276d9e2a0a21115a6ade580063c73e 100644
>--- a/Source/WebCore/platform/mock/MockRealtimeAudioSource.h
>+++ b/Source/WebCore/platform/mock/MockRealtimeAudioSource.h
>@@ -36,23 +36,19 @@
> #include "MockMediaDevice.h"
> #include "RealtimeMediaSourceFactory.h"
> #include <wtf/RunLoop.h>
>+#include <wtf/WorkQueue.h>
> 
> namespace WebCore {
> 
> class MockRealtimeAudioSource : public RealtimeMediaSource {
> public:
>-
>     static CaptureSourceOrError create(String&& deviceID, String&& name, String&& hashSalt, const MediaConstraints*);
>-
>     virtual ~MockRealtimeAudioSource();
> 
> protected:
>     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;
> 
>     static Seconds renderInterval() { return 60_ms; }
>@@ -61,13 +57,20 @@ private:
>     const RealtimeMediaSourceCapabilities& capabilities() final;
>     const RealtimeMediaSourceSettings& settings() final;
> 
>-    void tick();
>+    void startProducingData() final;
>+    void stopProducingData() final;
> 
>     bool isCaptureSource() const final { return true; }
>     CaptureDevice::DeviceType deviceType() const final { return CaptureDevice::DeviceType::Microphone; }
> 
>     void delaySamples(Seconds) final;
> 
>+    void tick();
>+
>+protected:
>+    Ref<WorkQueue> m_workQueue;
>+
>+private:
>     Optional<RealtimeMediaSourceCapabilities> m_capabilities;
>     Optional<RealtimeMediaSourceSettings> m_currentSettings;
>     RealtimeMediaSourceSupportedConstraints m_supportedConstraints;

Actions: View | Formatted Diff | Diff

Attachments on bug 199538: 373605 | 373608