Title: [291279] releases/WebKitGTK/webkit-2.36
Revision
291279
Author
carlo...@webkit.org
Date
2022-03-15 03:00:05 -0700 (Tue, 15 Mar 2022)

Log Message

Merge r291111 - REGRESSION(r284711): [GStreamer] Buffering, seek broken on youtube.com
https://bugs.webkit.org/show_bug.cgi?id=233861

Unreviewed, manual revert of 284711.

Patch by Philippe Normand <pnorm...@igalia.com> on 2022-03-10

Source/WebCore:

* Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::currentTimeFudgeFactor):
* platform/graphics/SourceBufferPrivate.h:
(WebCore::SourceBufferPrivate::timeFudgeFactor const):
* platform/graphics/gstreamer/GStreamerCommon.h:
(WebCore::toGstClockTime):
* platform/graphics/gstreamer/MediaSampleGStreamer.cpp:
(WebCore::MediaSampleGStreamer::MediaSampleGStreamer):
* platform/graphics/gstreamer/mse/AppendPipeline.cpp:
(WebCore::AppendPipeline::appsinkNewSample):
(WebCore::bufferTimeToStreamTime): Deleted.

LayoutTests:

* platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.36/LayoutTests/ChangeLog (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/LayoutTests/ChangeLog	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/LayoutTests/ChangeLog	2022-03-15 10:00:05 UTC (rev 291279)
@@ -1,3 +1,12 @@
+2022-03-10  Philippe Normand  <pnorm...@igalia.com>
+
+        REGRESSION(r284711): [GStreamer] Buffering, seek broken on youtube.com
+        https://bugs.webkit.org/show_bug.cgi?id=233861
+
+        Unreviewed, manual revert of 284711.
+
+        * platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt:
+
 2022-03-02  Carlos Garcia Campos  <cgar...@igalia.com>
 
         REGRESSION(r216096): [GTK] Test accessibility/gtk/menu-list-unfocused-notifications.html is failing since r216096

Modified: releases/WebKitGTK/webkit-2.36/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt	2022-03-15 10:00:05 UTC (rev 291279)
@@ -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'.
-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) }"
+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) }"
 

Modified: releases/WebKitGTK/webkit-2.36/Source/WebCore/ChangeLog (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/Source/WebCore/ChangeLog	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/Source/WebCore/ChangeLog	2022-03-15 10:00:05 UTC (rev 291279)
@@ -1,3 +1,22 @@
+2022-03-10  Philippe Normand  <pnorm...@igalia.com>
+
+        REGRESSION(r284711): [GStreamer] Buffering, seek broken on youtube.com
+        https://bugs.webkit.org/show_bug.cgi?id=233861
+
+        Unreviewed, manual revert of 284711.
+
+        * Modules/mediasource/MediaSource.cpp:
+        (WebCore::MediaSource::currentTimeFudgeFactor):
+        * platform/graphics/SourceBufferPrivate.h:
+        (WebCore::SourceBufferPrivate::timeFudgeFactor const):
+        * platform/graphics/gstreamer/GStreamerCommon.h:
+        (WebCore::toGstClockTime):
+        * platform/graphics/gstreamer/MediaSampleGStreamer.cpp:
+        (WebCore::MediaSampleGStreamer::MediaSampleGStreamer):
+        * platform/graphics/gstreamer/mse/AppendPipeline.cpp:
+        (WebCore::AppendPipeline::appsinkNewSample):
+        (WebCore::bufferTimeToStreamTime): Deleted.
+
 2022-03-07  Alberto Garcia  <be...@igalia.com>
 
         makeprop.pl breaks reproducible builds

Modified: releases/WebKitGTK/webkit-2.36/Source/WebCore/Modules/mediasource/MediaSource.cpp (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/Source/WebCore/Modules/mediasource/MediaSource.cpp	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/Source/WebCore/Modules/mediasource/MediaSource.cpp	2022-03-15 10:00:05 UTC (rev 291279)
@@ -324,8 +324,8 @@
 
 const MediaTime& MediaSource::currentTimeFudgeFactor()
 {
-    // Allow hasCurrentTime() to be off by as much as 100ms.
-    static NeverDestroyed<MediaTime> fudgeFactor(1, 10);
+    // Allow hasCurrentTime() to be off by as much as the length of two 24fps video frames
+    static NeverDestroyed<MediaTime> fudgeFactor(2002, 24000);
     return fudgeFactor;
 }
 

Modified: releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/SourceBufferPrivate.h (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/SourceBufferPrivate.h	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/SourceBufferPrivate.h	2022-03-15 10:00:05 UTC (rev 291279)
@@ -153,7 +153,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 { 1, 10 }; }
+    virtual MediaTime timeFudgeFactor() const { return { 2002, 24000 }; }
     virtual bool isActive() const { return false; }
     virtual bool isSeeking() const { return false; }
     virtual MediaTime currentMediaTime() const { return { }; }

Modified: releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h	2022-03-15 10:00:05 UTC (rev 291279)
@@ -80,10 +80,6 @@
 
 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: releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp	2022-03-15 10:00:05 UTC (rev 291279)
@@ -49,9 +49,7 @@
     if (metadata)
         buffer = webkitGstBufferSetVideoSampleMetadata(buffer, WTFMove(metadata));
 
-    m_sample = adoptGRef(gst_sample_new(buffer, gst_sample_get_caps(sample.get()), nullptr,
-        gst_sample_get_info(sample.get()) ? gst_structure_copy(gst_sample_get_info(sample.get())) : nullptr));
-
+    m_sample = sample;
     initializeFromBuffer();
 }
 

Modified: releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp (291278 => 291279)


--- releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp	2022-03-15 09:35:36 UTC (rev 291278)
+++ releases/WebKitGTK/webkit-2.36/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp	2022-03-15 10:00:05 UTC (rev 291279)
@@ -374,57 +374,23 @@
     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());
 
-    GstBuffer* buffer = gst_sample_get_buffer(sample.get());
-    if (UNLIKELY(!buffer)) {
+    if (UNLIKELY(!gst_sample_get_buffer(sample.get()))) {
         GST_WARNING("Received sample without buffer from appsink.");
         return;
     }
 
-    if (!GST_BUFFER_PTS_IS_VALID(buffer)) {
+    if (!GST_BUFFER_PTS_IS_VALID(gst_sample_get_buffer(sample.get()))) {
         // 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(),
@@ -432,6 +398,25 @@
         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());
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to