RESOLVED FIXED214132
[MSE][GStreamer] Remove m_appendPipelinesMap
https://bugs.webkit.org/show_bug.cgi?id=214132
Summary [MSE][GStreamer] Remove m_appendPipelinesMap
Alicia Boya García
Reported 2020-07-09 05:13:32 PDT
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.
Attachments
Patch (17.68 KB, patch)
2020-07-09 05:15 PDT, Alicia Boya García
no flags
Patch for landing (17.17 KB, patch)
2020-07-09 08:32 PDT, Alicia Boya García
no flags
Alicia Boya García
Comment 1 2020-07-09 05:15:06 PDT
Xabier Rodríguez Calvar
Comment 2 2020-07-09 07:58:22 PDT
Comment on attachment 403857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403857&action=review > Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:78 > +void SourceBufferPrivateGStreamer::finishCreation() > +{ > + // Initializing the append pipeline must happen after Ref<SourceBufferPrivateGStreamer> has been constructed and > + // adopted, otherwise the makeRef() below would crash on ASSERT(!m_adoptionIsRequired). > + ASSERT(!m_appendPipeline); > + m_appendPipeline = WTF::makeUnique<AppendPipeline>(m_client, makeRef(*this), m_playerPrivate); > +} You don't need this, just call relaxAdoptionRequirement() in SourceBufferPrivateGStreamer::SourceBufferPrivateGStreamer() and you should be good.
Alicia Boya García
Comment 3 2020-07-09 08:32:56 PDT
Created attachment 403866 [details] Patch for landing
EWS
Comment 4 2020-07-09 09:23:04 PDT
Committed r264175: <https://trac.webkit.org/changeset/264175> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403866 [details].
Note You need to log in before you can comment on or make changes to this bug.