Title: [295286] trunk
Revision
295286
Author
[email protected]
Date
2022-06-06 07:34:46 -0700 (Mon, 06 Jun 2022)

Log Message

Revert of revert of [MSE][GStreamer] Honor MP4 edit lists, bis
https://bugs.webkit.org/show_bug.cgi?id=231019

Reviewed by Xabier Rodriguez-Calvar.

This patch reintroduces https://commits.webkit.org/243426@main with
some corrections that avoid the problems detected in
https://bugs.webkit.org/show_bug.cgi?id=233861, which motivated the
original patch revert.

Original author: Alicia Boya Garcia <[email protected]>

Source/WebCore:

This patch takes into consideration the GstSegment attached to a
sample to offset the PTS and DTS. This ensures accurate timestamps are
obtained for MP4 files containing edit lists (commonly necessary for
files containing video with B frames to have PTS starting at zero).

Before this was implemented, a workaround was in place based on a
heuristic (DTS = 0 && PTS > 0 && PTS < 0.1). The workaround is
preserved for the sake of content without proper edit lists, but
any edit list takes preference.

The time fudge factor has been modified from 0.083 seconds up to
0.100 seconds to accomodate the size of the empty edit in test.mp4
used by Web Platform Tests.

This test fixes improves expectation results and fixes two subtests in
imported/w3c/web-platform-tests/media-source/mediasource-remove.html.

This is a reworked version that avoids using gst_sample_set_buffer()
which is not available on GStreamer 1.14, and fixes an issue where
frames that would get a negative DTS were not being enqueued properly.

LayoutTests:

Update expectations for mediasource-remove.html in the GStreamer
ports, as a couple subtests get fixed.

* LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt:
* Source/WebCore/Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::currentTimeFudgeFactor):
* Source/WebCore/platform/graphics/SourceBufferPrivate.h:
(WebCore::SourceBufferPrivate::timeFudgeFactor const):
* Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:
(WebCore::toGstClockTime):
* Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp:
(WebCore::MediaSampleGStreamer::MediaSampleGStreamer):
* Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:
(WebCore::bufferTimeToStreamTime):
(WebCore::AppendPipeline::appsinkNewSample):
(WebCore::matroskademuxForceSegmentStartToEqualZero): Now we reset segment.time in addition to segment.start. This avoids the regressions.

Canonical link: https://commits.webkit.org/251332@main

Modified Paths

Diff

Modified: trunk/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt (295285 => 295286)


--- trunk/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt	2022-06-06 14:18:26 UTC (rev 295285)
+++ trunk/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt	2022-06-06 14:34:46 UTC (rev 295286)
@@ -11,8 +11,8 @@
 PASS Test aborting a remove operation.
 PASS Test remove with a start at the duration.
 PASS Test remove transitioning readyState from 'ended' to 'open'.
-FAIL Test removing all appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
-FAIL Test removing beginning of appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
-FAIL Test removing the middle of appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
-FAIL Test removing the end of appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
+PASS Test removing all appended data.
+PASS Test removing beginning of appended data.
+FAIL Test removing the middle of appended data. assert_equals: Buffered ranges after remove(). expected "{ [0.095, 0.997) [3.298, 6.548) }" but got "{ [0.095, 0.975) [3.298, 6.548) }"
+FAIL Test removing the end of appended data. assert_equals: Buffered ranges after remove(). expected "{ [0.095, 1.022) }" but got "{ [0.095, 0.995) }"
 

Modified: trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp (295285 => 295286)


--- trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp	2022-06-06 14:18:26 UTC (rev 295285)
+++ trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp	2022-06-06 14:34:46 UTC (rev 295286)
@@ -324,8 +324,8 @@
 
 const MediaTime& MediaSource::currentTimeFudgeFactor()
 {
-    // Allow hasCurrentTime() to be off by as much as the length of two 24fps video frames
-    static NeverDestroyed<MediaTime> fudgeFactor(2002, 24000);
+    // Allow hasCurrentTime() to be off by as much as 100ms.
+    static NeverDestroyed<MediaTime> fudgeFactor(1, 10);
     return fudgeFactor;
 }
 

Modified: trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h (295285 => 295286)


--- trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h	2022-06-06 14:18:26 UTC (rev 295285)
+++ trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h	2022-06-06 14:34:46 UTC (rev 295286)
@@ -154,7 +154,7 @@
 protected:
     // The following method should never be called directly and be overridden instead.
     WEBCORE_EXPORT virtual void append(Vector<unsigned char>&&);
-    virtual MediaTime timeFudgeFactor() const { return { 2002, 24000 }; }
+    virtual MediaTime timeFudgeFactor() const { return { 1, 10 }; }
     virtual bool isActive() const { return false; }
     virtual bool isSeeking() const { return false; }
     virtual MediaTime currentMediaTime() const { return { }; }

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h (295285 => 295286)


--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h	2022-06-06 14:18:26 UTC (rev 295285)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h	2022-06-06 14:34:46 UTC (rev 295286)
@@ -89,6 +89,10 @@
 
 inline GstClockTime toGstClockTime(const MediaTime &mediaTime)
 {
+    if (mediaTime.isInvalid())
+        return GST_CLOCK_TIME_NONE;
+    if (mediaTime < MediaTime::zeroTime())
+        return 0;
     return static_cast<GstClockTime>(toGstUnsigned64Time(mediaTime));
 }
 

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp (295285 => 295286)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp	2022-06-06 14:18:26 UTC (rev 295285)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp	2022-06-06 14:34:46 UTC (rev 295286)
@@ -38,8 +38,8 @@
     , m_presentationSize(presentationSize)
 {
     ASSERT(sample);
+    const GstClockTime minimumDuration = 1000; // 1 us
     m_sample = sample;
-    const GstClockTime minimumDuration = 1000; // 1 us
     auto* buffer = gst_sample_get_buffer(m_sample.get());
     RELEASE_ASSERT(buffer);
 
@@ -64,6 +64,8 @@
     }
 
     m_size = gst_buffer_get_size(buffer);
+    m_sample = adoptGRef(gst_sample_new(buffer, gst_sample_get_caps(m_sample.get()), nullptr,
+        gst_sample_get_info(m_sample.get()) ? gst_structure_copy(gst_sample_get_info(m_sample.get())) : nullptr));
 
     if (GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT))
         m_flags = MediaSample::None;

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp (295285 => 295286)


--- trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp	2022-06-06 14:18:26 UTC (rev 295285)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp	2022-06-06 14:34:46 UTC (rev 295286)
@@ -377,23 +377,57 @@
     sourceBufferPrivate().didReceiveAllPendingSamples();
 }
 
+static MediaTime bufferTimeToStreamTime(const GstSegment* segment, GstClockTime bufferTime)
+{
+    if (bufferTime == GST_CLOCK_TIME_NONE)
+        return MediaTime::invalidTime();
+
+    guint64 streamTime;
+    int sign = gst_segment_to_stream_time_full(segment, GST_FORMAT_TIME, bufferTime, &streamTime);
+    if (!sign) {
+        GST_ERROR("Couldn't map buffer time %" GST_TIME_FORMAT " to segment %" GST_PTR_FORMAT, GST_TIME_ARGS(bufferTime), segment);
+        return MediaTime::invalidTime();
+    }
+    return MediaTime(sign * streamTime, GST_SECOND);
+}
+
 void AppendPipeline::appsinkNewSample(const Track& track, GRefPtr<GstSample>&& sample)
 {
     ASSERT(isMainThread());
 
-    if (UNLIKELY(!gst_sample_get_buffer(sample.get()))) {
+    GstBuffer* buffer = gst_sample_get_buffer(sample.get());
+    if (UNLIKELY(!buffer)) {
         GST_WARNING("Received sample without buffer from appsink.");
         return;
     }
 
-    if (!GST_BUFFER_PTS_IS_VALID(gst_sample_get_buffer(sample.get()))) {
+    if (!GST_BUFFER_PTS_IS_VALID(buffer)) {
         // When demuxing Vorbis, matroskademux creates several PTS-less frames with header information. We don't need those.
         GST_DEBUG("Ignoring sample without PTS: %" GST_PTR_FORMAT, gst_sample_get_buffer(sample.get()));
         return;
     }
 
+    GstSegment* segment = gst_sample_get_segment(sample.get());
     auto mediaSample = MediaSampleGStreamer::create(WTFMove(sample), track.presentationSize, track.trackId);
 
+    if (segment && (segment->time || segment->start)) {
+        // MP4 has the concept of edit lists, where some buffer time needs to be offsetted, often very slightly,
+        // to get exact timestamps.
+        MediaTime pts = bufferTimeToStreamTime(segment, GST_BUFFER_PTS(buffer));
+        MediaTime dts = bufferTimeToStreamTime(segment, GST_BUFFER_DTS(buffer));
+        GST_TRACE_OBJECT(track.appsinkPad.get(), "Mapped buffer to segment, PTS %" GST_TIME_FORMAT " -> %s DTS %" GST_TIME_FORMAT " -> %s",
+            GST_TIME_ARGS(GST_BUFFER_PTS(buffer)), pts.toString().utf8().data(), GST_TIME_ARGS(GST_BUFFER_DTS(buffer)), dts.toString().utf8().data());
+        mediaSample->setTimestamps(pts, dts);
+    } else if (!GST_BUFFER_DTS(buffer) && GST_BUFFER_PTS(buffer) > 0 && GST_BUFFER_PTS(buffer) <= 100'000'000) {
+        // 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.
+        // This is relevant for files that should have an edit list but don't, or when using GStreamer < 1.16, where
+        // edit lists are not parsed in push-mode.
+
+        GST_DEBUG("Extending first sample of track '%s' to make it start at PTS=0 %" GST_PTR_FORMAT, track.trackId.string().utf8().data(), buffer);
+        mediaSample->extendToTheBeginning();
+    }
+
     GST_TRACE("append: trackId=%s PTS=%s DTS=%s DUR=%s presentationSize=%.0fx%.0f",
         mediaSample->trackID().string().utf8().data(),
         mediaSample->presentationTime().toString().utf8().data(),
@@ -401,25 +435,6 @@
         mediaSample->duration().toString().utf8().data(),
         mediaSample->presentationSize().width(), mediaSample->presentationSize().height());
 
-    // 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("Extending first sample to make it start at PTS=0");
-        mediaSample->extendToTheBeginning();
-    }
-
     m_sourceBufferPrivate.didReceiveSample(mediaSample.get());
 }
 
@@ -1014,6 +1029,7 @@
         gst_event_copy_segment(event, &segment);
 
         segment.start = 0;
+        segment.time = 0;
 
         GRefPtr<GstEvent> newEvent = adoptGRef(gst_event_new_segment(&segment));
         gst_event_replace(reinterpret_cast<GstEvent**>(&info->data), newEvent.get());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to