Changeset 258844 in webkit

Timestamp:

Mar 23, 2020 8:27:38 AM (4 years ago)

Author:

aboya@igalia.com

Message:

[MSE][GStreamer] Clean and explain first sample PTS hack
​https://bugs.webkit.org/show_bug.cgi?id=209335

Reviewed by Philippe Normand.

MediaSample::applyPtsOffset() had a rather confusing name, so it has
been changed to something more descriptive of its actual function:
extendToTheBeginning().

Also, its only argument has been removed, as it's always zero.

An explanation of the hack has also been added.

This patch introduces no behavior changes.

• platform/graphics/gstreamer/MediaSampleGStreamer.cpp:

(WebCore::MediaSampleGStreamer::extendToTheBeginning):
(WebCore::MediaSampleGStreamer::applyPtsOffset): Deleted.

• platform/graphics/gstreamer/MediaSampleGStreamer.h:
• platform/graphics/gstreamer/mse/AppendPipeline.cpp:

(WebCore::AppendPipeline::appsinkNewSample):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/gstreamer/MediaSampleGStreamer.cpp (modified) (1 diff)
• platform/graphics/gstreamer/MediaSampleGStreamer.h (modified) (1 diff)
• platform/graphics/gstreamer/mse/AppendPipeline.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-03-23  Alicia Boya García  <aboya@igalia.com>
+
+        [MSE][GStreamer] Clean and explain first sample PTS hack
+        https://bugs.webkit.org/show_bug.cgi?id=209335
+
+        Reviewed by Philippe Normand.
+
+        MediaSample::applyPtsOffset() had a rather confusing name, so it has
+        been changed to something more descriptive of its actual function:
+        extendToTheBeginning().
+
+        Also, its only argument has been removed, as it's always zero.
+
+        An explanation of the hack has also been added.
+
+        This patch introduces no behavior changes.
+
+        * platform/graphics/gstreamer/MediaSampleGStreamer.cpp:
+        (WebCore::MediaSampleGStreamer::extendToTheBeginning):
+        (WebCore::MediaSampleGStreamer::applyPtsOffset): Deleted.
+        * platform/graphics/gstreamer/MediaSampleGStreamer.h:
+        * platform/graphics/gstreamer/mse/AppendPipeline.cpp:
+        (WebCore::AppendPipeline::appsinkNewSample):
+
 2020-03-23  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp

@@ -96 +96 @@
 }
 
-void MediaSampleGStreamer::applyPtsOffset(MediaTime timestampOffset)
+void MediaSampleGStreamer::extendToTheBeginning()
 {
-    if (m_pts > timestampOffset) {
-        m_duration = m_duration + (m_pts - timestampOffset);
-        m_pts = timestampOffset;
-    }
+    // Only to be used with the first sample, as a hack for lack of support for edit lists.
+    // See AppendPipeline::appsinkNewSample()
+    ASSERT(m_dts == MediaTime::zeroTime());
+    m_duration += m_pts;
+    m_pts = MediaTime::zeroTime();
 }
 

trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.h

@@ -39 +39 @@
     static Ref<MediaSampleGStreamer> createFakeSample(GstCaps*, MediaTime pts, MediaTime dts, MediaTime duration, const FloatSize& presentationSize, const AtomString& trackId);
 
-    void applyPtsOffset(MediaTime);
+    void extendToTheBeginning();
     MediaTime presentationTime() const override { return m_pts; }
     MediaTime decodeTime() const override { return m_dts; }

trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp

@@ -475 +475 @@
     }
 
-    // Add a gap sample if a gap is detected before the first sample.
+    // Hack, rework when GStreamer >= 1.16 becomes a requirement:
+    // We're not applying edit lists. GStreamer < 1.16 doesn't emit the correct segments to do so.
+    // GStreamer fix in https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/commit/c2a0da8096009f0f99943f78dc18066965be60f9
+    // Also, in order to apply them we would need to convert the timestamps to stream time, which we're not currently
+    // doing for consistency between GStreamer versions.
+    //
+    // In consequence, the timestamps we're handling here are unedited track time. In track time, the first sample is
+    // guaranteed to have DTS == 0, but in the case of streams with B-frames, often PTS > 0. Edit lists fix this by
+    // offsetting all timestamps by that amount in movie time, but we can't do that if we don't have access to them.
+    // (We could assume the track PTS of the sample with track DTS = 0 is the offset, but we don't have any guarantee
+    // we will get appended that sample first, or ever).
+    //
+    // Because a track presentation time starting at some close to zero, but not exactly zero time can cause unexpected
+    // results for applications, we extend the duration of this first sample to the left so that it starts at zero.
     if (mediaSample->decodeTime() == MediaTime::zeroTime() && mediaSample->presentationTime() > MediaTime::zeroTime() && mediaSample->presentationTime() <= MediaTime(1, 10)) {
-        GST_DEBUG("Adding gap offset");
-        mediaSample->applyPtsOffset(MediaTime::zeroTime());
+        GST_DEBUG("Extending first sample to make it start at PTS=0");
+        mediaSample->extendToTheBeginning();
     }