Changeset 238412 in webkit
- Timestamp:
- Nov 21, 2018 12:49:39 AM (5 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r238410 r238412 1 2018-11-21 Alicia Boya García <aboya@igalia.com> 2 3 [MSE][GStreamer] Refactor AppendPipeline deinitialization 4 https://bugs.webkit.org/show_bug.cgi?id=191759 5 6 Reviewed by Xabier Rodriguez-Calvar. 7 8 AppendPipeline currently has a method, clearPlayerPrivate(), that the 9 client code uses to deinitialize most of the AppendPipeline... just 10 before actually destructing it in the next line of code. 11 12 Since at that point there should not be alive RefPtr's pointing to the 13 AppendPipeline there is no need for this kind of pre-deinitialization 14 in this usage pattern. Instead, we can just rely on C++ destructors, 15 cleaning the code a bit and removing the potential for the question 16 "what if `clearPlayerPrivate() has been called before?`": it has not. 17 18 Assertions have been added to ensure that there is only one alive 19 RefPtr pointing to AppendPipeline, therefore guaranteeing its immediate 20 destruction. 21 22 * platform/graphics/gstreamer/mse/AppendPipeline.cpp: 23 (WebCore::AppendPipeline::~AppendPipeline): 24 (WebCore::AppendPipeline::deinitialize): 25 (WebCore::AppendPipeline::clearPlayerPrivate): Deleted. 26 * platform/graphics/gstreamer/mse/AppendPipeline.h: 27 * platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: 28 (WebCore::MediaPlayerPrivateGStreamerMSE::~MediaPlayerPrivateGStreamerMSE): 29 * platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp: 30 (WebCore::MediaSourceClientGStreamerMSE::removedFromMediaSource): 31 1 32 2018-11-20 Dean Jackson <dino@apple.com> 2 33 -
trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
r238287 r238412 226 226 AppendPipeline::~AppendPipeline() 227 227 { 228 GST_TRACE("Destructing AppendPipeline (%p)", this); 228 229 ASSERT(isMainThread()); 229 230 … … 232 233 m_taskQueue.startAborting(); 233 234 234 GST_TRACE("Destroying AppendPipeline (%p)", this); 235 236 // FIXME: Maybe notify appendComplete here? 235 // Disconnect all synchronous event handlers and probes susceptible of firing from the main thread 236 // when changing the pipeline state. 237 237 238 238 if (m_pipeline) { … … 241 241 gst_bus_disable_sync_message_emission(m_bus.get()); 242 242 gst_bus_remove_signal_watch(m_bus.get()); 243 gst_element_set_state(m_pipeline.get(), GST_STATE_NULL); 244 m_pipeline = nullptr; 245 } 246 247 if (m_appsrc) { 243 } 244 245 if (m_appsrc) 248 246 g_signal_handlers_disconnect_by_data(m_appsrc.get(), this); 249 m_appsrc = nullptr;250 }251 247 252 248 if (m_demux) { … … 257 253 258 254 g_signal_handlers_disconnect_by_data(m_demux.get(), this); 259 m_demux = nullptr;260 255 } 261 256 … … 272 267 gst_pad_remove_probe(appsinkPad.get(), m_appsinkPadEventProbeInformation.probeId); 273 268 #endif 274 m_appsink = nullptr;275 } 276 277 m_appsinkCaps = nullptr;278 m_demuxerSrcPadCaps = nullptr;279 } ;269 } 270 271 // We can tear down the pipeline safely now. 272 if (m_pipeline) 273 gst_element_set_state(m_pipeline.get(), GST_STATE_NULL); 274 } 280 275 281 276 GstPadProbeReturn AppendPipeline::appsrcEndOfAppendCheckerProbe(GstPadProbeInfo* padProbeInfo) … … 298 293 }); 299 294 return GST_PAD_PROBE_DROP; 300 }301 302 void AppendPipeline::clearPlayerPrivate()303 {304 ASSERT(isMainThread());305 GST_DEBUG("cleaning private player");306 307 // Make sure that AppendPipeline won't process more data from now on and308 // instruct handleNewSample to abort itself from now on as well.309 setAppendState(AppendState::Invalid);310 m_taskQueue.startAborting();311 312 // And now that no handleNewSample operations will remain stalled waiting313 // for the main thread, stop the pipeline.314 if (m_pipeline)315 gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);316 295 } 317 296 -
trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h
r238084 r238412 51 51 52 52 GstFlowReturn pushNewBuffer(GstBuffer*); 53 void clearPlayerPrivate();54 53 void abort(); 55 54 Ref<SourceBufferPrivateGStreamer> sourceBufferPrivate() { return m_sourceBufferPrivate.get(); } -
trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp
r238131 r238412 98 98 GST_TRACE("destroying the player (%p)", this); 99 99 100 // Clear the AppendPipeline map. This should cause the destruction of all the AppendPipeline's since there should 101 // be no alive references at this point. 102 #ifndef NDEBUG 100 103 for (auto iterator : m_appendPipelinesMap) 101 iterator.value->clearPlayerPrivate(); 104 ASSERT(iterator.value->hasOneRef()); 105 #endif 106 m_appendPipelinesMap.clear(); 102 107 103 108 if (m_source) { -
trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp
r232505 r238412 170 170 ASSERT(m_playerPrivate->m_playbackPipeline); 171 171 172 RefPtr<AppendPipeline> appendPipeline = m_playerPrivate->m_appendPipelinesMap.get(sourceBufferPrivate); 173 174 ASSERT(appendPipeline); 175 176 appendPipeline->clearPlayerPrivate(); 172 // Remove the AppendPipeline from the map. This should cause its destruction since there should be no alive 173 // references at this point. 174 ASSERT(m_playerPrivate->m_appendPipelinesMap.get(sourceBufferPrivate)->hasOneRef()); 177 175 m_playerPrivate->m_appendPipelinesMap.remove(sourceBufferPrivate); 178 // AppendPipeline destructor will take care of cleaning up when appropriate.179 176 180 177 m_playerPrivate->m_playbackPipeline->removeSourceBuffer(sourceBufferPrivate);
Note: See TracChangeset
for help on using the changeset viewer.