WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
214132
[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
Details
Formatted Diff
Diff
Patch for landing
(17.17 KB, patch)
2020-07-09 08:32 PDT
,
Alicia Boya García
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Alicia Boya García
Comment 1
2020-07-09 05:15:06 PDT
Created
attachment 403857
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug