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

Changeset 264175 in webkit


Ignore:
Timestamp:
Jul 9, 2020, 9:22:59 AM (6 years ago)
Author:
aboya@igalia.com
Message:

[MSE][GStreamer] Remove m_appendPipelinesMap
​https://bugs.webkit.org/show_bug.cgi?id=214132

Reviewed by Xabier Rodriguez-Calvar.

m_appendPipelinesMap was owned by MediaPlayerPrivateGStreamerMSE but
was only used by MediaPlayerPrivateGStreamerMSE to clear it during
destruction, while the other uses were in
MediaSourceClientGStreamerMSE.

After analysis, it was found keeping a HashMap of AppendPipelines is not
necessary. An AppendPipeline only needs to be used by the SourceBuffer
receiving the muxed data: making AppendPipeline a member of
SourceBufferPrivateGStreamer reflects this dependency in a much
clearer way. No need for a HashMap of AppendPipeline's.

Moreso, there are no other users of AppendPipeline, which means
AppendPipeline doesn't need to be ref counted. This patch removes that
feature, using std::unique_ptr<AppendPipeline> for ownership instead.

This patch is a refactor: it doesn't introduce behavior changes and
it's covered by existing tests.

  • platform/graphics/gstreamer/mse/AppendPipeline.cpp:

(WebCore::AppendPipeline::appsinkCapsChanged):
(WebCore::AppendPipeline::connectDemuxerSrcPadToAppsink):

  • platform/graphics/gstreamer/mse/AppendPipeline.h:
  • platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:

(WebCore::MediaPlayerPrivateGStreamerMSE::~MediaPlayerPrivateGStreamerMSE):
(WebCore::MediaPlayerPrivateGStreamerMSE::trackDetected):

  • platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h:
  • platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp:

(WebCore::MediaSourceClientGStreamerMSE::addSourceBuffer):
(WebCore::MediaSourceClientGStreamerMSE::abort):
(WebCore::MediaSourceClientGStreamerMSE::resetParserState):
(WebCore::MediaSourceClientGStreamerMSE::append):
(WebCore::MediaSourceClientGStreamerMSE::removedFromMediaSource):

  • platform/graphics/gstreamer/mse/MediaSourcePrivateGStreamer.cpp:

(WebCore::MediaSourcePrivateGStreamer::addSourceBuffer):

  • platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:

