Attachment 340628 Details for Bug 185740 – Patch

[patch] Patch

bug-185740-20180517215131.patch (text/plain), 4.65 KB, created by Alicia Boya García on 2018-05-17 12:51:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alicia Boya García

Created: 2018-05-17 12:51:33 PDT

Size: 4.65 KB

patch

obsolete

>Subversion Revision: 231892
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6548aa877dfbe7195b09e90351d96a377b41b460..9a6bfd5bd622b985c91463806ff449c0d45d0d72 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2018-05-17  Alicia Boya GarcÃa  <aboya@igalia.com>
>+
>+        [MSE][GStreamer] Force segment.start = 0 after matroskademux
>+        https://bugs.webkit.org/show_bug.cgi?id=185740
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch ensures that when WebM MSE media segments are appended in
>+        an out of order fashion their frames are not discarded by opusparse or
>+        any other potential elements downstream in the AppendPipeline that
>+        perform segment clipping.
>+
>+        This patch fixes the following YTTV 2018 tests:
>+
>+        38. OpusAudioWithOverlap
>+        39. OpusAudioWithSmallGap
>+        40. OpusAudioWithLargeGap
>+        70. VP9VideoWithOverlap
>+        71. VP9VideoWithSmallGap
>+
>+        This patch is necessary, but not sufficient for fixing the following
>+        YTTV 2018 tests:
>+
>+        36. AppendOpusAudioOutOfOrder
>+        67. AppendVP9VideoOutOfOrder
>+
>+        * platform/graphics/gstreamer/mse/AppendPipeline.cpp:
>+        (WebCore::AppendPipeline::connectDemuxerSrcPadToAppsink):
>+        (WebCore::matroskademuxForceSegmentStartToEqualZero):
>+
> 2018-05-17  Alicia Boya GarcÃa  <aboya@igalia.com>
> 
>         [MSE][GStreamer] Stream::decodebinSinkPad is read but never assigned
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
>index 9e586f4c1380a18d8d02acaabac7b08b644cd955..e213d2de41cd21d92197fd43d51983dbc16bce33 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
>@@ -85,6 +85,8 @@ static GstPadProbeReturn appendPipelineDemuxerBlackHolePadProbe(GstPad*, GstPadP
> static GstFlowReturn appendPipelineAppsinkNewSample(GstElement*, AppendPipeline*);
> static void appendPipelineAppsinkEOS(GstElement*, AppendPipeline*);
> 
>+static GstPadProbeReturn matroskademuxForceSegmentStartToEqualZero(GstPad*, GstPadProbeInfo*, void*);
>+
> static void appendPipelineNeedContextMessageCallback(GstBus*, GstMessage* message, AppendPipeline* appendPipeline)
> {
>     GST_TRACE("received callback");
>@@ -978,6 +980,10 @@ void AppendPipeline::connectDemuxerSrcPadToAppsink(GstPad* demuxerSrcPad)
>     ASSERT(WTF::isMainThread());
>     GST_DEBUG("Connecting to appsink");
> 
>+    const String& type = m_sourceBufferPrivate->type().containerType();
>+    if (type.endsWith("webm"))
>+        gst_pad_add_probe(demuxerSrcPad, GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM, matroskademuxForceSegmentStartToEqualZero, nullptr, nullptr);
>+
>     LockHolder locker(m_padAddRemoveLock);
>     GRefPtr<GstPad> sinkSinkPad = adoptGRef(gst_element_get_static_pad(m_appsink.get(), "sink"));
> 
>@@ -1148,7 +1154,35 @@ static void appendPipelineAppsinkEOS(GstElement*, AppendPipeline* appendPipeline
>     GST_DEBUG("%s main thread", (WTF::isMainThread()) ? "Is" : "Not");
> }
> 
>-
>+static GstPadProbeReturn matroskademuxForceSegmentStartToEqualZero(GstPad*, GstPadProbeInfo* info, void*)
>+{
>+    // matroskademux sets GstSegment.start to the PTS of the first frame.
>+    //
>+    // This way in the unlikely case a user made a .mkv or .webm file where a certain portion of the movie is skipped
>+    // (e.g. by concatenating a MSE initialization segment with any MSE media segment other than the first) and opened
>+    // it with a regular player, playback would start immediately. GstSegment.duration is not modified in any case.
>+    //
>+    // Leaving the usefulness of that feature aside, the fact that it uses GstSegment.start is problematic for MSE.
>+    // In MSE is not unusual to process unordered MSE media segments. In this case, a frame may have
>+    // PTS <<< GstSegment.start and be discarded by downstream. This happens for instance in elements derived from
>+    // audiobasefilter, such as opusparse.
>+    //
>+    // This probe remedies the problem by setting GstSegment.start to 0 in all cases, not only when the PTS of the first
>+    // frame is zero.
>+
>+    ASSERT(info->type & GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM);
>+    GstEvent* event = static_cast<GstEvent*>(info->data);
>+    if (event->type == GST_EVENT_SEGMENT) {
>+        GstSegment segment;
>+        gst_event_copy_segment(event, &segment);
>+
>+        segment.start = 0;
>+
>+        GRefPtr<GstEvent> newEvent = adoptGRef(gst_event_new_segment(&segment));
>+        gst_event_replace(reinterpret_cast<GstEvent**>(&info->data), newEvent.get());
>+    }
>+    return GST_PAD_PROBE_OK;
>+}
> 
> } // namespace WebCore.
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185740: 340628