Changeset 252987 in webkit


Ignore:
Timestamp:
Dec 2, 2019 9:26:29 AM (4 years ago)
Author:
youenn@apple.com
Message:

Simplify RealtimeOutgoingAudioSource and RealtimeOutgoingVideooSource observeSource/unobserveSource pattern
​https://bugs.webkit.org/show_bug.cgi?id=204570

Reviewed by Eric Carlson.

Previously, the outgoing source was observing/unobserving its track if sinks were added/removed.
We made LibWebRTCRTCRtpSenderBackend stopping the observing in its destructor.
This makes things complex and we are not always unobserving fast enough for all outgoing sources.

To make it simpler, the outgoing source is no longer handling the observing/unobserving of its track.
Instead, its LibWebRTCRtpSenderBackend is handling it directly.
Observing starts when LibWebRTCRtpSenderBackend gets the track and unobserving happens either on LibWebRTCRtpSenderBackend destruction
or when the LibWebRTCRtpSenderBackend is no longer interested in the track.
This is slight less optimal as we might observe the track even if the source has no sink but this should not happen often in practice.

Removed some unneeded methods.

Covered by existing tests no longer failing a debug ASSERTION.

  • Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp:

(WebCore::LibWebRTCRtpSenderBackend::LibWebRTCRtpSenderBackend):
(WebCore::LibWebRTCRtpSenderBackend::~LibWebRTCRtpSenderBackend):
(WebCore::LibWebRTCRtpSenderBackend::startSource):
(WebCore::LibWebRTCRtpSenderBackend::stopSource):
(WebCore::LibWebRTCRtpSenderBackend::replaceTrack):
(WebCore::LibWebRTCRtpSenderBackend::audioSource):
(WebCore::LibWebRTCRtpSenderBackend::videoSource):
(WebCore::LibWebRTCRtpSenderBackend::hasSource const):
(WebCore::LibWebRTCRtpSenderBackend::setSource):
(WebCore::LibWebRTCRtpSenderBackend::takeSource):

  • Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h:
  • platform/mediastream/RealtimeOutgoingAudioSource.cpp:

(WebCore::RealtimeOutgoingAudioSource::~RealtimeOutgoingAudioSource):
(WebCore::RealtimeOutgoingAudioSource::observeSource):
(WebCore::RealtimeOutgoingAudioSource::setSource):
(WebCore::RealtimeOutgoingAudioSource::AddSink):
(WebCore::RealtimeOutgoingAudioSource::RemoveSink):

  • platform/mediastream/RealtimeOutgoingAudioSource.h:

(WebCore::RealtimeOutgoingAudioSource::start):

  • platform/mediastream/RealtimeOutgoingVideoSource.cpp:

(WebCore::RealtimeOutgoingVideoSource::~RealtimeOutgoingVideoSource):
(WebCore::RealtimeOutgoingVideoSource::observeSource):
(WebCore::RealtimeOutgoingVideoSource::setSource):
(WebCore::RealtimeOutgoingVideoSource::AddOrUpdateSink):
(WebCore::RealtimeOutgoingVideoSource::RemoveSink):

  • platform/mediastream/RealtimeOutgoingVideoSource.h:

(WebCore::RealtimeOutgoingVideoSource::start):

  • platform/mediastream/mac/RealtimeOutgoingAudioSourceCocoa.cpp:

