WebKit Bugzilla
Attachment 356528 Details for
Bug 192204
: [MSE][GStreamer] Remove the AppendPipeline state machine
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for landing
bug-192204-20181204225650.patch (text/plain), 50.51 KB, created by
Alicia Boya García
on 2018-12-04 13:56:52 PST
(
hide
)
Description:
Patch for landing
Filename:
MIME Type:
Creator:
Alicia Boya García
Created:
2018-12-04 13:56:52 PST
Size:
50.51 KB
patch
obsolete
>Subversion Revision: 238414 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 230131df4508d5ea8ba0cb856259fad1701cdba7..f6d1a40ecb55c15434faccb034ea4f23f765de9b 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,77 @@ >+2018-11-29 Alicia Boya GarcÃa <aboya@igalia.com> >+ >+ [MSE][GStreamer] Remove the AppendPipeline state machine >+ https://bugs.webkit.org/show_bug.cgi?id=192204 >+ >+ Reviewed by Xabier Rodriguez-Calvar. >+ >+ This patch tries to reduce the complexity of the AppendPipeline by >+ removing the appendState state machine and cleaning all the >+ conditional code around it that is not necessary anymore. >+ >+ For the most part the behavior is the same, but some edge cases have >+ been improved in the process: >+ >+ Demuxing errors now result in the append being flagged as >+ ParsingFailed and the error being propagated to the application. This >+ fixes media/media-source/media-source-error-crash.html (or at least >+ gets it up to date with cross platform expectations). >+ >+ AbortableTaskQueue now allows the task handler to perform an abort >+ safely. This is used in the GstBus error message sync handler, since >+ it needs to ask the MainThread to raise a parse error, which will in >+ turn abort. An API test has been added for this new functionality. >+ Also, code has been added to the API tests to ensure the correct >+ destruction of the response object, especially in this case. >+ >+ The code handling invalid track codecs has been made clearer by also >+ explicitly raising a parse error, but it should not expose behavior >+ differences for the application. A test has been added for this >+ behavior: web-platform-tests/media-source/mediasource-invalid-codec.html >+ >+ The reporting of EOS events have been made more rigorous. EOS is only >+ expected after a demuxing error, otherwise it's a g_critical. >+ >+ AppendPipeline::abort() has been renamed to >+ AppendPipeline::resetParserState() to honor the fact that it's not >+ only called when the user calls abort() and match better the names >+ used in the spec. >+ >+ Test: imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec.html >+ >+ * platform/AbortableTaskQueue.h: >+ * platform/graphics/gstreamer/mse/AppendPipeline.cpp: >+ (WebCore::assertedElementSetState): >+ (WebCore::AppendPipeline::AppendPipeline): >+ (WebCore::AppendPipeline::~AppendPipeline): >+ (WebCore::AppendPipeline::handleErrorSyncMessage): >+ (WebCore::AppendPipeline::appsrcEndOfAppendCheckerProbe): >+ (WebCore::AppendPipeline::handleNeedContextSyncMessage): >+ (WebCore::AppendPipeline::appsinkCapsChanged): >+ (WebCore::AppendPipeline::handleEndOfAppend): >+ (WebCore::AppendPipeline::appsinkNewSample): >+ (WebCore::AppendPipeline::didReceiveInitializationSegment): >+ (WebCore::AppendPipeline::resetParserState): >+ (WebCore::AppendPipeline::pushNewBuffer): >+ (WebCore::AppendPipeline::handleAppsinkNewSampleFromStreamingThread): >+ (WebCore::AppendPipeline::connectDemuxerSrcPadToAppsinkFromStreamingThread): >+ (WebCore::AppendPipeline::connectDemuxerSrcPadToAppsink): >+ (WebCore::AppendPipeline::disconnectDemuxerSrcPadFromAppsinkFromAnyThread): >+ (WebCore::AppendPipeline::dumpAppendState): Deleted. >+ (WebCore::AppendPipeline::demuxerNoMorePads): Deleted. >+ (WebCore::AppendPipeline::setAppendState): Deleted. >+ (WebCore::AppendPipeline::appsinkEOS): Deleted. >+ (WebCore::AppendPipeline::resetPipeline): Deleted. >+ (WebCore::AppendPipeline::abort): Deleted. >+ * platform/graphics/gstreamer/mse/AppendPipeline.h: >+ (WebCore::AppendPipeline::appendState): Deleted. >+ * platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp: >+ (WebCore::MediaSourceClientGStreamerMSE::abort): >+ (WebCore::MediaSourceClientGStreamerMSE::resetParserState): >+ * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp: >+ (WebCore::SourceBufferPrivateGStreamer::appendParsingFailed): >+ * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h: >+ > 2018-11-21 Carlos Garcia Campos <cgarcia@igalia.com> > > REGRESSION(r237845): [cairo] Hyperlink underscore layout issue >diff --git a/Source/WebCore/platform/AbortableTaskQueue.h b/Source/WebCore/platform/AbortableTaskQueue.h >index 78a37f8a4630a345210b9867770d24d3e7a27f6d..a669d9415cdefa41dedb82eba29a16cbbb2c1cab 100644 >--- a/Source/WebCore/platform/AbortableTaskQueue.h >+++ b/Source/WebCore/platform/AbortableTaskQueue.h >@@ -134,6 +134,9 @@ public: > // forwarded to the background thread, wrapped in an optional. > // > // If we are aborting, the call finishes immediately, returning an empty optional. >+ // >+ // It is allowed for the main thread task handler to abort the AbortableTaskQueue. In that case, the return >+ // value is discarded and the caller receives an empty optional. > template<typename R> > std::optional<R> enqueueTaskAndWait(WTF::Function<R()>&& mainThreadTaskHandler) > { >@@ -148,7 +151,8 @@ public: > postTask([this, &response, &mainThreadTaskHandler]() { > R responseValue = mainThreadTaskHandler(); > LockHolder lockHolder(m_mutex); >- response = WTFMove(responseValue); >+ if (!m_aborting) >+ response = WTFMove(responseValue); > m_abortedOrResponseSet.notifyAll(); > }); > m_abortedOrResponseSet.wait(m_mutex, [this, &response]() { >diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp >index b1fbac6c70be6778d8530115f0055b1839b5784e..026a20573ec32adb2a3bc8a686023ea80ad83108 100644 >--- a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp >+++ b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp >@@ -67,28 +67,6 @@ void AppendPipeline::staticInitialization() > s_webKitEndOfAppendMetaInfo = gst_meta_register(s_endOfAppendMetaType, "WebKitEndOfAppendMeta", sizeof(EndOfAppendMeta), EndOfAppendMeta::init, EndOfAppendMeta::free, EndOfAppendMeta::transform); > } > >-const char* AppendPipeline::dumpAppendState(AppendPipeline::AppendState appendState) >-{ >- switch (appendState) { >- case AppendPipeline::AppendState::Invalid: >- return "Invalid"; >- case AppendPipeline::AppendState::NotStarted: >- return "NotStarted"; >- case AppendPipeline::AppendState::Ongoing: >- return "Ongoing"; >- case AppendPipeline::AppendState::DataStarve: >- return "DataStarve"; >- case AppendPipeline::AppendState::Sampling: >- return "Sampling"; >- case AppendPipeline::AppendState::LastSample: >- return "LastSample"; >- case AppendPipeline::AppendState::Aborting: >- return "Aborting"; >- default: >- return "(unknown)"; >- } >-} >- > #if !LOG_DISABLED > static GstPadProbeReturn appendPipelinePadProbeDebugInformation(GstPad*, GstPadProbeInfo*, struct PadProbeInformation*); > #endif >@@ -101,14 +79,30 @@ static GstPadProbeReturn appendPipelineDemuxerBlackHolePadProbe(GstPad*, GstPadP > > static GstPadProbeReturn matroskademuxForceSegmentStartToEqualZero(GstPad*, GstPadProbeInfo*, void*); > >+// Wrapper for gst_element_set_state() that emits a critical if the state change fails or is not synchronous. >+static void assertedElementSetState(GstElement* element, GstState desiredState) >+{ >+ GstState oldState; >+ gst_element_get_state(element, &oldState, nullptr, 0); >+ >+ GstStateChangeReturn result = gst_element_set_state(element, desiredState); >+ >+ GstState newState; >+ gst_element_get_state(element, &newState, nullptr, 0); >+ >+ if (desiredState != newState || result != GST_STATE_CHANGE_SUCCESS) { >+ GST_ERROR("AppendPipeline state change failed (returned %d): %" GST_PTR_FORMAT " %d -> %d (expected %d)", >+ static_cast<int>(result), element, static_cast<int>(oldState), static_cast<int>(newState), static_cast<int>(desiredState)); >+ ASSERT_NOT_REACHED(); >+ } >+} >+ > AppendPipeline::AppendPipeline(Ref<MediaSourceClientGStreamerMSE> mediaSourceClient, Ref<SourceBufferPrivateGStreamer> sourceBufferPrivate, MediaPlayerPrivateGStreamerMSE& playerPrivate) > : m_mediaSourceClient(mediaSourceClient.get()) > , m_sourceBufferPrivate(sourceBufferPrivate.get()) > , m_playerPrivate(&playerPrivate) > , m_id(0) > , m_wasBusAlreadyNotifiedOfAvailableSamples(false) >- , m_appendState(AppendState::NotStarted) >- , m_abortPending(false) > , m_streamType(Unknown) > { > ASSERT(isMainThread()); >@@ -127,6 +121,9 @@ AppendPipeline::AppendPipeline(Ref<MediaSourceClientGStreamerMSE> mediaSourceCli > gst_bus_add_signal_watch_full(m_bus.get(), RunLoopSourcePriority::RunLoopDispatcher); > gst_bus_enable_sync_message_emission(m_bus.get()); > >+ g_signal_connect(m_bus.get(), "sync-message::error", G_CALLBACK(+[](GstBus*, GstMessage* message, AppendPipeline* appendPipeline) { >+ appendPipeline->handleErrorSyncMessage(message); >+ }), this); > g_signal_connect(m_bus.get(), "sync-message::need-context", G_CALLBACK(+[](GstBus*, GstMessage* message, AppendPipeline* appendPipeline) { > appendPipeline->handleNeedContextSyncMessage(message); > }), this); >@@ -155,6 +152,8 @@ AppendPipeline::AppendPipeline(Ref<MediaSourceClientGStreamerMSE> mediaSourceCli > > gst_app_sink_set_emit_signals(GST_APP_SINK(m_appsink.get()), TRUE); > gst_base_sink_set_sync(GST_BASE_SINK(m_appsink.get()), FALSE); >+ gst_base_sink_set_async_enabled(GST_BASE_SINK(m_appsink.get()), FALSE); // No prerolls, no async state changes. >+ gst_base_sink_set_drop_out_of_segment(GST_BASE_SINK(m_appsink.get()), FALSE); > gst_base_sink_set_last_sample_enabled(GST_BASE_SINK(m_appsink.get()), FALSE); > > GRefPtr<GstPad> appsinkPad = adoptGRef(gst_element_get_static_pad(m_appsink.get(), "sink")); >@@ -202,33 +201,32 @@ AppendPipeline::AppendPipeline(Ref<MediaSourceClientGStreamerMSE> mediaSourceCli > ASSERT(!isMainThread()); > GST_DEBUG("Posting no-more-pads task to main thread"); > appendPipeline->m_taskQueue.enqueueTask([appendPipeline]() { >- appendPipeline->demuxerNoMorePads(); >+ appendPipeline->didReceiveInitializationSegment(); > }); > }), this); > g_signal_connect(m_appsink.get(), "new-sample", G_CALLBACK(+[](GstElement* appsink, AppendPipeline* appendPipeline) { > appendPipeline->handleAppsinkNewSampleFromStreamingThread(appsink); > }), this); > g_signal_connect(m_appsink.get(), "eos", G_CALLBACK(+[](GstElement*, AppendPipeline* appendPipeline) { >- ASSERT(!isMainThread()); >- GST_DEBUG("Posting appsink-eos task to main thread"); >- appendPipeline->m_taskQueue.enqueueTask([appendPipeline]() { >- appendPipeline->appsinkEOS(); >- }); >+ // basesrc will emit an EOS after it has received a GST_FLOW_ERROR. That's the only case we are expecting. >+ if (!appendPipeline->m_errorReceived) { >+ GST_ERROR("Unexpected appsink EOS in AppendPipeline"); >+ ASSERT_NOT_REACHED(); >+ } > }), this); > > // Add_many will take ownership of a reference. That's why we used an assignment before. > gst_bin_add_many(GST_BIN(m_pipeline.get()), m_appsrc.get(), m_demux.get(), nullptr); > gst_element_link(m_appsrc.get(), m_demux.get()); > >- gst_element_set_state(m_pipeline.get(), GST_STATE_READY); >-}; >+ assertedElementSetState(m_pipeline.get(), GST_STATE_PLAYING); >+} > > AppendPipeline::~AppendPipeline() > { >- GST_TRACE("Destructing AppendPipeline (%p)", this); >+ GST_DEBUG_OBJECT(m_pipeline.get(), "Destructing AppendPipeline (%p)", this); > ASSERT(isMainThread()); > >- setAppendState(AppendState::Invalid); > // Forget all pending tasks and unblock the streaming thread if it was blocked. > m_taskQueue.startAborting(); > >@@ -273,6 +271,21 @@ AppendPipeline::~AppendPipeline() > gst_element_set_state(m_pipeline.get(), GST_STATE_NULL); > } > >+void AppendPipeline::handleErrorSyncMessage(GstMessage* message) >+{ >+ ASSERT(!isMainThread()); >+ GST_WARNING_OBJECT(m_pipeline.get(), "Demuxing error: %" GST_PTR_FORMAT, message); >+ // Notify the main thread that the append has a decode error. >+ auto response = m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([this]() { >+ m_errorReceived = true; >+ // appendParsingFailed() will cause resetParserState() to be called. >+ m_sourceBufferPrivate->appendParsingFailed(); >+ return AbortableTaskQueue::Void(); >+ }); >+ // The streaming thread has now been unblocked because we are aborting in the main thread. >+ ASSERT(!response); >+} >+ > GstPadProbeReturn AppendPipeline::appsrcEndOfAppendCheckerProbe(GstPadProbeInfo* padProbeInfo) > { > ASSERT(!isMainThread()); >@@ -281,6 +294,8 @@ GstPadProbeReturn AppendPipeline::appsrcEndOfAppendCheckerProbe(GstPadProbeInfo* > GstBuffer* buffer = GST_BUFFER(padProbeInfo->data); > ASSERT(GST_IS_BUFFER(buffer)); > >+ GST_TRACE_OBJECT(m_pipeline.get(), "Buffer entered appsrcEndOfAppendCheckerProbe: %" GST_PTR_FORMAT, buffer); >+ > EndOfAppendMeta* endOfAppendMeta = reinterpret_cast<EndOfAppendMeta*>(gst_buffer_get_meta(buffer, s_endOfAppendMetaType)); > if (!endOfAppendMeta) { > // Normal buffer, nothing to do. >@@ -301,16 +316,7 @@ void AppendPipeline::handleNeedContextSyncMessage(GstMessage* message) > GST_TRACE("context type: %s", contextType); > > // MediaPlayerPrivateGStreamerBase will take care of setting up encryption. >- if (m_playerPrivate) >- m_playerPrivate->handleSyncMessage(message); >-} >- >-void AppendPipeline::demuxerNoMorePads() >-{ >- GST_TRACE("calling didReceiveInitializationSegment"); >- didReceiveInitializationSegment(); >- GST_TRACE("set pipeline to playing"); >- gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING); >+ m_playerPrivate->handleSyncMessage(message); > } > > void AppendPipeline::handleStateChangeMessage(GstMessage* message) >@@ -364,146 +370,6 @@ gint AppendPipeline::id() > return m_id; > } > >-void AppendPipeline::setAppendState(AppendState newAppendState) >-{ >- ASSERT(isMainThread()); >- // Valid transitions: >- // NotStarted-->Ongoing-->DataStarve-->NotStarted >- // | | `->Aborting-->NotStarted >- // | `->Sampling-···->Sampling-->LastSample-->NotStarted >- // | `->Aborting-->NotStarted >- // `->Aborting-->NotStarted >- AppendState oldAppendState = m_appendState; >- AppendState nextAppendState = AppendState::Invalid; >- >- if (oldAppendState != newAppendState) >- GST_TRACE("%s --> %s", dumpAppendState(oldAppendState), dumpAppendState(newAppendState)); >- >- bool ok = false; >- >- switch (oldAppendState) { >- case AppendState::NotStarted: >- switch (newAppendState) { >- case AppendState::Ongoing: >- ok = true; >- gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING); >- break; >- case AppendState::NotStarted: >- ok = true; >- if (m_pendingBuffer) { >- GST_TRACE("pushing pending buffer %" GST_PTR_FORMAT, m_pendingBuffer.get()); >- gst_app_src_push_buffer(GST_APP_SRC(appsrc()), m_pendingBuffer.leakRef()); >- nextAppendState = AppendState::Ongoing; >- } >- break; >- case AppendState::Aborting: >- ok = true; >- nextAppendState = AppendState::NotStarted; >- break; >- case AppendState::Invalid: >- ok = true; >- break; >- default: >- break; >- } >- break; >- case AppendState::Ongoing: >- switch (newAppendState) { >- case AppendState::Sampling: >- case AppendState::Invalid: >- ok = true; >- break; >- case AppendState::DataStarve: >- ok = true; >- GST_DEBUG("received all pending samples"); >- m_sourceBufferPrivate->didReceiveAllPendingSamples(); >- if (m_abortPending) >- nextAppendState = AppendState::Aborting; >- else >- nextAppendState = AppendState::NotStarted; >- break; >- default: >- break; >- } >- break; >- case AppendState::DataStarve: >- switch (newAppendState) { >- case AppendState::NotStarted: >- case AppendState::Invalid: >- ok = true; >- break; >- case AppendState::Aborting: >- ok = true; >- nextAppendState = AppendState::NotStarted; >- break; >- default: >- break; >- } >- break; >- case AppendState::Sampling: >- switch (newAppendState) { >- case AppendState::Sampling: >- case AppendState::Invalid: >- ok = true; >- break; >- case AppendState::LastSample: >- ok = true; >- GST_DEBUG("received all pending samples"); >- m_sourceBufferPrivate->didReceiveAllPendingSamples(); >- if (m_abortPending) >- nextAppendState = AppendState::Aborting; >- else >- nextAppendState = AppendState::NotStarted; >- break; >- default: >- break; >- } >- break; >- case AppendState::LastSample: >- switch (newAppendState) { >- case AppendState::NotStarted: >- case AppendState::Invalid: >- ok = true; >- break; >- case AppendState::Aborting: >- ok = true; >- nextAppendState = AppendState::NotStarted; >- break; >- default: >- break; >- } >- break; >- case AppendState::Aborting: >- switch (newAppendState) { >- case AppendState::NotStarted: >- ok = true; >- resetPipeline(); >- m_abortPending = false; >- nextAppendState = AppendState::NotStarted; >- break; >- case AppendState::Invalid: >- ok = true; >- break; >- default: >- break; >- } >- break; >- case AppendState::Invalid: >- ok = true; >- break; >- } >- >- if (ok) >- m_appendState = newAppendState; >- else >- GST_ERROR("Invalid append state transition %s --> %s", dumpAppendState(oldAppendState), dumpAppendState(newAppendState)); >- >- ASSERT(ok); >- >- if (nextAppendState != AppendState::Invalid) >- setAppendState(nextAppendState); >-} >- > void AppendPipeline::parseDemuxerSrcPadCaps(GstCaps* demuxerSrcPadCaps) > { > ASSERT(isMainThread()); >@@ -536,9 +402,6 @@ void AppendPipeline::appsinkCapsChanged() > { > ASSERT(isMainThread()); > >- if (!m_appsink) >- return; >- > GRefPtr<GstPad> pad = adoptGRef(gst_element_get_static_pad(m_appsink.get(), "sink")); > GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(pad.get())); > >@@ -550,31 +413,16 @@ void AppendPipeline::appsinkCapsChanged() > > if (m_appsinkCaps != caps) { > m_appsinkCaps = WTFMove(caps); >- if (m_playerPrivate) >- m_playerPrivate->trackDetected(this, m_track, previousCapsWereNull); >- gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING); >+ m_playerPrivate->trackDetected(this, m_track, previousCapsWereNull); > } > } > > void AppendPipeline::handleEndOfAppend() > { > ASSERT(isMainThread()); >- GST_TRACE_OBJECT(m_pipeline.get(), "received end-of-append"); >- >- // Regardless of the state transition, the result is the same: didReceiveAllPendingSamples() is called. >- switch (m_appendState) { >- case AppendState::Ongoing: >- GST_TRACE("DataStarve"); >- setAppendState(AppendState::DataStarve); >- break; >- case AppendState::Sampling: >- GST_TRACE("LastSample"); >- setAppendState(AppendState::LastSample); >- break; >- default: >- ASSERT_NOT_REACHED(); >- break; >- } >+ consumeAppsinkAvailableSamples(); >+ GST_TRACE_OBJECT(m_pipeline.get(), "Notifying SourceBufferPrivate the append is complete"); >+ sourceBufferPrivate()->didReceiveAllPendingSamples(); > } > > void AppendPipeline::appsinkNewSample(GRefPtr<GstSample>&& sample) >@@ -595,11 +443,10 @@ void AppendPipeline::appsinkNewSample(GRefPtr<GstSample>&& sample) > mediaSample->duration().toString().utf8().data(), > mediaSample->presentationSize().width(), mediaSample->presentationSize().height()); > >- // If we're beyond the duration, ignore this sample and the remaining ones. >+ // If we're beyond the duration, ignore this sample. > MediaTime duration = m_mediaSourceClient->duration(); > if (duration.isValid() && !duration.indefiniteTime() && mediaSample->presentationTime() > duration) { >- GST_DEBUG("Detected sample (%f) beyond the duration (%f), declaring LastSample", mediaSample->presentationTime().toFloat(), duration.toFloat()); >- setAppendState(AppendState::LastSample); >+ GST_DEBUG_OBJECT(m_pipeline.get(), "Detected sample (%s) beyond the duration (%s), discarding", mediaSample->presentationTime().toString().utf8().data(), duration.toString().utf8().data()); > return; > } > >@@ -610,28 +457,6 @@ void AppendPipeline::appsinkNewSample(GRefPtr<GstSample>&& sample) > } > > m_sourceBufferPrivate->didReceiveSample(*mediaSample); >- setAppendState(AppendState::Sampling); >-} >- >-void AppendPipeline::appsinkEOS() >-{ >- ASSERT(isMainThread()); >- >- switch (m_appendState) { >- case AppendState::Aborting: >- // Ignored. Operation completion will be managed by the Aborting->NotStarted transition. >- return; >- case AppendState::Ongoing: >- // Finish Ongoing and Sampling states. >- setAppendState(AppendState::DataStarve); >- break; >- case AppendState::Sampling: >- setAppendState(AppendState::LastSample); >- break; >- default: >- GST_DEBUG("Unexpected EOS"); >- break; >- } > } > > void AppendPipeline::didReceiveInitializationSegment() >@@ -690,51 +515,45 @@ void AppendPipeline::consumeAppsinkAvailableSamples() > GST_TRACE_OBJECT(m_pipeline.get(), "batchedSampleCount = %d", batchedSampleCount); > } > >-void AppendPipeline::resetPipeline() >+void AppendPipeline::resetParserState() > { > ASSERT(isMainThread()); >- GST_DEBUG("resetting pipeline"); >+ GST_DEBUG_OBJECT(m_pipeline.get(), "Handling resetParserState() in AppendPipeline by resetting the pipeline"); >+ >+ // FIXME: Implement a flush event-based resetParserState() implementation would allow the initialization segment to >+ // survive, in accordance with the spec. >+ >+ // This function restores the GStreamer pipeline to the same state it was when the AppendPipeline constructor >+ // finished. All previously enqueued data is lost and the demuxer is reset, losing all pads and track data. >+ >+ // Unlock the streaming thread. >+ m_taskQueue.startAborting(); >+ >+ // Reset the state of all elements in the pipeline. >+ assertedElementSetState(m_pipeline.get(), GST_STATE_READY); >+ >+ // The parser is tear down automatically when the demuxer is reset (see disconnectDemuxerSrcPadFromAppsinkFromAnyThread()). >+ ASSERT(!m_parser); >+ >+ // Set the pipeline to PLAYING so that it can be used again. >+ assertedElementSetState(m_pipeline.get(), GST_STATE_PLAYING); > >- gst_element_set_state(m_pipeline.get(), GST_STATE_READY); >- gst_element_get_state(m_pipeline.get(), nullptr, nullptr, 0); >+ // All processing related to the previous append has been aborted and the pipeline is idle. >+ // We can listen again to new requests coming from the streaming thread. >+ m_taskQueue.finishAborting(); > > #if (!(LOG_DISABLED || defined(GST_DISABLE_GST_DEBUG))) > { > static unsigned i = 0; > // This is here for debugging purposes. It does not make sense to have it as class member. >- WTF::String dotFileName = String::format("reset-pipeline-%d", ++i); >+ WTF::String dotFileName = String::format("reset-pipeline-%d", ++i); > gst_debug_bin_to_dot_file(GST_BIN(m_pipeline.get()), GST_DEBUG_GRAPH_SHOW_ALL, dotFileName.utf8().data()); > } > #endif >- >-} >- >-void AppendPipeline::abort() >-{ >- ASSERT(isMainThread()); >- GST_DEBUG("aborting"); >- >- m_pendingBuffer = nullptr; >- >- // Abort already ongoing. >- if (m_abortPending) >- return; >- >- m_abortPending = true; >- if (m_appendState == AppendState::NotStarted) >- setAppendState(AppendState::Aborting); >- // Else, the automatic state transitions will take care when the ongoing append finishes. > } > > GstFlowReturn AppendPipeline::pushNewBuffer(GstBuffer* buffer) > { >- if (m_abortPending) { >- m_pendingBuffer = adoptGRef(buffer); >- return GST_FLOW_OK; >- } >- >- setAppendState(AppendPipeline::AppendState::Ongoing); >- > GST_TRACE_OBJECT(m_pipeline.get(), "pushing data buffer %" GST_PTR_FORMAT, buffer); > GstFlowReturn pushDataBufferRet = gst_app_src_push_buffer(GST_APP_SRC(m_appsrc.get()), buffer); > // Pushing buffers to appsrc can only fail if the appsrc is flushing, in EOS or stopped. Neither of these should >@@ -770,15 +589,10 @@ GstFlowReturn AppendPipeline::handleAppsinkNewSampleFromStreamingThread(GstEleme > // removes the appsrcEndOfAppendCheckerProbe(). Either way, the end-of-append detection would be broken. > // AppendPipeline should have only one streaming thread. Otherwise we can't detect reliably when an appends has > // been demuxed completely.; >- g_critical("Appsink received a sample in a different thread than appsrcEndOfAppendCheckerProbe run."); >+ GST_ERROR_OBJECT(m_pipeline.get(), "Appsink received a sample in a different thread than appsrcEndOfAppendCheckerProbe run."); > ASSERT_NOT_REACHED(); > } > >- if (!m_playerPrivate || m_appendState == AppendState::Invalid) { >- GST_WARNING("AppendPipeline has been disabled, ignoring this sample"); >- return GST_FLOW_ERROR; >- } >- > if (!m_wasBusAlreadyNotifiedOfAvailableSamples.test_and_set()) { > GST_TRACE("Posting appsink-new-sample task to the main thread"); > m_taskQueue.enqueueTask([this]() { >@@ -825,8 +639,6 @@ createOptionalParserForFormat(GstPad* demuxerSrcPad) > void AppendPipeline::connectDemuxerSrcPadToAppsinkFromStreamingThread(GstPad* demuxerSrcPad) > { > ASSERT(!isMainThread()); >- if (!m_appsink) >- return; > > GST_DEBUG("connecting to appsink"); > >@@ -865,7 +677,6 @@ void AppendPipeline::connectDemuxerSrcPadToAppsinkFromStreamingThread(GstPad* de > || (m_streamType == WebCore::MediaSourceStreamTypeGStreamer::Text); > > if (isData) { >- // FIXME: Only add appsink one time. This method can be called several times. > GRefPtr<GstObject> parent = adoptGRef(gst_element_get_parent(m_appsink.get())); > if (!parent) > gst_bin_add(GST_BIN(m_pipeline.get()), m_appsink.get()); >@@ -892,9 +703,6 @@ void AppendPipeline::connectDemuxerSrcPadToAppsinkFromStreamingThread(GstPad* de > > gst_element_sync_state_with_parent(m_appsink.get()); > >- gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED); >- gst_element_sync_state_with_parent(m_appsink.get()); >- > GST_DEBUG_BIN_TO_DOT_FILE_WITH_TS(GST_BIN(m_pipeline.get()), GST_DEBUG_GRAPH_SHOW_ALL, "webkit-after-link"); > } > } >@@ -915,10 +723,6 @@ void AppendPipeline::connectDemuxerSrcPadToAppsink(GstPad* demuxerSrcPad) > > GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(GST_PAD(demuxerSrcPad))); > >- if (!caps || m_appendState == AppendState::Invalid || !m_playerPrivate) { >- return; >- } >- > #ifndef GST_DISABLE_GST_DEBUG > { > GUniquePtr<gchar> strcaps(gst_caps_to_string(caps.get())); >@@ -933,34 +737,31 @@ void AppendPipeline::connectDemuxerSrcPadToAppsink(GstPad* demuxerSrcPad) > > switch (m_streamType) { > case WebCore::MediaSourceStreamTypeGStreamer::Audio: >- if (m_playerPrivate) >- m_track = WebCore::AudioTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get()); >+ m_track = WebCore::AudioTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get()); > break; > case WebCore::MediaSourceStreamTypeGStreamer::Video: >- if (m_playerPrivate) >- m_track = WebCore::VideoTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get()); >+ m_track = WebCore::VideoTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get()); > break; > case WebCore::MediaSourceStreamTypeGStreamer::Text: > m_track = WebCore::InbandTextTrackPrivateGStreamer::create(id(), sinkSinkPad.get()); > break; > case WebCore::MediaSourceStreamTypeGStreamer::Invalid: >- { >- GUniquePtr<gchar> strcaps(gst_caps_to_string(caps.get())); >- GST_DEBUG("Unsupported track codec: %s", strcaps.get()); >- } >- // This is going to cause an error which will detach the SourceBuffer and tear down this >- // AppendPipeline, so we need the padAddRemove lock released before continuing. >- m_track = nullptr; >- didReceiveInitializationSegment(); >+ GST_WARNING_OBJECT(m_pipeline.get(), "Unsupported track codec: %" GST_PTR_FORMAT, caps.get()); >+ // 3.5.7 Initialization Segment Received >+ // 5.1. If the initialization segment contains tracks with codecs the user agent does not support, then run the >+ // append error algorithm and abort these steps. >+ >+ // appendParsingFailed() will immediately cause a resetParserState() which will stop demuxing, then the >+ // AppendPipeline will be destroyed. >+ m_sourceBufferPrivate->appendParsingFailed(); > return; > default: >- // No useful data. >+ GST_WARNING_OBJECT(m_pipeline.get(), "Pad has unknown track type, ignoring: %" GST_PTR_FORMAT, caps.get()); > break; > } > > m_appsinkCaps = WTFMove(caps); >- if (m_playerPrivate) >- m_playerPrivate->trackDetected(this, m_track, true); >+ m_playerPrivate->trackDetected(this, m_track, true); > } > > void AppendPipeline::disconnectDemuxerSrcPadFromAppsinkFromAnyThread(GstPad*) >@@ -973,7 +774,7 @@ void AppendPipeline::disconnectDemuxerSrcPadFromAppsinkFromAnyThread(GstPad*) > GST_DEBUG("Disconnecting appsink"); > > if (m_parser) { >- gst_element_set_state(m_parser.get(), GST_STATE_NULL); >+ assertedElementSetState(m_parser.get(), GST_STATE_NULL); > gst_bin_remove(GST_BIN(m_pipeline.get()), m_parser.get()); > m_parser = nullptr; > } >diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h >index 871b436ea001d4a46f4a6c2a5b91c0f176018f73..d3a520d8c7220089cb5f59eddf91c9a9d98a873d 100644 >--- a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h >+++ b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h >@@ -50,21 +50,20 @@ public: > virtual ~AppendPipeline(); > > GstFlowReturn pushNewBuffer(GstBuffer*); >- void abort(); >+ void resetParserState(); > Ref<SourceBufferPrivateGStreamer> sourceBufferPrivate() { return m_sourceBufferPrivate.get(); } > GstCaps* appsinkCaps() { return m_appsinkCaps.get(); } > RefPtr<WebCore::TrackPrivateBase> track() { return m_track; } > MediaPlayerPrivateGStreamerMSE* playerPrivate() { return m_playerPrivate; } > > private: >- enum class AppendState { Invalid, NotStarted, Ongoing, DataStarve, Sampling, LastSample, Aborting }; > >+ void handleErrorSyncMessage(GstMessage*); > void handleNeedContextSyncMessage(GstMessage*); >+ // For debug purposes only: > void handleStateChangeMessage(GstMessage*); > > gint id(); >- AppendState appendState() { return m_appendState; } >- void setAppendState(AppendState); > > GstFlowReturn handleAppsinkNewSampleFromStreamingThread(GstElement*); > >@@ -72,7 +71,6 @@ private: > void parseDemuxerSrcPadCaps(GstCaps*); > void appsinkCapsChanged(); > void appsinkNewSample(GRefPtr<GstSample>&&); >- void appsinkEOS(); > void handleEndOfAppend(); > void didReceiveInitializationSegment(); > AtomicString trackId(); >@@ -89,15 +87,12 @@ private: > void connectDemuxerSrcPadToAppsink(GstPad*); > > void resetPipeline(); >- void checkEndOfAppend(); >- void demuxerNoMorePads(); > > void consumeAppsinkAvailableSamples(); > > GstPadProbeReturn appsrcEndOfAppendCheckerProbe(GstPadProbeInfo*); > > static void staticInitialization(); >- static const char* dumpAppendState(AppendPipeline::AppendState); > > static std::once_flag s_staticInitializationFlag; > static GType s_endOfAppendMetaType; >@@ -107,6 +102,9 @@ private: > // Only the pointers are compared. > WTF::Thread* m_streamingThread; > >+ // Used only for asserting EOS events are only caused by demuxing errors. >+ bool m_errorReceived { false }; >+ > Ref<MediaSourceClientGStreamerMSE> m_mediaSourceClient; > Ref<SourceBufferPrivateGStreamer> m_sourceBufferPrivate; > MediaPlayerPrivateGStreamerMSE* m_playerPrivate; >@@ -143,14 +141,6 @@ private: > #if ENABLE(ENCRYPTED_MEDIA) > struct PadProbeInformation m_appsinkPadEventProbeInformation; > #endif >- // Keeps track of the states of append processing, to avoid performing actions inappropriate for the current state >- // (eg: processing more samples when the last one has been detected, etc.). See setAppendState() for valid >- // transitions. >- AppendState m_appendState; >- >- // Aborts can only be completed when the normal sample detection has finished. Meanwhile, the willing to abort is >- // expressed in this field. >- bool m_abortPending; > > WebCore::MediaSourceStreamTypeGStreamer m_streamType; > RefPtr<WebCore::TrackPrivateBase> m_track; >diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp >index 38bdd6c24138c9344ac785579a0224acffa5a9fb..c84fc3659854b9e04b126d3e60300febf218bc6a 100644 >--- a/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp >+++ b/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp >@@ -106,7 +106,7 @@ void MediaSourceClientGStreamerMSE::abort(RefPtr<SourceBufferPrivateGStreamer> s > > ASSERT(appendPipeline); > >- appendPipeline->abort(); >+ appendPipeline->resetParserState(); > } > > void MediaSourceClientGStreamerMSE::resetParserState(RefPtr<SourceBufferPrivateGStreamer> sourceBufferPrivate) >@@ -122,7 +122,7 @@ void MediaSourceClientGStreamerMSE::resetParserState(RefPtr<SourceBufferPrivateG > > ASSERT(appendPipeline); > >- appendPipeline->abort(); >+ appendPipeline->resetParserState(); > } > > bool MediaSourceClientGStreamerMSE::append(RefPtr<SourceBufferPrivateGStreamer> sourceBufferPrivate, Vector<unsigned char>&& data) >diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp >index 0f1c4e6c93a27c5cfd183cdf7202193fe5189122..fa4f5616cd0144aca01adfcefd374ebdea577d80 100644 >--- a/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp >+++ b/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp >@@ -178,5 +178,11 @@ void SourceBufferPrivateGStreamer::didReceiveAllPendingSamples() > m_sourceBufferPrivateClient->sourceBufferPrivateAppendComplete(SourceBufferPrivateClient::AppendSucceeded); > } > >+void SourceBufferPrivateGStreamer::appendParsingFailed() >+{ >+ if (m_sourceBufferPrivateClient) >+ m_sourceBufferPrivateClient->sourceBufferPrivateAppendComplete(SourceBufferPrivateClient::ParsingFailed); >+} >+ > } > #endif >diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h >index b07c2b02718ab8957b26f1b77e2b0a6b841d4998..18f1abd6d116871a6c4a82116a23b637c82e907d 100644 >--- a/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h >+++ b/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h >@@ -76,6 +76,7 @@ public: > void didReceiveInitializationSegment(const SourceBufferPrivateClient::InitializationSegment&); > void didReceiveSample(MediaSample&); > void didReceiveAllPendingSamples(); >+ void appendParsingFailed(); > > ContentType type() const { return m_type; } > >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 1a5715b606a2a787571e15e33e0aa4becbf701fd..f71a072e31789b7089b9a5a332658150f347325d 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,24 @@ >+2018-11-29 Alicia Boya GarcÃa <aboya@igalia.com> >+ >+ [MSE][GStreamer] Remove the AppendPipeline state machine >+ https://bugs.webkit.org/show_bug.cgi?id=192204 >+ >+ Reviewed by Xabier Rodriguez-Calvar. >+ >+ Updated AbortableTaskQueue tests: >+ >+ Added test: AbortedBySyncTaskHandler. >+ >+ Renamed test: AbortDuringSyncTask -> AbortBeforeSyncTaskRun (in >+ order to avoid confusion with the new test). >+ >+ Added checks for the correct destruction of response objects. >+ >+ * TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp: >+ (TestWebKitAPI::FancyResponse::FancyResponse): >+ (TestWebKitAPI::FancyResponse::~FancyResponse): >+ (TestWebKitAPI::TEST): >+ > 2018-11-20 Jeff Miller <jeffm@apple.com> > > Return nullptr immediately if the key doesn't exist in the HashMap. >diff --git a/Tools/TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp b/Tools/TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp >index 5df8f5a1041c289fd549bb8f22b13bdeecda125e..5fee1f7cbebdecc2a3b64b36ca92f56b145a9a2b 100644 >--- a/Tools/TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp >+++ b/Tools/TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp >@@ -65,13 +65,25 @@ TEST(AbortableTaskQueue, AsyncTasks) > struct FancyResponse { > WTF_MAKE_NONCOPYABLE(FancyResponse); > public: >- FancyResponse(int fancyInt) >+ FancyResponse(int fancyInt, bool* destructedFlagPointer = nullptr) > : fancyInt(fancyInt) >+ , destructedFlagPointer(destructedFlagPointer) > { } > >- FancyResponse(FancyResponse&& a) >- : fancyInt(a.fancyInt) >- { } >+ FancyResponse(FancyResponse&& original) >+ : fancyInt(original.fancyInt) >+ , destructedFlagPointer(original.destructedFlagPointer) >+ { >+ original.destructedFlagPointer = nullptr; >+ } >+ >+ ~FancyResponse() >+ { >+ if (destructedFlagPointer) { >+ RELEASE_ASSERT(!*destructedFlagPointer); >+ *destructedFlagPointer = true; >+ } >+ } > > FancyResponse& operator=(FancyResponse&& a) > { >@@ -80,12 +92,14 @@ public: > } > > int fancyInt; >+ bool* destructedFlagPointer; > }; > > TEST(AbortableTaskQueue, SyncTasks) > { > AbortableTaskQueue taskQueue; > bool testFinished { false }; >+ bool destructedResponseFlag { false }; > int currentStep { 0 }; > RunLoop::initializeMainRunLoop(); > >@@ -95,13 +109,16 @@ TEST(AbortableTaskQueue, SyncTasks) > EXPECT_TRUE(isMainThread()); > currentStep++; > EXPECT_EQ(1, currentStep); >- FancyResponse returnValue(100); >+ FancyResponse returnValue(100, &destructedResponseFlag); > return returnValue; > }); > currentStep++; > EXPECT_EQ(2, currentStep); > EXPECT_TRUE(response); >+ EXPECT_FALSE(destructedResponseFlag); > EXPECT_EQ(100, response->fancyInt); >+ response = std::nullopt; >+ EXPECT_TRUE(destructedResponseFlag); > RunLoop::main().dispatch([&]() { > testFinished = true; > }); >@@ -219,7 +236,7 @@ TEST(AbortableTaskQueue, Abort) > Util::run(&testFinished); > } > >-TEST(AbortableTaskQueue, AbortDuringSyncTask) >+TEST(AbortableTaskQueue, AbortBeforeSyncTaskRun) > { > AbortableTaskQueue taskQueue; > bool testFinished { false }; >@@ -254,4 +271,46 @@ TEST(AbortableTaskQueue, AbortDuringSyncTask) > Util::run(&testFinished); > } > >+TEST(AbortableTaskQueue, AbortedBySyncTaskHandler) >+{ >+ AbortableTaskQueue taskQueue; >+ bool testFinished { false }; >+ int currentStep { 0 }; >+ bool destructedResponseFlag { false }; >+ RunLoop::initializeMainRunLoop(); >+ >+ auto backgroundThreadFunction = [&]() { >+ EXPECT_FALSE(isMainThread()); >+ currentStep++; >+ EXPECT_EQ(1, currentStep); >+ >+ std::optional<FancyResponse> response = taskQueue.enqueueTaskAndWait<FancyResponse>([&]() -> FancyResponse { >+ currentStep++; >+ EXPECT_EQ(2, currentStep); >+ taskQueue.startAborting(); >+ // This task should not have been able to run under the scheduling of this test. >+ return FancyResponse(100, &destructedResponseFlag); >+ }); >+ >+ currentStep++; >+ EXPECT_EQ(3, currentStep); >+ >+ // Main thread has called startAborting(). >+ EXPECT_FALSE(response); >+ >+ // The response object has not been leaked. >+ EXPECT_TRUE(destructedResponseFlag); >+ >+ RunLoop::main().dispatch([&]() { >+ testFinished = true; >+ }); >+ }; >+ RunLoop::current().dispatch([&, backgroundThreadFunction = WTFMove(backgroundThreadFunction)]() mutable { >+ EXPECT_TRUE(isMainThread()); >+ WTF::Thread::create("atq-background", WTFMove(backgroundThreadFunction))->detach(); >+ }); >+ >+ Util::run(&testFinished); >+} >+ > } // namespace TestWebKitAPI >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 5696faa869f5e59fe207a6fd721da45433cfa0fb..115560199806ab3be5100044cd14210b85463cf4 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,20 @@ >+2018-11-29 Alicia Boya GarcÃa <aboya@igalia.com> >+ >+ [MSE][GStreamer] Remove the AppendPipeline state machine >+ https://bugs.webkit.org/show_bug.cgi?id=192204 >+ >+ Reviewed by Xabier Rodriguez-Calvar. >+ >+ Removed timeout expectations for >+ media/media-source/media-source-error-crash.html >+ >+ Added expectations for mediasource-invalid-codec.html for Mac, where >+ WebM is not supported. >+ >+ * platform/gtk/TestExpectations: >+ * platform/wpe/TestExpectations: >+ * platform/mac/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt: Added. >+ > 2018-11-20 Ryosuke Niwa <rniwa@webkit.org> > > Input element gains focus when a selectstart event listener on document prevents the default action >diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog >index 28bc847255c832a38eaac198864983432d529831..6c138d25c26de28b3f5bb6f9aa012268ce183205 100644 >--- a/LayoutTests/imported/w3c/ChangeLog >+++ b/LayoutTests/imported/w3c/ChangeLog >@@ -1,3 +1,18 @@ >+2018-11-29 Alicia Boya GarcÃa <aboya@igalia.com> >+ >+ [MSE][GStreamer] Remove the AppendPipeline state machine >+ https://bugs.webkit.org/show_bug.cgi?id=192204 >+ >+ Reviewed by Xabier Rodriguez-Calvar. >+ >+ Added a test checking that initialization segments with invalid codec >+ identifiers are flagged as errors. >+ >+ * web-platform-tests/media-source/mediasource-invalid-codec-expected.txt: Added. >+ * web-platform-tests/media-source/mediasource-invalid-codec.html: Added. >+ * web-platform-tests/media-source/mp4/invalid-codec.mp4: Added. >+ * web-platform-tests/media-source/webm/invalid-codec.webm: Added. >+ > 2018-11-20 Manuel Rego Casasnovas <rego@igalia.com> > > [css-grid] Consider scrollbars in populateGridPositionsForDirection() >diff --git a/LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..6d9023652abdde5ed72af07ed24bd2b49b31dd73 >--- /dev/null >+++ b/LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt >@@ -0,0 +1,4 @@ >+ >+PASS Test an MP4 with an invalid codec results in an error. >+PASS Test a WebM with an invalid codec results in an error. >+ >diff --git a/LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec.html b/LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec.html >new file mode 100644 >index 0000000000000000000000000000000000000000..19aa00c4d5f4966ef8a5de657341141f69c08ea3 >--- /dev/null >+++ b/LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec.html >@@ -0,0 +1,45 @@ >+<!DOCTYPE html> >+<html> >+<head> >+ <meta charset="utf-8"> >+ <title>SourceBuffer handling of invalid codecs in the initialization segment</title> >+ <link rel="author" title="Alicia Boya GarcÃa" href="mailto:aboya@igalia.com"> >+ <script src="/resources/testharness.js"></script> >+ <script src="/resources/testharnessreport.js"></script> >+ <script src="mediasource-util.js"></script> >+</head> >+<body> >+<div id="log"></div> >+<script> >+ function testInvalidCodec(test, mediaElement, mediaSource, mediaType, url) { >+ assert_true(MediaSource.isTypeSupported(mediaType), `Media type not supported in this browser: isTypeSupported('${mediaType}')`); >+ >+ MediaSourceUtil.loadBinaryData(test, url, (mediaData) => { >+ _testInvalidCodecWithData(test, mediaElement, mediaSource, mediaType, mediaData); >+ }); >+ } >+ >+ function _testInvalidCodecWithData(test, mediaElement, mediaSource, mediaType, mediaData) { >+ const sourceBuffer = mediaSource.addSourceBuffer(mediaType); >+ sourceBuffer.appendBuffer(mediaData); >+ test.expectEvent(sourceBuffer, 'error', 'Append ended with error'); >+ test.waitForExpectedEvents(() => { >+ test.done(); >+ }) >+ } >+ >+ // These test cases provide a typical media MIME type, but the actual files have been mangled to declare a different, >+ // unsupported, fictitious codec (MP4 fourcc: 'zzzz', WebM codec id 'V_ZZZ'). The browser should report a parsing >+ // error. >+ >+ mediasource_test((test, mediaElement, mediaSource) => { >+ testInvalidCodec(test, mediaElement, mediaSource, 'video/mp4;codecs="avc1.4D4001"', 'mp4/invalid-codec.mp4'); >+ }, 'Test an MP4 with an invalid codec results in an error.'); >+ >+ mediasource_test((test, mediaElement, mediaSource) => { >+ testInvalidCodec(test, mediaElement, mediaSource, 'video/webm; codecs="vp8"', 'webm/invalid-codec.webm'); >+ }, 'Test a WebM with an invalid codec results in an error.'); >+ >+</script> >+</body> >+</html> >diff --git a/LayoutTests/imported/w3c/web-platform-tests/media-source/mp4/invalid-codec.mp4 b/LayoutTests/imported/w3c/web-platform-tests/media-source/mp4/invalid-codec.mp4 >new file mode 100644 >index 0000000000000000000000000000000000000000..6fcc7c21a698ab9d9622c4a6fb6685aeafdd6f40 >GIT binary patch >literal 1542 >zcmchWF>ljA6vv;FQYxqrYJ-#^LPj!dNGwPlKyq7J(!zicWkM*%=MXD)670hgOcqF) >zSU`x0p&x*W2?>S_@C_JPkRq0DC=yZ!mhyj>Say9C0|PI4xp%+&zjyEK3lY`Y^2y+l >zG)?nZm3Sngg?8k5M3t3LIP%%n^EFzYoZ6=qt5Ls0bj#VlMD*~?ueqN;zJ5=BcVF+( >zknC4)X*;F43-5^Tzx*<uq1!f9QL;d8YUD<%tl8GcmX_5t%URbo&E{@VGeV2&X_0W# >zn^Yznnr6c+Yp(5T#4eNdu|`sj=vh?MaL=o)BH^CbMjA;qVrzuTWOb~Okw#LDv@~Mo >zgsA#H?DrGgqaaK=0waC1c0R&C5uN;?+#td?w?h-)(%jCKM@1FvX~@XwV)35rsy&bA >zduG*m?~;}F$&{JOMqu#I1WsgmLdF;yUJy$~RHw7Ls#fJnD0~NNW+;w6`_5;Td9fpc >zh@-F*M2Ro(f6Hz0E<nHO^hDr6WbB20uZ`te5~`vG*(vb{b}lO<P{;{<Zt##<Vw&6# >zk;g^YxYxy*7_l*o<%0n1O)O)<0vKzCPhn~{ozFr%aH{Z=nNSlUI~!;G-RcJi5G=X< >zAnLh+OY_K?2RYI9y+Ea(9ysv;geO_OdMwp*2U6@TM((nPZnNWN`UaTGLRiLF&+!Ya >zWvAhF=I7zd@TLFk=-usOPd4t&f?s_sq=WHt=&2E5{(AYI;BKiq!wZ=$_|wby@5z4v >D7C|I3 > >literal 0 >HcmV?d00001 > >diff --git a/LayoutTests/imported/w3c/web-platform-tests/media-source/webm/invalid-codec.webm b/LayoutTests/imported/w3c/web-platform-tests/media-source/webm/invalid-codec.webm >new file mode 100644 >index 0000000000000000000000000000000000000000..f1c8bdd7abba1f6b24dc0a5d44463c3bda770d0d >GIT binary patch >literal 1206 >zcmZA1O=uHQ5C`zt7@C8iu_aPUz!i%Jk<c8(9ygoR8Z}Vdq?H^rvE7Ce(nyk4z3t1F >z5yUUVkJdvVNT`<5ASgB-Ql#kF3OPtI=phIq6{Ug*b<@l<ne1hk-~4CZK6tx*L!Sx) >z>|AC!nYpZ9jDUQN8RQHz^eS4YSu@_;=b2An?uMs~Pu<G1w8gzJsVwH2Bj6#a?e{K) >z|JMZ5x4E`Hv$J?fuWjQ+`m9R1)n}X}ecj$Sp|-xp@;G(;NVSyOD@(5HIq8<P^Fki2 >zIIJXMYA~Y4#7Hm^i^jvj$jlXSR*l7{qSK<?W<O>du-ReE@u-#bU{chhY7AV#oT>VB >z{rzAcG<Jl`1(xt(tjdHG6XI(*^GId299{i*=u0Xs{LTB6>Fns#gc^k!M*EkOg0#Xm >z=a|8_jAr(|VuG*4x_m48z0A6@+0hGrzaI`bU(7T!hI*YrVSxc?Nq0(yp3Oa5FxYSH >z^#Y6pw*~-0mY>G&pFeEO{t4f!L*W5~gOCO3?qfKmf${Jn!1+6CH7ML?;KagN33iEX >zK7oFw!!zio12kXc3v~E9ba%&`ujnU5GDp~rK1}n%QS=jen)m2l(KqJQs7HAosNY5( >z>d2S4i!{HE?#fgD#a$w9u%J6@)Q@x5sGsBBG3P3Gp!s>-8~b$xsK3U%T%o?fU8BCu >zT_UcZf#yMxfw}|TQKLT2y+S?BU8DYldw}{|?qbUuAAa-5gO&eedY&_QiD>%Ggda&E >F{tq?TdY}LR > >literal 0 >HcmV?d00001 > >diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations >index ab9c2223a8b623c2563b36760c9652a42bcbdba8..9d0c1105f41c0cccdf0f8595ecd0943cbf033874 100644 >--- a/LayoutTests/platform/gtk/TestExpectations >+++ b/LayoutTests/platform/gtk/TestExpectations >@@ -2342,7 +2342,6 @@ webkit.org/b/163780 media/media-controls-drag-timeline-set-controls-property.htm > > webkit.org/b/168373 http/tests/media/track-in-band-hls-metadata-crash.html [ Timeout ] > webkit.org/b/168373 media/media-fullscreen-loop-inline.html [ Timeout ] >-webkit.org/b/168373 media/media-source/media-source-error-crash.html [ Timeout ] > webkit.org/b/168373 media/media-source/only-bcp47-language-tags-accepted-as-valid.html [ Timeout ] > > webkit.org/b/169211 fast/parser/adoption-agency-unload-iframe-4.html [ Timeout ] >diff --git a/LayoutTests/platform/mac/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt b/LayoutTests/platform/mac/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..0a3fdfe1d739b0e2c3058c27482431e12d5c9404 >--- /dev/null >+++ b/LayoutTests/platform/mac/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt >@@ -0,0 +1,4 @@ >+ >+PASS Test an MP4 with an invalid codec results in an error. >+FAIL Test a WebM with an invalid codec results in an error. assert_true: Media type not supported in this browser: isTypeSupported('video/webm; codecs="vp8"') expected true got false >+ >diff --git a/LayoutTests/platform/wpe/TestExpectations b/LayoutTests/platform/wpe/TestExpectations >index c4fa0ed78b827f204d86224e3baaf7f222ed7f19..8f99974cefecb68a689543731dbd1560b43455d2 100644 >--- a/LayoutTests/platform/wpe/TestExpectations >+++ b/LayoutTests/platform/wpe/TestExpectations >@@ -1212,7 +1212,6 @@ legacy-animation-engine/transitions/svg-text-shadow-transition.html [ Pass ] > > # ENABLE_MEDIA_SOURCE > media/media-source/ [ Pass ] >-webkit.org/b/168373 media/media-source/media-source-error-crash.html [ Timeout ] > webkit.org/b/171726 media/media-source/media-source-init-segment-duration.html [ Failure ] > webkit.org/b/168373 media/media-source/media-source-resize.html [ Failure ] > webkit.org/b/165394 media/media-source/media-source-seek-detach-crash.html [ Timeout ]
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=356528">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192204
:
356099
|
356104
|
356109
|
356116
|
356159
| 356528 |
356541