(WebCore::SourceBufferPrivateGStreamer::create):
(WebCore::SourceBufferPrivateGStreamer::SourceBufferPrivateGStreamer):
(WebCore::SourceBufferPrivateGStreamer::finishCreation):

  • platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h:
Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r264169 r264175  
     12020-07-09  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE][GStreamer] Remove m_appendPipelinesMap
     4        https://bugs.webkit.org/show_bug.cgi?id=214132
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        m_appendPipelinesMap was owned by MediaPlayerPrivateGStreamerMSE but
     9        was only used by MediaPlayerPrivateGStreamerMSE to clear it during
     10        destruction, while the other uses were in
     11        MediaSourceClientGStreamerMSE.
     12
     13        After analysis, it was found keeping a HashMap of AppendPipelines is not
     14        necessary. An AppendPipeline only needs to be used by the SourceBuffer
     15        receiving the muxed data: making AppendPipeline a member of
     16        SourceBufferPrivateGStreamer reflects this dependency in a much
     17        clearer way. No need for a HashMap of AppendPipeline's.
     18
     19        Moreso, there are no other users of AppendPipeline, which means
     20        AppendPipeline doesn't need to be ref counted. This patch removes that
     21        feature, using std::unique_ptr<AppendPipeline> for ownership instead.
     22
     23        This patch is a refactor: it doesn't introduce behavior changes and
     24        it's covered by existing tests.
     25
     26        * platform/graphics/gstreamer/mse/AppendPipeline.cpp:
     27        (WebCore::AppendPipeline::appsinkCapsChanged):
     28        (WebCore::AppendPipeline::connectDemuxerSrcPadToAppsink):
     29        * platform/graphics/gstreamer/mse/AppendPipeline.h:
     30        * platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:
     31        (WebCore::MediaPlayerPrivateGStreamerMSE::~MediaPlayerPrivateGStreamerMSE):
     32        (WebCore::MediaPlayerPrivateGStreamerMSE::trackDetected):
     33        * platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h:
     34        * platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp:
     35        (WebCore::MediaSourceClientGStreamerMSE::addSourceBuffer):
     36        (WebCore::MediaSourceClientGStreamerMSE::abort):
     37        (WebCore::MediaSourceClientGStreamerMSE::resetParserState):
     38        (WebCore::MediaSourceClientGStreamerMSE::append):
     39        (WebCore::MediaSourceClientGStreamerMSE::removedFromMediaSource):
     40        * platform/graphics/gstreamer/mse/MediaSourcePrivateGStreamer.cpp:
     41        (WebCore::MediaSourcePrivateGStreamer::addSourceBuffer):
     42        * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:
     43        (WebCore::SourceBufferPrivateGStreamer::create):
     44        (WebCore::SourceBufferPrivateGStreamer::SourceBufferPrivateGStreamer):
     45        (WebCore::SourceBufferPrivateGStreamer::finishCreation):
     46        * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h:
     47
    1482020-07-09  Zalan Bujtas  <zalan@apple.com>
    249

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp

    r264164 r264175  
    441441    if (m_appsinkCaps != caps) {
    442442        m_appsinkCaps = WTFMove(caps);
    443         m_playerPrivate->trackDetected(this, m_track, previousCapsWereNull);
     443        m_playerPrivate->trackDetected(*this, m_track, previousCapsWereNull);
    444444    }
    445445}
    … …  
    793793
    794794    m_appsinkCaps = WTFMove(caps);
    795     m_playerPrivate->trackDetected(this, m_track, true);
     795    m_playerPrivate->trackDetected(*this, m_track, true);
    796796}
    797797

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h

    r251365 r264175  
    4545#endif
    4646
    47 class AppendPipeline : public ThreadSafeRefCounted<AppendPipeline> {
     47class AppendPipeline {
     48    WTF_MAKE_FAST_ALLOCATED;
    4849public:
    4950    AppendPipeline(Ref<MediaSourceClientGStreamerMSE>, Ref<SourceBufferPrivateGStreamer>, MediaPlayerPrivateGStreamerMSE&);

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp

    r264164 r264175  
    115115    GST_TRACE("destroying the player (%p)", this);
    116116
    117     // Clear the AppendPipeline map. This should cause the destruction of all the AppendPipeline's since there should
    118     // be no alive references at this point.
    119 #ifndef NDEBUG
    120     for (auto iterator : m_appendPipelinesMap)
    121         ASSERT(iterator.value->hasOneRef());
    122 #endif
    123     m_appendPipelinesMap.clear();
    124 
    125117    if (m_source) {
    126118        webKitMediaSrcSetMediaPlayerPrivate(WEBKIT_MEDIA_SRC(m_source.get()), nullptr);
    … …  
    711703}
    712704
    713 void MediaPlayerPrivateGStreamerMSE::trackDetected(RefPtr<AppendPipeline> appendPipeline, RefPtr<WebCore::TrackPrivateBase> newTrack, bool firstTrackDetected)
    714 {
    715     ASSERT(appendPipeline->track() == newTrack);
    716 
    717     GstCaps* caps = appendPipeline->appsinkCaps();
     705void MediaPlayerPrivateGStreamerMSE::trackDetected(AppendPipeline& appendPipeline, RefPtr<WebCore::TrackPrivateBase> newTrack, bool firstTrackDetected)
     706{
     707    ASSERT(appendPipeline.track() == newTrack);
     708
     709    GstCaps* caps = appendPipeline.appsinkCaps();
    718710    ASSERT(caps);
    719711    GST_DEBUG("track ID: %s, caps: %" GST_PTR_FORMAT, newTrack->id().string().latin1().data(), caps);
    … …  
    726718
    727719    if (firstTrackDetected)
    728         m_playbackPipeline->attachTrack(appendPipeline->sourceBufferPrivate(), newTrack, caps);
     720        m_playbackPipeline->attachTrack(appendPipeline.sourceBufferPrivate(), newTrack, caps);
    729721    else
    730         m_playbackPipeline->reattachTrack(appendPipeline->sourceBufferPrivate(), newTrack, caps);
     722        m_playbackPipeline->reattachTrack(appendPipeline.sourceBufferPrivate(), newTrack, caps);
    731723}
    732724

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h

    r263656 r264175  
    7878    void markEndOfStream(MediaSourcePrivate::EndOfStreamStatus);
    7979
    80     void trackDetected(RefPtr<AppendPipeline>, RefPtr<WebCore::TrackPrivateBase>, bool firstTrackDetected);
     80    void trackDetected(AppendPipeline&, RefPtr<WebCore::TrackPrivateBase>, bool firstTrackDetected);
    8181    void notifySeekNeedsDataForTime(const MediaTime&);
    8282
    … …  
    109109    RefPtr<MediaSourceClientGStreamerMSE> mediaSourceClient();
    110110
    111     HashMap<RefPtr<SourceBufferPrivateGStreamer>, RefPtr<AppendPipeline>> m_appendPipelinesMap;
    112111    bool m_eosMarked = false;
    113112    mutable bool m_eosPending = false;

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp

    r264166 r264175  
    6363    ASSERT(sourceBufferPrivate);
    6464
    65     RefPtr<AppendPipeline> appendPipeline = adoptRef(new AppendPipeline(*this, *sourceBufferPrivate, m_playerPrivate));
    66     GST_TRACE("Adding SourceBuffer to AppendPipeline: this=%p sourceBuffer=%p appendPipeline=%p", this, sourceBufferPrivate.get(), appendPipeline.get());
    67     m_playerPrivate.m_appendPipelinesMap.add(sourceBufferPrivate, appendPipeline);
    68 
    6965    return m_playerPrivate.m_playbackPipeline->addSourceBuffer(sourceBufferPrivate);
    7066}
    … …  
    9490    GST_DEBUG("aborting");
    9591
    96     RefPtr<AppendPipeline> appendPipeline = m_playerPrivate.m_appendPipelinesMap.get(sourceBufferPrivate);
    97 
    98     ASSERT(appendPipeline);
    99 
    100     appendPipeline->resetParserState();
     92    sourceBufferPrivate->appendPipeline().resetParserState();
    10193}
    10294
    … …  
    10799    GST_DEBUG("resetting parser state");
    108100
    109     RefPtr<AppendPipeline> appendPipeline = m_playerPrivate.m_appendPipelinesMap.get(sourceBufferPrivate);
    110 
    111     ASSERT(appendPipeline);
    112 
    113     appendPipeline->resetParserState();
     101    sourceBufferPrivate->appendPipeline().resetParserState();
    114102}
    115103
    … …  
    119107
    120108    GST_DEBUG("Appending %zu bytes", data.size());
    121 
    122     RefPtr<AppendPipeline> appendPipeline = m_playerPrivate.m_appendPipelinesMap.get(sourceBufferPrivate);
    123 
    124     ASSERT(appendPipeline);
    125109
    126110    // Wrap the whole Vector object in case the data is stored in the inlined buffer.
    … …  
    133117        }));
    134118
    135     appendPipeline->pushNewBuffer(WTFMove(buffer));
     119    sourceBufferPrivate->appendPipeline().pushNewBuffer(WTFMove(buffer));
    136120}
    137121
    … …  
    148132
    149133    ASSERT(m_playerPrivate.m_playbackPipeline);
    150 
    151     // Remove the AppendPipeline from the map. This should cause its destruction since there should be no alive
    152     // references at this point.
    153     ASSERT(m_playerPrivate.m_appendPipelinesMap.get(sourceBufferPrivate)->hasOneRef());
    154     m_playerPrivate.m_appendPipelinesMap.remove(sourceBufferPrivate);
    155134
    156135    m_playerPrivate.m_playbackPipeline->removeSourceBuffer(sourceBufferPrivate);

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourcePrivateGStreamer.cpp

    r263656 r264175  
    7979{
    8080    DEBUG_LOG(LOGIDENTIFIER, contentType);
    81     sourceBufferPrivate = SourceBufferPrivateGStreamer::create(this, m_client.get(), contentType);
     81    sourceBufferPrivate = SourceBufferPrivateGStreamer::create(this, m_client.get(), contentType, m_playerPrivate);
    8282    RefPtr<SourceBufferPrivateGStreamer> sourceBufferPrivateGStreamer = static_cast<SourceBufferPrivateGStreamer*>(sourceBufferPrivate.get());
    8383    m_sourceBuffers.add(sourceBufferPrivateGStreamer);

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp

    r264161 r264175  
    3838#if ENABLE(MEDIA_SOURCE) && USE(GSTREAMER)
    3939
     40#include "AppendPipeline.h"
    4041#include "ContentType.h"
    4142#include "GStreamerCommon.h"
    … …  
    4950namespace WebCore {
    5051
    51 Ref<SourceBufferPrivateGStreamer> SourceBufferPrivateGStreamer::create(MediaSourcePrivateGStreamer* mediaSource, Ref<MediaSourceClientGStreamerMSE> client, const ContentType& contentType)
     52Ref<SourceBufferPrivateGStreamer> SourceBufferPrivateGStreamer::create(MediaSourcePrivateGStreamer* mediaSource, Ref<MediaSourceClientGStreamerMSE> client, const ContentType& contentType, MediaPlayerPrivateGStreamerMSE& playerPrivate)
    5253{
    53     return adoptRef(*new SourceBufferPrivateGStreamer(mediaSource, client.get(), contentType));
     54    return adoptRef(*new SourceBufferPrivateGStreamer(mediaSource, client.get(), contentType, playerPrivate));
    5455}
    5556
    56 SourceBufferPrivateGStreamer::SourceBufferPrivateGStreamer(MediaSourcePrivateGStreamer* mediaSource, Ref<MediaSourceClientGStreamerMSE> client, const ContentType& contentType)
     57SourceBufferPrivateGStreamer::SourceBufferPrivateGStreamer(MediaSourcePrivateGStreamer* mediaSource, Ref<MediaSourceClientGStreamerMSE> client, const ContentType& contentType, MediaPlayerPrivateGStreamerMSE& playerPrivate)
    5758    : SourceBufferPrivate()
    5859    , m_mediaSource(mediaSource)
    5960    , m_type(contentType)
    6061    , m_client(client.get())
     62    , m_playerPrivate(playerPrivate)
    6163#if !RELEASE_LOG_DISABLED
    6264    , m_logger(mediaSource->logger())
    … …  
    6466#endif
    6567{
     68    relaxAdoptionRequirement();
     69    m_appendPipeline = WTF::makeUnique<AppendPipeline>(m_client, makeRef(*this), m_playerPrivate);
    6670}
    6771

  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h

    r263656 r264175  
    5555
    5656public:
    57     static Ref<SourceBufferPrivateGStreamer> create(MediaSourcePrivateGStreamer*, Ref<MediaSourceClientGStreamerMSE>, const ContentType&);
     57    static Ref<SourceBufferPrivateGStreamer> create(MediaSourcePrivateGStreamer*, Ref<MediaSourceClientGStreamerMSE>, const ContentType&, MediaPlayerPrivateGStreamerMSE&);
    5858    virtual ~SourceBufferPrivateGStreamer() = default;
    5959
    … …  
    8585    ContentType type() const { return m_type; }
    8686
     87    AppendPipeline& appendPipeline() { return *m_appendPipeline; }
     88
    8789#if !RELEASE_LOG_DISABLED
    8890    const Logger& logger() const final { return m_logger.get(); }
    … …  
    9597
    9698private:
    97     SourceBufferPrivateGStreamer(MediaSourcePrivateGStreamer*, Ref<MediaSourceClientGStreamerMSE>, const ContentType&);
     99    SourceBufferPrivateGStreamer(MediaSourcePrivateGStreamer*, Ref<MediaSourceClientGStreamerMSE>, const ContentType&, MediaPlayerPrivateGStreamerMSE&);
    98100    friend class MediaSourceClientGStreamerMSE;
    99101
    … …  
    101103    ContentType m_type;
    102104    Ref<MediaSourceClientGStreamerMSE> m_client;
     105    MediaPlayerPrivateGStreamerMSE& m_playerPrivate;
     106    std::unique_ptr<AppendPipeline> m_appendPipeline;
    103107    SourceBufferPrivateClient* m_sourceBufferPrivateClient { nullptr };
    104108    bool m_isReadyForMoreSamples = true;
Note: See TracChangeset for help on using the changeset viewer.