Diff
Modified: trunk/LayoutTests/ChangeLog (284710 => 284711)
--- trunk/LayoutTests/ChangeLog 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/LayoutTests/ChangeLog 2021-10-22 20:22:52 UTC (rev 284711)
@@ -1,3 +1,15 @@
+2021-10-22 Alicia Boya García <[email protected]>
+
+ [MSE][GStreamer] Honor MP4 edit lists, bis
+ https://bugs.webkit.org/show_bug.cgi?id=231019
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ Update expectations for mediasource-remove.html in the GStreamer
+ ports, as a couple subtests get fixed.
+
+ * platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt:
+
2021-10-22 Chris Dumez <[email protected]>
Unreviewed, drop custom iOS expectations for noopener-noreferrer-sizing.window.html.
Modified: trunk/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt (284710 => 284711)
--- trunk/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/LayoutTests/platform/glib/imported/w3c/web-platform-tests/media-source/mediasource-remove-expected.txt 2021-10-22 20:22:52 UTC (rev 284711)
@@ -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/ChangeLog (284710 => 284711)
--- trunk/Source/WebCore/ChangeLog 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/Source/WebCore/ChangeLog 2021-10-22 20:22:52 UTC (rev 284711)
@@ -1,3 +1,43 @@
+2021-10-22 Alicia Boya García <[email protected]>
+
+ [MSE][GStreamer] Honor MP4 edit lists, bis
+ https://bugs.webkit.org/show_bug.cgi?id=231019
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ 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.
+
+ * 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::bufferTimeToStreamTime):
+ (WebCore::AppendPipeline::appsinkNewSample):
+
2021-10-22 Alex Christensen <[email protected]>
Remove release assertion in MediaResourceClient::responseReceived
Modified: trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp (284710 => 284711)
--- trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp 2021-10-22 20:22:52 UTC (rev 284711)
@@ -323,8 +323,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 (284710 => 284711)
--- trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h 2021-10-22 20:22:52 UTC (rev 284711)
@@ -150,7 +150,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 (284710 => 284711)
--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h 2021-10-22 20:22:52 UTC (rev 284711)
@@ -80,6 +80,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 (284710 => 284711)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp 2021-10-22 20:22:52 UTC (rev 284711)
@@ -72,7 +72,8 @@
}
m_size = gst_buffer_get_size(buffer);
- m_sample = sample;
+ 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));
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 (284710 => 284711)
--- trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp 2021-10-22 19:50:43 UTC (rev 284710)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp 2021-10-22 20:22:52 UTC (rev 284711)
@@ -374,23 +374,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(),
@@ -398,25 +432,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());
}
