Changeset 238892 in webkit


Ignore:
Timestamp:
Dec 5, 2018 6:56:33 AM (5 years ago)
Author:
aboya@igalia.com
Message:

[MSE][GStreamer] Remove the AppendPipeline state machine
https://bugs.webkit.org/show_bug.cgi?id=192204

Reviewed by Xabier Rodriguez-Calvar.

LayoutTests/imported/w3c:

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.

Source/WebCore:

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:

Tools:

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):

LayoutTests:

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.
Location:
trunk
Files:
5 added
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r238891 r238892  
     12018-12-05  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE][GStreamer] Remove the AppendPipeline state machine
     4        https://bugs.webkit.org/show_bug.cgi?id=192204
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        Removed timeout expectations for
     9        media/media-source/media-source-error-crash.html
     10
     11        Added expectations for mediasource-invalid-codec.html for Mac, where
     12        WebM is not supported.
     13
     14        * platform/gtk/TestExpectations:
     15        * platform/wpe/TestExpectations:
     16        * platform/mac/imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec-expected.txt: Added.
     17
    1182018-12-05  Rob Buis  <rbuis@igalia.com>
    219
  • trunk/LayoutTests/imported/w3c/ChangeLog

    r238891 r238892  
     12018-12-05  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE][GStreamer] Remove the AppendPipeline state machine
     4        https://bugs.webkit.org/show_bug.cgi?id=192204
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        Added a test checking that initialization segments with invalid codec
     9        identifiers are flagged as errors.
     10
     11        * web-platform-tests/media-source/mediasource-invalid-codec-expected.txt: Added.
     12        * web-platform-tests/media-source/mediasource-invalid-codec.html: Added.
     13        * web-platform-tests/media-source/mp4/invalid-codec.mp4: Added.
     14        * web-platform-tests/media-source/webm/invalid-codec.webm: Added.
     15
    1162018-12-05  Rob Buis  <rbuis@igalia.com>
    217
  • trunk/LayoutTests/platform/gtk/TestExpectations

    r238874 r238892  
    23612361webkit.org/b/168373 http/tests/media/track-in-band-hls-metadata-crash.html [ Timeout ]
    23622362webkit.org/b/168373 media/media-fullscreen-loop-inline.html [ Timeout ]
    2363 webkit.org/b/168373 media/media-source/media-source-error-crash.html [ Timeout ]
    23642363webkit.org/b/168373 media/media-source/only-bcp47-language-tags-accepted-as-valid.html [ Timeout ]
    23652364
  • trunk/LayoutTests/platform/wpe/TestExpectations

    r238874 r238892  
    12141214# ENABLE_MEDIA_SOURCE
    12151215media/media-source/ [ Pass ]
    1216 webkit.org/b/168373 media/media-source/media-source-error-crash.html [ Timeout ]
    12171216webkit.org/b/171726 media/media-source/media-source-init-segment-duration.html [ Failure ]
    12181217webkit.org/b/168373 media/media-source/media-source-resize.html [ Failure ]
  • trunk/Source/WebCore/ChangeLog

    r238891 r238892  
     12018-12-05  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE][GStreamer] Remove the AppendPipeline state machine
     4        https://bugs.webkit.org/show_bug.cgi?id=192204
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        This patch tries to reduce the complexity of the AppendPipeline by
     9        removing the appendState state machine and cleaning all the
     10        conditional code around it that is not necessary anymore.
     11
     12        For the most part the behavior is the same, but some edge cases have
     13        been improved in the process:
     14
     15        Demuxing errors now result in the append being flagged as
     16        ParsingFailed and the error being propagated to the application. This
     17        fixes media/media-source/media-source-error-crash.html (or at least
     18        gets it up to date with cross platform expectations).
     19
     20        AbortableTaskQueue now allows the task handler to perform an abort
     21        safely. This is used in the GstBus error message sync handler, since
     22        it needs to ask the MainThread to raise a parse error, which will in
     23        turn abort. An API test has been added for this new functionality.
     24        Also, code has been added to the API tests to ensure the correct
     25        destruction of the response object, especially in this case.
     26
     27        The code handling invalid track codecs has been made clearer by also
     28        explicitly raising a parse error, but it should not expose behavior
     29        differences for the application. A test has been added for this
     30        behavior: web-platform-tests/media-source/mediasource-invalid-codec.html
     31
     32        The reporting of EOS events have been made more rigorous. EOS is only
     33        expected after a demuxing error, otherwise it's a g_critical.
     34
     35        AppendPipeline::abort() has been renamed to
     36        AppendPipeline::resetParserState() to honor the fact that it's not
     37        only called when the user calls abort() and match better the names
     38        used in the spec.
     39
     40        Test: imported/w3c/web-platform-tests/media-source/mediasource-invalid-codec.html
     41
     42        * platform/AbortableTaskQueue.h:
     43        * platform/graphics/gstreamer/mse/AppendPipeline.cpp:
     44        (WebCore::assertedElementSetState):
     45        (WebCore::AppendPipeline::AppendPipeline):
     46        (WebCore::AppendPipeline::~AppendPipeline):
     47        (WebCore::AppendPipeline::handleErrorSyncMessage):
     48        (WebCore::AppendPipeline::appsrcEndOfAppendCheckerProbe):
     49        (WebCore::AppendPipeline::handleNeedContextSyncMessage):
     50        (WebCore::AppendPipeline::appsinkCapsChanged):
     51        (WebCore::AppendPipeline::handleEndOfAppend):
     52        (WebCore::AppendPipeline::appsinkNewSample):
     53        (WebCore::AppendPipeline::didReceiveInitializationSegment):
     54        (WebCore::AppendPipeline::resetParserState):
     55        (WebCore::AppendPipeline::pushNewBuffer):
     56        (WebCore::AppendPipeline::handleAppsinkNewSampleFromStreamingThread):
     57        (WebCore::AppendPipeline::connectDemuxerSrcPadToAppsinkFromStreamingThread):
     58        (WebCore::AppendPipeline::connectDemuxerSrcPadToAppsink):
     59        (WebCore::AppendPipeline::disconnectDemuxerSrcPadFromAppsinkFromAnyThread):
     60        (WebCore::AppendPipeline::dumpAppendState): Deleted.
     61        (WebCore::AppendPipeline::demuxerNoMorePads): Deleted.
     62        (WebCore::AppendPipeline::setAppendState): Deleted.
     63        (WebCore::AppendPipeline::appsinkEOS): Deleted.
     64        (WebCore::AppendPipeline::resetPipeline): Deleted.
     65        (WebCore::AppendPipeline::abort): Deleted.
     66        * platform/graphics/gstreamer/mse/AppendPipeline.h:
     67        (WebCore::AppendPipeline::appendState): Deleted.
     68        * platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp:
     69        (WebCore::MediaSourceClientGStreamerMSE::abort):
     70        (WebCore::MediaSourceClientGStreamerMSE::resetParserState):
     71        * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:
     72        (WebCore::SourceBufferPrivateGStreamer::appendParsingFailed):
     73        * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h:
     74
    1752018-12-05  Rob Buis  <rbuis@igalia.com>
    276
  • trunk/Source/WebCore/platform/AbortableTaskQueue.h

    r238084 r238892  
    135135    //
    136136    // If we are aborting, the call finishes immediately, returning an empty optional.
     137    //
     138    // It is allowed for the main thread task handler to abort the AbortableTaskQueue. In that case, the return
     139    // value is discarded and the caller receives an empty optional.
    137140    template<typename R>
    138141    std::optional<R> enqueueTaskAndWait(WTF::Function<R()>&& mainThreadTaskHandler)
     
    149152            R responseValue = mainThreadTaskHandler();
    150153            LockHolder lockHolder(m_mutex);
    151             response = WTFMove(responseValue);
     154            if (!m_aborting)
     155                response = WTFMove(responseValue);
    152156            m_abortedOrResponseSet.notifyAll();
    153157        });
  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp

    r238412 r238892  
    6868}
    6969
    70 const char* AppendPipeline::dumpAppendState(AppendPipeline::AppendState appendState)
    71 {
    72     switch (appendState) {
    73     case AppendPipeline::AppendState::Invalid:
    74         return "Invalid";
    75     case AppendPipeline::AppendState::NotStarted:
    76         return "NotStarted";
    77     case AppendPipeline::AppendState::Ongoing:
    78         return "Ongoing";
    79     case AppendPipeline::AppendState::DataStarve:
    80         return "DataStarve";
    81     case AppendPipeline::AppendState::Sampling:
    82         return "Sampling";
    83     case AppendPipeline::AppendState::LastSample:
    84         return "LastSample";
    85     case AppendPipeline::AppendState::Aborting:
    86         return "Aborting";
    87     default:
    88         return "(unknown)";
    89     }
    90 }
    91 
    9270#if !LOG_DISABLED
    9371static GstPadProbeReturn appendPipelinePadProbeDebugInformation(GstPad*, GstPadProbeInfo*, struct PadProbeInformation*);
     
    10179
    10280static GstPadProbeReturn matroskademuxForceSegmentStartToEqualZero(GstPad*, GstPadProbeInfo*, void*);
     81
     82// Wrapper for gst_element_set_state() that emits a critical if the state change fails or is not synchronous.
     83static void assertedElementSetState(GstElement* element, GstState desiredState)
     84{
     85    GstState oldState;
     86    gst_element_get_state(element, &oldState, nullptr, 0);
     87
     88    GstStateChangeReturn result = gst_element_set_state(element, desiredState);
     89
     90    GstState newState;
     91    gst_element_get_state(element, &newState, nullptr, 0);
     92
     93    if (desiredState != newState || result != GST_STATE_CHANGE_SUCCESS) {
     94        GST_ERROR("AppendPipeline state change failed (returned %d): %" GST_PTR_FORMAT " %d -> %d (expected %d)",
     95            static_cast<int>(result), element, static_cast<int>(oldState), static_cast<int>(newState), static_cast<int>(desiredState));
     96        ASSERT_NOT_REACHED();
     97    }
     98}
    10399
    104100AppendPipeline::AppendPipeline(Ref<MediaSourceClientGStreamerMSE> mediaSourceClient, Ref<SourceBufferPrivateGStreamer> sourceBufferPrivate, MediaPlayerPrivateGStreamerMSE& playerPrivate)
     
    108104    , m_id(0)
    109105    , m_wasBusAlreadyNotifiedOfAvailableSamples(false)
    110     , m_appendState(AppendState::NotStarted)
    111     , m_abortPending(false)
    112106    , m_streamType(Unknown)
    113107{
     
    128122    gst_bus_enable_sync_message_emission(m_bus.get());
    129123
     124    g_signal_connect(m_bus.get(), "sync-message::error", G_CALLBACK(+[](GstBus*, GstMessage* message, AppendPipeline* appendPipeline) {
     125        appendPipeline->handleErrorSyncMessage(message);
     126    }), this);
    130127    g_signal_connect(m_bus.get(), "sync-message::need-context", G_CALLBACK(+[](GstBus*, GstMessage* message, AppendPipeline* appendPipeline) {
    131128        appendPipeline->handleNeedContextSyncMessage(message);
     
    156153    gst_app_sink_set_emit_signals(GST_APP_SINK(m_appsink.get()), TRUE);
    157154    gst_base_sink_set_sync(GST_BASE_SINK(m_appsink.get()), FALSE);
     155    gst_base_sink_set_async_enabled(GST_BASE_SINK(m_appsink.get()), FALSE); // No prerolls, no async state changes.
     156    gst_base_sink_set_drop_out_of_segment(GST_BASE_SINK(m_appsink.get()), FALSE);
    158157    gst_base_sink_set_last_sample_enabled(GST_BASE_SINK(m_appsink.get()), FALSE);
    159158
     
    203202        GST_DEBUG("Posting no-more-pads task to main thread");
    204203        appendPipeline->m_taskQueue.enqueueTask([appendPipeline]() {
    205             appendPipeline->demuxerNoMorePads();
     204            appendPipeline->didReceiveInitializationSegment();
    206205        });
    207206    }), this);
     
    210209    }), this);
    211210    g_signal_connect(m_appsink.get(), "eos", G_CALLBACK(+[](GstElement*, AppendPipeline* appendPipeline) {
    212         ASSERT(!isMainThread());
    213         GST_DEBUG("Posting appsink-eos task to main thread");
    214         appendPipeline->m_taskQueue.enqueueTask([appendPipeline]() {
    215             appendPipeline->appsinkEOS();
    216         });
     211        // basesrc will emit an EOS after it has received a GST_FLOW_ERROR. That's the only case we are expecting.
     212        if (!appendPipeline->m_errorReceived) {
     213            GST_ERROR("Unexpected appsink EOS in AppendPipeline");
     214            ASSERT_NOT_REACHED();
     215        }
    217216    }), this);
    218217
     
    221220    gst_element_link(m_appsrc.get(), m_demux.get());
    222221
    223     gst_element_set_state(m_pipeline.get(), GST_STATE_READY);
    224 };
     222    assertedElementSetState(m_pipeline.get(), GST_STATE_PLAYING);
     223}
    225224
    226225AppendPipeline::~AppendPipeline()
    227226{
    228     GST_TRACE("Destructing AppendPipeline (%p)", this);
    229     ASSERT(isMainThread());
    230 
    231     setAppendState(AppendState::Invalid);
     227    GST_DEBUG_OBJECT(m_pipeline.get(), "Destructing AppendPipeline (%p)", this);
     228    ASSERT(isMainThread());
     229
    232230    // Forget all pending tasks and unblock the streaming thread if it was blocked.
    233231    m_taskQueue.startAborting();
     
    274272}
    275273
     274void AppendPipeline::handleErrorSyncMessage(GstMessage* message)
     275{
     276    ASSERT(!isMainThread());
     277    GST_WARNING_OBJECT(m_pipeline.get(), "Demuxing error: %" GST_PTR_FORMAT, message);
     278    // Notify the main thread that the append has a decode error.
     279    auto response = m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([this]() {
     280        m_errorReceived = true;
     281        // appendParsingFailed() will cause resetParserState() to be called.
     282        m_sourceBufferPrivate->appendParsingFailed();
     283        return AbortableTaskQueue::Void();
     284    });
     285    // The streaming thread has now been unblocked because we are aborting in the main thread.
     286    ASSERT(!response);
     287}
     288
    276289GstPadProbeReturn AppendPipeline::appsrcEndOfAppendCheckerProbe(GstPadProbeInfo* padProbeInfo)
    277290{
     
    281294    GstBuffer* buffer = GST_BUFFER(padProbeInfo->data);
    282295    ASSERT(GST_IS_BUFFER(buffer));
     296
     297    GST_TRACE_OBJECT(m_pipeline.get(), "Buffer entered appsrcEndOfAppendCheckerProbe: %" GST_PTR_FORMAT, buffer);
    283298
    284299    EndOfAppendMeta* endOfAppendMeta = reinterpret_cast<EndOfAppendMeta*>(gst_buffer_get_meta(buffer, s_endOfAppendMetaType));
     
    302317
    303318    // MediaPlayerPrivateGStreamerBase will take care of setting up encryption.
    304     if (m_playerPrivate)
    305         m_playerPrivate->handleSyncMessage(message);
    306 }
    307 
    308 void AppendPipeline::demuxerNoMorePads()
    309 {
    310     GST_TRACE("calling didReceiveInitializationSegment");
    311     didReceiveInitializationSegment();
    312     GST_TRACE("set pipeline to playing");
    313     gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING);
     319    m_playerPrivate->handleSyncMessage(message);
    314320}
    315321
     
    365371}
    366372
    367 void AppendPipeline::setAppendState(AppendState newAppendState)
    368 {
    369     ASSERT(isMainThread());
    370     // Valid transitions:
    371     // NotStarted-->Ongoing-->DataStarve-->NotStarted
    372     //           |         |            `->Aborting-->NotStarted
    373     //           |         `->Sampling-···->Sampling-->LastSample-->NotStarted
    374     //           |                                               `->Aborting-->NotStarted
    375     //           `->Aborting-->NotStarted
    376     AppendState oldAppendState = m_appendState;
    377     AppendState nextAppendState = AppendState::Invalid;
    378 
    379     if (oldAppendState != newAppendState)
    380         GST_TRACE("%s --> %s", dumpAppendState(oldAppendState), dumpAppendState(newAppendState));
    381 
    382     bool ok = false;
    383 
    384     switch (oldAppendState) {
    385     case AppendState::NotStarted:
    386         switch (newAppendState) {
    387         case AppendState::Ongoing:
    388             ok = true;
    389             gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING);
    390             break;
    391         case AppendState::NotStarted:
    392             ok = true;
    393             if (m_pendingBuffer) {
    394                 GST_TRACE("pushing pending buffer %" GST_PTR_FORMAT, m_pendingBuffer.get());
    395                 gst_app_src_push_buffer(GST_APP_SRC(appsrc()), m_pendingBuffer.leakRef());
    396                 nextAppendState = AppendState::Ongoing;
    397             }
    398             break;
    399         case AppendState::Aborting:
    400             ok = true;
    401             nextAppendState = AppendState::NotStarted;
    402             break;
    403         case AppendState::Invalid:
    404             ok = true;
    405             break;
    406         default:
    407             break;
    408         }
    409         break;
    410     case AppendState::Ongoing:
    411         switch (newAppendState) {
    412         case AppendState::Sampling:
    413         case AppendState::Invalid:
    414             ok = true;
    415             break;
    416         case AppendState::DataStarve:
    417             ok = true;
    418             GST_DEBUG("received all pending samples");
    419             m_sourceBufferPrivate->didReceiveAllPendingSamples();
    420             if (m_abortPending)
    421                 nextAppendState = AppendState::Aborting;
    422             else
    423                 nextAppendState = AppendState::NotStarted;
    424             break;
    425         default:
    426             break;
    427         }
    428         break;
    429     case AppendState::DataStarve:
    430         switch (newAppendState) {
    431         case AppendState::NotStarted:
    432         case AppendState::Invalid:
    433             ok = true;
    434             break;
    435         case AppendState::Aborting:
    436             ok = true;
    437             nextAppendState = AppendState::NotStarted;
    438             break;
    439         default:
    440             break;
    441         }
    442         break;
    443     case AppendState::Sampling:
    444         switch (newAppendState) {
    445         case AppendState::Sampling:
    446         case AppendState::Invalid:
    447             ok = true;
    448             break;
    449         case AppendState::LastSample:
    450             ok = true;
    451             GST_DEBUG("received all pending samples");
    452             m_sourceBufferPrivate->didReceiveAllPendingSamples();
    453             if (m_abortPending)
    454                 nextAppendState = AppendState::Aborting;
    455             else
    456                 nextAppendState = AppendState::NotStarted;
    457             break;
    458         default:
    459             break;
    460         }
    461         break;
    462     case AppendState::LastSample:
    463         switch (newAppendState) {
    464         case AppendState::NotStarted:
    465         case AppendState::Invalid:
    466             ok = true;
    467             break;
    468         case AppendState::Aborting:
    469             ok = true;
    470             nextAppendState = AppendState::NotStarted;
    471             break;
    472         default:
    473             break;
    474         }
    475         break;
    476     case AppendState::Aborting:
    477         switch (newAppendState) {
    478         case AppendState::NotStarted:
    479             ok = true;
    480             resetPipeline();
    481             m_abortPending = false;
    482             nextAppendState = AppendState::NotStarted;
    483             break;
    484         case AppendState::Invalid:
    485             ok = true;
    486             break;
    487         default:
    488             break;
    489         }
    490         break;
    491     case AppendState::Invalid:
    492         ok = true;
    493         break;
    494     }
    495 
    496     if (ok)
    497         m_appendState = newAppendState;
    498     else
    499         GST_ERROR("Invalid append state transition %s --> %s", dumpAppendState(oldAppendState), dumpAppendState(newAppendState));
    500 
    501     ASSERT(ok);
    502 
    503     if (nextAppendState != AppendState::Invalid)
    504         setAppendState(nextAppendState);
    505 }
    506 
    507373void AppendPipeline::parseDemuxerSrcPadCaps(GstCaps* demuxerSrcPadCaps)
    508374{
     
    537403    ASSERT(isMainThread());
    538404
    539     if (!m_appsink)
    540         return;
    541 
    542405    GRefPtr<GstPad> pad = adoptGRef(gst_element_get_static_pad(m_appsink.get(), "sink"));
    543406    GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(pad.get()));
     
    551414    if (m_appsinkCaps != caps) {
    552415        m_appsinkCaps = WTFMove(caps);
    553         if (m_playerPrivate)
    554             m_playerPrivate->trackDetected(this, m_track, previousCapsWereNull);
    555         gst_element_set_state(m_pipeline.get(), GST_STATE_PLAYING);
     416        m_playerPrivate->trackDetected(this, m_track, previousCapsWereNull);
    556417    }
    557418}
     
    560421{
    561422    ASSERT(isMainThread());
    562     GST_TRACE_OBJECT(m_pipeline.get(), "received end-of-append");
    563 
    564     // Regardless of the state transition, the result is the same: didReceiveAllPendingSamples() is called.
    565     switch (m_appendState) {
    566     case AppendState::Ongoing:
    567         GST_TRACE("DataStarve");
    568         setAppendState(AppendState::DataStarve);
    569         break;
    570     case AppendState::Sampling:
    571         GST_TRACE("LastSample");
    572         setAppendState(AppendState::LastSample);
    573         break;
    574     default:
    575         ASSERT_NOT_REACHED();
    576         break;
    577     }
     423    consumeAppsinkAvailableSamples();
     424    GST_TRACE_OBJECT(m_pipeline.get(), "Notifying SourceBufferPrivate the append is complete");
     425    sourceBufferPrivate()->didReceiveAllPendingSamples();
    578426}
    579427
     
    596444        mediaSample->presentationSize().width(), mediaSample->presentationSize().height());
    597445
    598     // If we're beyond the duration, ignore this sample and the remaining ones.
     446    // If we're beyond the duration, ignore this sample.
    599447    MediaTime duration = m_mediaSourceClient->duration();
    600448    if (duration.isValid() && !duration.indefiniteTime() && mediaSample->presentationTime() > duration) {
    601         GST_DEBUG("Detected sample (%f) beyond the duration (%f), declaring LastSample", mediaSample->presentationTime().toFloat(), duration.toFloat());
    602         setAppendState(AppendState::LastSample);
     449        GST_DEBUG_OBJECT(m_pipeline.get(), "Detected sample (%s) beyond the duration (%s), discarding", mediaSample->presentationTime().toString().utf8().data(), duration.toString().utf8().data());
    603450        return;
    604451    }
     
    611458
    612459    m_sourceBufferPrivate->didReceiveSample(*mediaSample);
    613     setAppendState(AppendState::Sampling);
    614 }
    615 
    616 void AppendPipeline::appsinkEOS()
    617 {
    618     ASSERT(isMainThread());
    619 
    620     switch (m_appendState) {
    621     case AppendState::Aborting:
    622         // Ignored. Operation completion will be managed by the Aborting->NotStarted transition.
    623         return;
    624     case AppendState::Ongoing:
    625         // Finish Ongoing and Sampling states.
    626         setAppendState(AppendState::DataStarve);
    627         break;
    628     case AppendState::Sampling:
    629         setAppendState(AppendState::LastSample);
    630         break;
    631     default:
    632         GST_DEBUG("Unexpected EOS");
    633         break;
    634     }
    635460}
    636461
     
    691516}
    692517
    693 void AppendPipeline::resetPipeline()
    694 {
    695     ASSERT(isMainThread());
    696     GST_DEBUG("resetting pipeline");
    697 
    698     gst_element_set_state(m_pipeline.get(), GST_STATE_READY);
    699     gst_element_get_state(m_pipeline.get(), nullptr, nullptr, 0);
     518void AppendPipeline::resetParserState()
     519{
     520    ASSERT(isMainThread());
     521    GST_DEBUG_OBJECT(m_pipeline.get(), "Handling resetParserState() in AppendPipeline by resetting the pipeline");
     522
     523    // FIXME: Implement a flush event-based resetParserState() implementation would allow the initialization segment to
     524    // survive, in accordance with the spec.
     525
     526    // This function restores the GStreamer pipeline to the same state it was when the AppendPipeline constructor
     527    // finished. All previously enqueued data is lost and the demuxer is reset, losing all pads and track data.
     528
     529    // Unlock the streaming thread.
     530    m_taskQueue.startAborting();
     531
     532    // Reset the state of all elements in the pipeline.
     533    assertedElementSetState(m_pipeline.get(), GST_STATE_READY);
     534
     535    // The parser is tear down automatically when the demuxer is reset (see disconnectDemuxerSrcPadFromAppsinkFromAnyThread()).
     536    ASSERT(!m_parser);
     537
     538    // Set the pipeline to PLAYING so that it can be used again.
     539    assertedElementSetState(m_pipeline.get(), GST_STATE_PLAYING);
     540
     541    // All processing related to the previous append has been aborted and the pipeline is idle.
     542    // We can listen again to new requests coming from the streaming thread.
     543    m_taskQueue.finishAborting();
    700544
    701545#if (!(LOG_DISABLED || defined(GST_DISABLE_GST_DEBUG)))
     
    703547        static unsigned i = 0;
    704548        // This is here for debugging purposes. It does not make sense to have it as class member.
    705         WTF::String  dotFileName = String::format("reset-pipeline-%d", ++i);
     549        WTF::String dotFileName = String::format("reset-pipeline-%d", ++i);
    706550        gst_debug_bin_to_dot_file(GST_BIN(m_pipeline.get()), GST_DEBUG_GRAPH_SHOW_ALL, dotFileName.utf8().data());
    707551    }
    708552#endif
    709 
    710 }
    711 
    712 void AppendPipeline::abort()
    713 {
    714     ASSERT(isMainThread());
    715     GST_DEBUG("aborting");
    716 
    717     m_pendingBuffer = nullptr;
    718 
    719     // Abort already ongoing.
    720     if (m_abortPending)
    721         return;
    722 
    723     m_abortPending = true;
    724     if (m_appendState == AppendState::NotStarted)
    725         setAppendState(AppendState::Aborting);
    726     // Else, the automatic state transitions will take care when the ongoing append finishes.
    727553}
    728554
    729555GstFlowReturn AppendPipeline::pushNewBuffer(GstBuffer* buffer)
    730556{
    731     if (m_abortPending) {
    732         m_pendingBuffer = adoptGRef(buffer);
    733         return GST_FLOW_OK;
    734     }
    735 
    736     setAppendState(AppendPipeline::AppendState::Ongoing);
    737 
    738557    GST_TRACE_OBJECT(m_pipeline.get(), "pushing data buffer %" GST_PTR_FORMAT, buffer);
    739558    GstFlowReturn pushDataBufferRet = gst_app_src_push_buffer(GST_APP_SRC(m_appsrc.get()), buffer);
     
    771590        // AppendPipeline should have only one streaming thread. Otherwise we can't detect reliably when an appends has
    772591        // been demuxed completely.;
    773         g_critical("Appsink received a sample in a different thread than appsrcEndOfAppendCheckerProbe run.");
     592        GST_ERROR_OBJECT(m_pipeline.get(), "Appsink received a sample in a different thread than appsrcEndOfAppendCheckerProbe run.");
    774593        ASSERT_NOT_REACHED();
    775     }
    776 
    777     if (!m_playerPrivate || m_appendState == AppendState::Invalid) {
    778         GST_WARNING("AppendPipeline has been disabled, ignoring this sample");
    779         return GST_FLOW_ERROR;
    780594    }
    781595
     
    826640{
    827641    ASSERT(!isMainThread());
    828     if (!m_appsink)
    829         return;
    830642
    831643    GST_DEBUG("connecting to appsink");
     
    866678
    867679    if (isData) {
    868         // FIXME: Only add appsink one time. This method can be called several times.
    869680        GRefPtr<GstObject> parent = adoptGRef(gst_element_get_parent(m_appsink.get()));
    870681        if (!parent)
     
    893704        gst_element_sync_state_with_parent(m_appsink.get());
    894705
    895         gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED);
    896         gst_element_sync_state_with_parent(m_appsink.get());
    897 
    898706        GST_DEBUG_BIN_TO_DOT_FILE_WITH_TS(GST_BIN(m_pipeline.get()), GST_DEBUG_GRAPH_SHOW_ALL, "webkit-after-link");
    899707    }
     
    915723
    916724    GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(GST_PAD(demuxerSrcPad)));
    917 
    918     if (!caps || m_appendState == AppendState::Invalid || !m_playerPrivate) {
    919         return;
    920     }
    921725
    922726#ifndef GST_DISABLE_GST_DEBUG
     
    934738    switch (m_streamType) {
    935739    case WebCore::MediaSourceStreamTypeGStreamer::Audio:
    936         if (m_playerPrivate)
    937             m_track = WebCore::AudioTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get());
     740        m_track = WebCore::AudioTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get());
    938741        break;
    939742    case WebCore::MediaSourceStreamTypeGStreamer::Video:
    940         if (m_playerPrivate)
    941             m_track = WebCore::VideoTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get());
     743        m_track = WebCore::VideoTrackPrivateGStreamer::create(makeWeakPtr(*m_playerPrivate), id(), sinkSinkPad.get());
    942744        break;
    943745    case WebCore::MediaSourceStreamTypeGStreamer::Text:
     
    945747        break;
    946748    case WebCore::MediaSourceStreamTypeGStreamer::Invalid:
    947         {
    948             GUniquePtr<gchar> strcaps(gst_caps_to_string(caps.get()));
    949             GST_DEBUG("Unsupported track codec: %s", strcaps.get());
    950         }
    951         // This is going to cause an error which will detach the SourceBuffer and tear down this
    952         // AppendPipeline, so we need the padAddRemove lock released before continuing.
    953         m_track = nullptr;
    954         didReceiveInitializationSegment();
     749        GST_WARNING_OBJECT(m_pipeline.get(), "Unsupported track codec: %" GST_PTR_FORMAT, caps.get());
     750        // 3.5.7 Initialization Segment Received
     751        // 5.1. If the initialization segment contains tracks with codecs the user agent does not support, then run the
     752        // append error algorithm and abort these steps.
     753
     754        // appendParsingFailed() will immediately cause a resetParserState() which will stop demuxing, then the
     755        // AppendPipeline will be destroyed.
     756        m_sourceBufferPrivate->appendParsingFailed();
    955757        return;
    956758    default:
    957         // No useful data.
     759        GST_WARNING_OBJECT(m_pipeline.get(), "Pad has unknown track type, ignoring: %" GST_PTR_FORMAT, caps.get());
    958760        break;
    959761    }
    960762
    961763    m_appsinkCaps = WTFMove(caps);
    962     if (m_playerPrivate)
    963         m_playerPrivate->trackDetected(this, m_track, true);
     764    m_playerPrivate->trackDetected(this, m_track, true);
    964765}
    965766
     
    974775
    975776    if (m_parser) {
    976         gst_element_set_state(m_parser.get(), GST_STATE_NULL);
     777        assertedElementSetState(m_parser.get(), GST_STATE_NULL);
    977778        gst_bin_remove(GST_BIN(m_pipeline.get()), m_parser.get());
    978779        m_parser = nullptr;
  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h

    r238412 r238892  
    5151
    5252    GstFlowReturn pushNewBuffer(GstBuffer*);
    53     void abort();
     53    void resetParserState();
    5454    Ref<SourceBufferPrivateGStreamer> sourceBufferPrivate() { return m_sourceBufferPrivate.get(); }
    5555    GstCaps* appsinkCaps() { return m_appsinkCaps.get(); }
     
    5858
    5959private:
    60     enum class AppendState { Invalid, NotStarted, Ongoing, DataStarve, Sampling, LastSample, Aborting };
    6160
     61    void handleErrorSyncMessage(GstMessage*);
    6262    void handleNeedContextSyncMessage(GstMessage*);
     63    // For debug purposes only:
    6364    void handleStateChangeMessage(GstMessage*);
    6465
    6566    gint id();
    66     AppendState appendState() { return m_appendState; }
    67     void setAppendState(AppendState);
    6867
    6968    GstFlowReturn handleAppsinkNewSampleFromStreamingThread(GstElement*);
     
    7372    void appsinkCapsChanged();
    7473    void appsinkNewSample(GRefPtr<GstSample>&&);
    75     void appsinkEOS();
    7674    void handleEndOfAppend();
    7775    void didReceiveInitializationSegment();
     
    9088
    9189    void resetPipeline();
    92     void checkEndOfAppend();
    93     void demuxerNoMorePads();
    9490
    9591    void consumeAppsinkAvailableSamples();
     
    9894
    9995    static void staticInitialization();
    100     static const char* dumpAppendState(AppendPipeline::AppendState);
    10196
    10297    static std::once_flag s_staticInitializationFlag;
     
    107102    // Only the pointers are compared.
    108103    WTF::Thread* m_streamingThread;
     104
     105    // Used only for asserting EOS events are only caused by demuxing errors.
     106    bool m_errorReceived { false };
    109107
    110108    Ref<MediaSourceClientGStreamerMSE> m_mediaSourceClient;
     
    144142    struct PadProbeInformation m_appsinkPadEventProbeInformation;
    145143#endif
    146     // Keeps track of the states of append processing, to avoid performing actions inappropriate for the current state
    147     // (eg: processing more samples when the last one has been detected, etc.). See setAppendState() for valid
    148     // transitions.
    149     AppendState m_appendState;
    150 
    151     // Aborts can only be completed when the normal sample detection has finished. Meanwhile, the willing to abort is
    152     // expressed in this field.
    153     bool m_abortPending;
    154144
    155145    WebCore::MediaSourceStreamTypeGStreamer m_streamType;
  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaSourceClientGStreamerMSE.cpp

    r238412 r238892  
    107107    ASSERT(appendPipeline);
    108108
    109     appendPipeline->abort();
     109    appendPipeline->resetParserState();
    110110}
    111111
     
    123123    ASSERT(appendPipeline);
    124124
    125     appendPipeline->abort();
     125    appendPipeline->resetParserState();
    126126}
    127127
  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp

    r230909 r238892  
    179179}
    180180
     181void SourceBufferPrivateGStreamer::appendParsingFailed()
     182{
     183    if (m_sourceBufferPrivateClient)
     184        m_sourceBufferPrivateClient->sourceBufferPrivateAppendComplete(SourceBufferPrivateClient::ParsingFailed);
     185}
     186
    181187}
    182188#endif
  • trunk/Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h

    r233248 r238892  
    7777    void didReceiveSample(MediaSample&);
    7878    void didReceiveAllPendingSamples();
     79    void appendParsingFailed();
    7980
    8081    ContentType type() const { return m_type; }
  • trunk/Tools/ChangeLog

    r238868 r238892  
     12018-12-05  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE][GStreamer] Remove the AppendPipeline state machine
     4        https://bugs.webkit.org/show_bug.cgi?id=192204
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        Updated AbortableTaskQueue tests:
     9
     10        Added test: AbortedBySyncTaskHandler.
     11
     12        Renamed test: AbortDuringSyncTask -> AbortBeforeSyncTaskRun (in
     13        order to avoid confusion with the new test).
     14
     15        Added checks for the correct destruction of response objects.
     16
     17        * TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp:
     18        (TestWebKitAPI::FancyResponse::FancyResponse):
     19        (TestWebKitAPI::FancyResponse::~FancyResponse):
     20        (TestWebKitAPI::TEST):
     21
    1222018-12-04  Chris Dumez  <cdumez@apple.com>
    223
  • trunk/Tools/TestWebKitAPI/Tests/WebCore/AbortableTaskQueue.cpp

    r238084 r238892  
    6666    WTF_MAKE_NONCOPYABLE(FancyResponse);
    6767public:
    68     FancyResponse(int fancyInt)
     68    FancyResponse(int fancyInt, bool* destructedFlagPointer = nullptr)
    6969        : fancyInt(fancyInt)
     70        , destructedFlagPointer(destructedFlagPointer)
    7071    { }
    7172
    72     FancyResponse(FancyResponse&& a)
    73         : fancyInt(a.fancyInt)
    74     { }
     73    FancyResponse(FancyResponse&& original)
     74        : fancyInt(original.fancyInt)
     75        , destructedFlagPointer(original.destructedFlagPointer)
     76    {
     77        original.destructedFlagPointer = nullptr;
     78    }
     79
     80    ~FancyResponse()
     81    {
     82        if (destructedFlagPointer) {
     83            RELEASE_ASSERT(!*destructedFlagPointer);
     84            *destructedFlagPointer = true;
     85        }
     86    }
    7587
    7688    FancyResponse& operator=(FancyResponse&& a)
     
    8193
    8294    int fancyInt;
     95    bool* destructedFlagPointer;
    8396};
    8497
     
    87100    AbortableTaskQueue taskQueue;
    88101    bool testFinished { false };
     102    bool destructedResponseFlag { false };
    89103    int currentStep { 0 };
    90104    RunLoop::initializeMainRunLoop();
     
    96110            currentStep++;
    97111            EXPECT_EQ(1, currentStep);
    98             FancyResponse returnValue(100);
     112            FancyResponse returnValue(100, &destructedResponseFlag);
    99113            return returnValue;
    100114        });
     
    102116        EXPECT_EQ(2, currentStep);
    103117        EXPECT_TRUE(response);
     118        EXPECT_FALSE(destructedResponseFlag);
    104119        EXPECT_EQ(100, response->fancyInt);
     120        response = std::nullopt;
     121        EXPECT_TRUE(destructedResponseFlag);
    105122        RunLoop::main().dispatch([&]() {
    106123            testFinished = true;
     
    220237}
    221238
    222 TEST(AbortableTaskQueue, AbortDuringSyncTask)
     239TEST(AbortableTaskQueue, AbortBeforeSyncTaskRun)
    223240{
    224241    AbortableTaskQueue taskQueue;
     
    255272}
    256273
     274TEST(AbortableTaskQueue, AbortedBySyncTaskHandler)
     275{
     276    AbortableTaskQueue taskQueue;
     277    bool testFinished { false };
     278    int currentStep { 0 };
     279    bool destructedResponseFlag { false };
     280    RunLoop::initializeMainRunLoop();
     281
     282    auto backgroundThreadFunction = [&]() {
     283        EXPECT_FALSE(isMainThread());
     284        currentStep++;
     285        EXPECT_EQ(1, currentStep);
     286
     287        std::optional<FancyResponse> response = taskQueue.enqueueTaskAndWait<FancyResponse>([&]() -> FancyResponse {
     288            currentStep++;
     289            EXPECT_EQ(2, currentStep);
     290            taskQueue.startAborting();
     291            // This task should not have been able to run under the scheduling of this test.
     292            return FancyResponse(100, &destructedResponseFlag);
     293        });
     294
     295        currentStep++;
     296        EXPECT_EQ(3, currentStep);
     297
     298        // Main thread has called startAborting().
     299        EXPECT_FALSE(response);
     300
     301        // The response object has not been leaked.
     302        EXPECT_TRUE(destructedResponseFlag);
     303
     304        RunLoop::main().dispatch([&]() {
     305            testFinished = true;
     306        });
     307    };
     308    RunLoop::current().dispatch([&, backgroundThreadFunction = WTFMove(backgroundThreadFunction)]() mutable {
     309        EXPECT_TRUE(isMainThread());
     310        WTF::Thread::create("atq-background", WTFMove(backgroundThreadFunction))->detach();
     311    });
     312
     313    Util::run(&testFinished);
     314}
     315
    257316} // namespace TestWebKitAPI
Note: See TracChangeset for help on using the changeset viewer.