(WebCore::RealtimeOutgoingAudioSourceCocoa::~RealtimeOutgoingAudioSourceCocoa): Deleted.

Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r252986 r252987  
     12019-12-02  youenn fablet  <youenn@apple.com>
     2
     3        Simplify RealtimeOutgoingAudioSource and RealtimeOutgoingVideooSource observeSource/unobserveSource pattern
     4        https://bugs.webkit.org/show_bug.cgi?id=204570
     5
     6        Reviewed by Eric Carlson.
     7
     8        Previously, the outgoing source was observing/unobserving its track if sinks were added/removed.
     9        We made LibWebRTCRTCRtpSenderBackend stopping the observing in its destructor.
     10        This makes things complex and we are not always unobserving fast enough for all outgoing sources.
     11
     12        To make it simpler, the outgoing source is no longer handling the observing/unobserving of its track.
     13        Instead, its LibWebRTCRtpSenderBackend is handling it directly.
     14        Observing starts when LibWebRTCRtpSenderBackend gets the track and unobserving happens either on LibWebRTCRtpSenderBackend destruction
     15        or when the LibWebRTCRtpSenderBackend is no longer interested in the track.
     16        This is slight less optimal as we might observe the track even if the source has no sink but this should not happen often in practice.
     17
     18        Removed some unneeded methods.
     19
     20        Covered by existing tests no longer failing a debug ASSERTION.
     21
     22        * Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp:
     23        (WebCore::LibWebRTCRtpSenderBackend::LibWebRTCRtpSenderBackend):
     24        (WebCore::LibWebRTCRtpSenderBackend::~LibWebRTCRtpSenderBackend):
     25        (WebCore::LibWebRTCRtpSenderBackend::startSource):
     26        (WebCore::LibWebRTCRtpSenderBackend::stopSource):
     27        (WebCore::LibWebRTCRtpSenderBackend::replaceTrack):
     28        (WebCore::LibWebRTCRtpSenderBackend::audioSource):
     29        (WebCore::LibWebRTCRtpSenderBackend::videoSource):
     30        (WebCore::LibWebRTCRtpSenderBackend::hasSource const):
     31        (WebCore::LibWebRTCRtpSenderBackend::setSource):
     32        (WebCore::LibWebRTCRtpSenderBackend::takeSource):
     33        * Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h:
     34        * platform/mediastream/RealtimeOutgoingAudioSource.cpp:
     35        (WebCore::RealtimeOutgoingAudioSource::~RealtimeOutgoingAudioSource):
     36        (WebCore::RealtimeOutgoingAudioSource::observeSource):
     37        (WebCore::RealtimeOutgoingAudioSource::setSource):
     38        (WebCore::RealtimeOutgoingAudioSource::AddSink):
     39        (WebCore::RealtimeOutgoingAudioSource::RemoveSink):
     40        * platform/mediastream/RealtimeOutgoingAudioSource.h:
     41        (WebCore::RealtimeOutgoingAudioSource::start):
     42        * platform/mediastream/RealtimeOutgoingVideoSource.cpp:
     43        (WebCore::RealtimeOutgoingVideoSource::~RealtimeOutgoingVideoSource):
     44        (WebCore::RealtimeOutgoingVideoSource::observeSource):
     45        (WebCore::RealtimeOutgoingVideoSource::setSource):
     46        (WebCore::RealtimeOutgoingVideoSource::AddOrUpdateSink):
     47        (WebCore::RealtimeOutgoingVideoSource::RemoveSink):
     48        * platform/mediastream/RealtimeOutgoingVideoSource.h:
     49        (WebCore::RealtimeOutgoingVideoSource::start):
     50        * platform/mediastream/mac/RealtimeOutgoingAudioSourceCocoa.cpp:
     51        (WebCore::RealtimeOutgoingAudioSourceCocoa::~RealtimeOutgoingAudioSourceCocoa): Deleted.
     52
    1532019-12-02  youenn fablet  <youenn@apple.com>
    254

  • trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp

    r251989 r252987  
    3838namespace WebCore {
    3939
     40LibWebRTCRtpSenderBackend::LibWebRTCRtpSenderBackend(LibWebRTCPeerConnectionBackend& backend, rtc::scoped_refptr<webrtc::RtpSenderInterface>&& rtcSender, Source&& source)
     41    : m_peerConnectionBackend(makeWeakPtr(&backend))
     42    , m_rtcSender(WTFMove(rtcSender))
     43    , m_source(WTFMove(source))
     44{
     45    startSource();
     46}
     47
    4048LibWebRTCRtpSenderBackend::~LibWebRTCRtpSenderBackend()
    4149{
    42     WTF::switchOn(m_source, [] (Ref<RealtimeOutgoingAudioSource>& source) {
    43         source->stop();
    44     }, [] (Ref<RealtimeOutgoingVideoSource>& source) {
    45         source->stop();
    46     }, [] (std::nullptr_t&) {
     50    stopSource();
     51}
     52
     53void LibWebRTCRtpSenderBackend::startSource()
     54{
     55    switchOn(m_source, [](Ref<RealtimeOutgoingAudioSource>& source) {
     56        source->start();
     57    }, [](Ref<RealtimeOutgoingVideoSource>& source) {
     58        source->start();
     59    }, [](std::nullptr_t&) {
    4760    });
    4861}
    4962
    50 template<typename Source>
    51 static inline bool updateTrackSource(Source& source, MediaStreamTrack* track)
     63void LibWebRTCRtpSenderBackend::stopSource()
    5264{
    53     if (!track) {
    54         source.stop();
    55         return true;
    56     }
    57     return source.setSource(track->privateTrack());
     65    switchOn(m_source, [](Ref<RealtimeOutgoingAudioSource>& source) {
     66        source->stop();
     67    }, [](Ref<RealtimeOutgoingVideoSource>& source) {
     68        source->stop();
     69    }, [](std::nullptr_t&) {
     70    });
     71    m_source = nullptr;
    5872}
    5973
    … …  
    6579    }
    6680
    67     auto* currentTrack = sender.track();
    68 
    69     ASSERT(!track || !currentTrack || currentTrack->source().type() == track->source().type());
    70     if (currentTrack) {
    71     switch (currentTrack->source().type()) {
    72     case RealtimeMediaSource::Type::None:
    73         ASSERT_NOT_REACHED();
    74         promise.reject(InvalidModificationError);
    75         break;
    76     case RealtimeMediaSource::Type::Audio:
    77         if (!updateTrackSource(*audioSource(), track.get())) {
    78             promise.reject(InvalidModificationError);
    79             return;
    80         }
    81         break;
    82     case RealtimeMediaSource::Type::Video:
    83         if (!updateTrackSource(*videoSource(), track.get())) {
    84             promise.reject(InvalidModificationError);
    85             return;
    86         }
    87         break;
    88     }
     81    if (!track)
     82        stopSource();
     83    else if (auto* currentTrack = sender.track()) {
     84        switchOn(m_source, [&](Ref<RealtimeOutgoingAudioSource>& source) {
     85            ASSERT(track->source().type() == RealtimeMediaSource::Type::Audio);
     86            source->stop();
     87            source->setSource(track->privateTrack());
     88            source->start();
     89        }, [&](Ref<RealtimeOutgoingVideoSource>& source) {
     90            ASSERT(track->source().type() == RealtimeMediaSource::Type::Video);
     91            source->stop();
     92            source->setSource(track->privateTrack());
     93            source->start();
     94        }, [](std::nullptr_t&) {
     95            ASSERT_NOT_REACHED();
     96        });
    8997    }
    9098
    … …  
    108116        }
    109117
    110         m_source = nullptr;
    111118        m_peerConnectionBackend->setSenderSourceFromTrack(*this, *protectedSender->track());
    112119        promise.resolve();
    … …  
    152159}
    153160
     161RealtimeOutgoingVideoSource* LibWebRTCRtpSenderBackend::videoSource()
     162{
     163    return switchOn(m_source,
     164        [](Ref<RealtimeOutgoingVideoSource>& source) { return source.ptr(); },
     165        [](const auto&) -> RealtimeOutgoingVideoSource* { return nullptr; }
     166    );
     167}
     168
     169bool LibWebRTCRtpSenderBackend::hasSource() const
     170{
     171    return switchOn(m_source,
     172        [](const std::nullptr_t&) { return false; },
     173        [](const auto&) { return true; }
     174    );
     175}
     176
     177void LibWebRTCRtpSenderBackend::setSource(Source&& source)
     178{
     179    stopSource();
     180    m_source = WTFMove(source);
     181    startSource();
     182}
     183
     184void LibWebRTCRtpSenderBackend::takeSource(LibWebRTCRtpSenderBackend& backend)
     185{
     186    ASSERT(backend.hasSource());
     187    stopSource();
     188    m_source = WTFMove(backend.m_source);
     189    backend.m_source = nullptr;
     190}
     191
    154192} // namespace WebCore
    155193

  • trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h

    r252472 r252987  
    5555
    5656    using Source = Variant<std::nullptr_t, Ref<RealtimeOutgoingAudioSource>, Ref<RealtimeOutgoingVideoSource>>;
    57     LibWebRTCRtpSenderBackend(LibWebRTCPeerConnectionBackend& backend, rtc::scoped_refptr<webrtc::RtpSenderInterface>&& rtcSender, Source&& source)
    58         : m_peerConnectionBackend(makeWeakPtr(&backend))
    59         , m_rtcSender(WTFMove(rtcSender))
    60         , m_source(WTFMove(source))
    61     {
    62     }
     57    LibWebRTCRtpSenderBackend(LibWebRTCPeerConnectionBackend&, rtc::scoped_refptr<webrtc::RtpSenderInterface>&&, Source&&);
    6358    ~LibWebRTCRtpSenderBackend();
    6459
    … …  
    6661    webrtc::RtpSenderInterface* rtcSender() { return m_rtcSender.get(); }
    6762
    68     RealtimeOutgoingAudioSource* audioSource()
    69     {
    70         return WTF::switchOn(m_source,
    71             [] (Ref<RealtimeOutgoingAudioSource>& source) { return source.ptr(); },
    72             [] (const auto&) -> RealtimeOutgoingAudioSource* { return nullptr; }
    73         );
    74     }
    75 
    76     RealtimeOutgoingVideoSource* videoSource()
    77     {
    78         return WTF::switchOn(m_source,
    79             [] (Ref<RealtimeOutgoingVideoSource>& source) { return source.ptr(); },
    80             [] (const auto&) -> RealtimeOutgoingVideoSource* { return nullptr; }
    81         );
    82     }
    83 
    84     bool hasSource() const
    85     {
    86         return WTF::switchOn(m_source,
    87             [] (const std::nullptr_t&) { return false; },
    88             [] (const auto&) { return true; }
    89         );
    90     }
    91 
    92     void clearSource()
    93     {
    94         ASSERT(hasSource());
    95         m_source = nullptr;
    96     }
    97 
    98     void setSource(Source&& source)
    99     {
    100         ASSERT(!hasSource());
    101         m_source = WTFMove(source);
    102         ASSERT(hasSource());
    103     }
    104 
    105     void takeSource(LibWebRTCRtpSenderBackend& backend)
    106     {
    107         ASSERT(backend.hasSource());
    108         setSource(WTFMove(backend.m_source));
    109         backend.m_source = nullptr;
    110     }
     63    RealtimeOutgoingVideoSource* videoSource();
     64    void clearSource() { setSource(nullptr); }
     65    void setSource(Source&&);
     66    void takeSource(LibWebRTCRtpSenderBackend&);
    11167
    11268private:
    … …  
    11571    void setParameters(const RTCRtpSendParameters&, DOMPromiseDeferred<void>&&) final;
    11672    std::unique_ptr<RTCDTMFSenderBackend> createDTMFBackend() final;
     73
     74    void startSource();
     75    void stopSource();
     76    bool hasSource() const;
    11777
    11878    WeakPtr<LibWebRTCPeerConnectionBackend> m_peerConnectionBackend;

  • trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.cpp

    r251989 r252987  
    4646RealtimeOutgoingAudioSource::~RealtimeOutgoingAudioSource()
    4747{
    48     ASSERT(!m_audioSource->hasObserver(*this));
     48ASSERT(!m_audioSource->hasObserver(*this));
     49#if !ASSERT_DISABLED
     50    auto locker = holdLock(m_sinksLock);
     51#endif
    4952    ASSERT(m_sinks.isEmpty());
     53
    5054    stop();
    5155}
    … …  
    5357void RealtimeOutgoingAudioSource::observeSource()
    5458{
     59    ASSERT(!m_audioSource->hasObserver(*this));
    5560    m_audioSource->addObserver(*this);
    5661    initializeConverter();
    … …  
    6267}
    6368
    64 bool RealtimeOutgoingAudioSource::setSource(Ref<MediaStreamTrackPrivate>&& newSource)
     69void RealtimeOutgoingAudioSource::setSource(Ref<MediaStreamTrackPrivate>&& newSource)
    6570{
    6671    ALWAYS_LOG("Changing source to ", newSource->logIdentifier());
    67     auto locker = holdLock(m_sinksLock);
    68     bool hasSinks = !m_sinks.isEmpty();
    6972
    70     if (hasSinks)
    71         unobserveSource();
     73    ASSERT(!m_audioSource->hasObserver(*this));
    7274    m_audioSource = WTFMove(newSource);
    73     if (hasSinks)
    74         observeSource();
    75 
    7675    sourceUpdated();
    77 
    78     return true;
    7976}
    8077
    … …  
    9794void RealtimeOutgoingAudioSource::AddSink(webrtc::AudioTrackSinkInterface* sink)
    9895{
    99     {
    10096    auto locker = holdLock(m_sinksLock);
    101     if (!m_sinks.add(sink) || m_sinks.size() != 1)
    102         return;
    103     }
    104 
    105     callOnMainThread([protectedThis = makeRef(*this)]() {
    106         protectedThis->observeSource();
    107     });
     97    m_sinks.add(sink);
    10898}
    10999
    110100void RealtimeOutgoingAudioSource::RemoveSink(webrtc::AudioTrackSinkInterface* sink)
    111101{
    112     {
    113102    auto locker = holdLock(m_sinksLock);
    114     if (!m_sinks.remove(sink) || !m_sinks.isEmpty())
    115         return;
    116     }
    117 
    118     unobserveSource();
     103    m_sinks.remove(sink);
    119104}
    120105

  • trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h

    r252472 r252987  
    6565    ~RealtimeOutgoingAudioSource();
    6666
     67    void start() { observeSource(); }
    6768    void stop() { unobserveSource(); }
    6869
    69     bool setSource(Ref<MediaStreamTrackPrivate>&&);
     70    void setSource(Ref<MediaStreamTrackPrivate>&&);
    7071    MediaStreamTrackPrivate& source() const { return m_audioSource.get(); }
    7172
    7273protected:
    7374    explicit RealtimeOutgoingAudioSource(Ref<MediaStreamTrackPrivate>&&);
    74 
    75     void unobserveSource();
    7675
    7776    bool isSilenced() const { return m_muted || !m_enabled; }
    … …  
    106105
    107106    void observeSource();
     107    void unobserveSource();
    108108
    109109    void sourceMutedChanged();

  • trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp

    r251989 r252987  
    5757RealtimeOutgoingVideoSource::~RealtimeOutgoingVideoSource()
    5858{
    59     ASSERT(!m_videoSource->hasObserver(*this));
     59ASSERT(!m_videoSource->hasObserver(*this));
     60#if !ASSERT_DISABLED
     61    auto locker = holdLock(m_sinksLock);
     62#endif
    6063    ASSERT(m_sinks.isEmpty());
    6164    stop();
    … …  
    6467void RealtimeOutgoingVideoSource::observeSource()
    6568{
     69    ASSERT(!m_videoSource->hasObserver(*this));
    6670    m_videoSource->addObserver(*this);
    6771    initializeFromSource();
    … …  
    7377}
    7478
    75 bool RealtimeOutgoingVideoSource::setSource(Ref<MediaStreamTrackPrivate>&& newSource)
    76 {
    77     if (!m_initialSettings)
    78         m_initialSettings = m_videoSource->source().settings();
    79 
    80     auto locker = holdLock(m_sinksLock);
    81     bool hasSinks = !m_sinks.isEmpty();
    82 
    83     if (hasSinks)
    84         unobserveSource();
     79void RealtimeOutgoingVideoSource::setSource(Ref<MediaStreamTrackPrivate>&& newSource)
     80{
     81    ASSERT(!m_videoSource->hasObserver(*this));
    8582    m_videoSource = WTFMove(newSource);
    86     if (hasSinks)
    87         observeSource();
    88 
    89     return true;
    9083}
    9184
    … …  
    145138        m_shouldApplyRotation = true;
    146139
    147     {
    148     auto locker = holdLock(m_sinksLock);
    149     if (!m_sinks.add(sink) || m_sinks.size() != 1)
    150         return;
    151     }
    152 
    153     callOnMainThread([protectedThis = makeRef(*this)]() {
    154         protectedThis->observeSource();
    155     });
     140    auto locker = holdLock(m_sinksLock);
     141    m_sinks.add(sink);
    156142}
    157143
    158144void RealtimeOutgoingVideoSource::RemoveSink(rtc::VideoSinkInterface<webrtc::VideoFrame>* sink)
    159145{
    160     {
    161     auto locker = holdLock(m_sinksLock);
    162 
    163     if (!m_sinks.remove(sink) || m_sinks.size())
    164         return;
    165     }
    166 
    167     unobserveSource();
    168 
    169     callOnMainThread([protectedThis = makeRef(*this)]() {
    170         if (protectedThis->m_blackFrameTimer.isActive())
    171             protectedThis->m_blackFrameTimer.stop();
    172     });
     146    auto locker = holdLock(m_sinksLock);
     147    m_sinks.remove(sink);
    173148}
    174149

  • trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h

    r252472 r252987  
    6060    ~RealtimeOutgoingVideoSource();
    6161
     62    void start() { observeSource(); }
    6263    void stop();
    63     bool setSource(Ref<MediaStreamTrackPrivate>&&);
     64    void setSource(Ref<MediaStreamTrackPrivate>&&);
    6465    MediaStreamTrackPrivate& source() const { return m_videoSource.get(); }
    6566
    … …  
    129130
    130131    Ref<MediaStreamTrackPrivate> m_videoSource;
    131     Optional<RealtimeMediaSourceSettings> m_initialSettings;
    132132    Timer m_blackFrameTimer;
    133133    rtc::scoped_refptr<webrtc::VideoFrameBuffer> m_blackFrame;

  • trunk/Source/WebCore/platform/mediastream/gstreamer/RealtimeOutgoingAudioSourceLibWebRTC.cpp

    r239921 r252987  
    3939RealtimeOutgoingAudioSourceLibWebRTC::~RealtimeOutgoingAudioSourceLibWebRTC()
    4040{
    41     unobserveSource();
    4241    m_sampleConverter = nullptr;
    4342}

  • trunk/Source/WebCore/platform/mediastream/mac/RealtimeOutgoingAudioSourceCocoa.cpp

    r252960 r252987  
    5151}
    5252
    53 RealtimeOutgoingAudioSourceCocoa::~RealtimeOutgoingAudioSourceCocoa()
    54 {
    55     unobserveSource();
    56 }
     53RealtimeOutgoingAudioSourceCocoa::~RealtimeOutgoingAudioSourceCocoa() = default;
    5754
    5855Ref<RealtimeOutgoingAudioSource> RealtimeOutgoingAudioSource::create(Ref<MediaStreamTrackPrivate>&& audioSource)
Note: See TracChangeset for help on using the changeset viewer.