Skip to content

Commit c6df424

Browse files
committed
[wpe-2.38] 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. (Patch only for wpe-2.38 downstream) (See: #1185) 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 <aboya@igalia.com> 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 git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295286 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 81bc92f commit c6df424

6 files changed

Lines changed: 50 additions & 31 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ PASS Test remove while update pending.
1111
PASS Test aborting a remove operation.
1212
PASS Test remove with a start at the duration.
1313
PASS Test remove transitioning readyState from 'ended' to 'open'.
14-
FAIL Test removing all appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
15-
FAIL Test removing beginning of appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
16-
FAIL Test removing the middle of appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
17-
FAIL Test removing the end of appended data. assert_equals: Initial buffered range. expected "{ [0.095, 6.548) }" but got "{ [0.000, 6.548) }"
14+
PASS Test removing all appended data.
15+
PASS Test removing beginning of appended data.
16+
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) }"
17+
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) }"
1818

Source/WebCore/Modules/mediasource/MediaSource.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,8 @@ ExceptionOr<void> MediaSource::clearLiveSeekableRange()
324324

325325
const MediaTime& MediaSource::currentTimeFudgeFactor()
326326
{
327-
// Allow hasCurrentTime() to be off by as much as the length of two 24fps video frames
328-
static NeverDestroyed<MediaTime> fudgeFactor(2002, 24000);
327+
// Allow hasCurrentTime() to be off by as much as 100ms.
328+
static NeverDestroyed<MediaTime> fudgeFactor(1, 10);
329329
return fudgeFactor;
330330
}
331331

Source/WebCore/platform/graphics/SourceBufferPrivate.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ class SourceBufferPrivate
133133
protected:
134134
// The following method should never be called directly and be overridden instead.
135135
WEBCORE_EXPORT virtual void append(Vector<unsigned char>&&);
136-
virtual MediaTime timeFudgeFactor() const { return { 2002, 24000 }; }
136+
virtual MediaTime timeFudgeFactor() const { return { 1, 10 }; }
137137
virtual bool isActive() const { return false; }
138138
virtual bool isSeeking() const { return false; }
139139
virtual MediaTime currentMediaTime() const { return { }; }

Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ uint64_t toGstUnsigned64Time(const MediaTime&);
8888

8989
inline GstClockTime toGstClockTime(const MediaTime& mediaTime)
9090
{
91+
if (mediaTime.isInvalid())
92+
return GST_CLOCK_TIME_NONE;
93+
if (mediaTime < MediaTime::zeroTime())
94+
return 0;
9195
return static_cast<GstClockTime>(toGstUnsigned64Time(mediaTime));
9296
}
9397

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ MediaSampleGStreamer::MediaSampleGStreamer(GRefPtr<GstSample>&& sample, const Fl
3838
, m_presentationSize(presentationSize)
3939
{
4040
ASSERT(sample);
41-
m_sample = sample;
4241
const GstClockTime minimumDuration = 1000; // 1 us
42+
m_sample = sample;
4343
auto* buffer = gst_sample_get_buffer(m_sample.get());
4444
RELEASE_ASSERT(buffer);
4545

@@ -64,6 +64,8 @@ MediaSampleGStreamer::MediaSampleGStreamer(GRefPtr<GstSample>&& sample, const Fl
6464
}
6565

6666
m_size = gst_buffer_get_size(buffer);
67+
m_sample = adoptGRef(gst_sample_new(buffer, gst_sample_get_caps(m_sample.get()), nullptr,
68+
gst_sample_get_info(m_sample.get()) ? gst_structure_copy(gst_sample_get_info(m_sample.get())) : nullptr));
6769

6870
if (GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT))
6971
m_flags = MediaSample::None;

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

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -379,52 +379,64 @@ void AppendPipeline::handleEndOfAppend()
379379
sourceBufferPrivate().didReceiveAllPendingSamples();
380380
}
381381

382+
static MediaTime bufferTimeToStreamTime(const GstSegment* segment, GstClockTime bufferTime)
383+
{
384+
if (bufferTime == GST_CLOCK_TIME_NONE)
385+
return MediaTime::invalidTime();
386+
387+
guint64 streamTime;
388+
int sign = gst_segment_to_stream_time_full(segment, GST_FORMAT_TIME, bufferTime, &streamTime);
389+
if (!sign) {
390+
GST_ERROR("Couldn't map buffer time %" GST_TIME_FORMAT " to segment %" GST_PTR_FORMAT, GST_TIME_ARGS(bufferTime), segment);
391+
return MediaTime::invalidTime();
392+
}
393+
return MediaTime(sign * streamTime, GST_SECOND);
394+
}
395+
382396
void AppendPipeline::appsinkNewSample(const Track& track, GRefPtr<GstSample>&& sample)
383397
{
384398
ASSERT(isMainThread());
385399

386-
if (UNLIKELY(!gst_sample_get_buffer(sample.get()))) {
400+
auto* buffer = gst_sample_get_buffer(sample.get());
401+
if (UNLIKELY(!buffer)) {
387402
GST_WARNING_OBJECT(pipeline(), "Received sample without buffer from appsink.");
388403
return;
389404
}
390405

391-
auto* buffer = gst_sample_get_buffer(sample.get());
392406
if (!GST_BUFFER_PTS_IS_VALID(buffer)) {
393407
// When demuxing Vorbis, matroskademux creates several PTS-less frames with header information. We don't need those.
394408
GST_DEBUG_OBJECT(pipeline(), "Ignoring sample without PTS: %" GST_PTR_FORMAT, buffer);
395409
return;
396410
}
397411

412+
GstSegment* segment = gst_sample_get_segment(sample.get());
398413
auto mediaSample = MediaSampleGStreamer::create(WTFMove(sample), track.presentationSize, track.trackId);
399414

415+
if (segment && (segment->time || segment->start)) {
416+
// MP4 has the concept of edit lists, where some buffer time needs to be offsetted, often very slightly,
417+
// to get exact timestamps.
418+
MediaTime pts = bufferTimeToStreamTime(segment, GST_BUFFER_PTS(buffer));
419+
MediaTime dts = bufferTimeToStreamTime(segment, GST_BUFFER_DTS(buffer));
420+
GST_TRACE_OBJECT(track.appsinkPad.get(), "Mapped buffer to segment, PTS %" GST_TIME_FORMAT " -> %s DTS %" GST_TIME_FORMAT " -> %s",
421+
GST_TIME_ARGS(GST_BUFFER_PTS(buffer)), pts.toString().utf8().data(), GST_TIME_ARGS(GST_BUFFER_DTS(buffer)), dts.toString().utf8().data());
422+
mediaSample->setTimestamps(pts, dts);
423+
} else if (!GST_BUFFER_DTS(buffer) && GST_BUFFER_PTS(buffer) > 0 && GST_BUFFER_PTS(buffer) <= 100'000'000) {
424+
// Because a track presentation time starting at some close to zero, but not exactly zero time can cause unexpected
425+
// results for applications, we extend the duration of this first sample to the left so that it starts at zero.
426+
// This is relevant for files that should have an edit list but don't, or when using GStreamer < 1.16, where
427+
// edit lists are not parsed in push-mode.
428+
429+
GST_DEBUG("Extending first sample of track '%s' to make it start at PTS=0 %" GST_PTR_FORMAT, track.trackId.string().utf8().data(), buffer);
430+
mediaSample->extendToTheBeginning();
431+
}
432+
400433
GST_TRACE_OBJECT(pipeline(), "append: trackId=%s PTS=%s DTS=%s DUR=%s presentationSize=%.0fx%.0f",
401434
mediaSample->trackID().string().utf8().data(),
402435
mediaSample->presentationTime().toString().utf8().data(),
403436
mediaSample->decodeTime().toString().utf8().data(),
404437
mediaSample->duration().toString().utf8().data(),
405438
mediaSample->presentationSize().width(), mediaSample->presentationSize().height());
406439

407-
// Hack, rework when GStreamer >= 1.16 becomes a requirement:
408-
// We're not applying edit lists. GStreamer < 1.16 doesn't emit the correct segments to do so.
409-
// GStreamer fix in https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/commit/c2a0da8096009f0f99943f78dc18066965be60f9
410-
// Also, in order to apply them we would need to convert the timestamps to stream time, which we're not currently
411-
// doing for consistency between GStreamer versions.
412-
//
413-
// In consequence, the timestamps we're handling here are unedited track time. In track time, the first sample is
414-
// guaranteed to have DTS == 0, but in the case of streams with B-frames, often PTS > 0. Edit lists fix this by
415-
// offsetting all timestamps by that amount in movie time, but we can't do that if we don't have access to them.
416-
// (We could assume the track PTS of the sample with track DTS = 0 is the offset, but we don't have any guarantee
417-
// we will get appended that sample first, or ever).
418-
//
419-
// Because a track presentation time starting at some close to zero, but not exactly zero time can cause unexpected
420-
// results for applications, we extend the duration of this first sample to the left so that it starts at zero.
421-
if (mediaSample->decodeTime() == MediaTime::zeroTime() && mediaSample->presentationTime() > MediaTime::zeroTime()
422-
&& mediaSample->presentationTime() <= MediaTime(1, 10)
423-
&& mediaSample->isSync()) {
424-
GST_DEBUG_OBJECT(pipeline(), "Extending first sample to make it start at PTS=0");
425-
mediaSample->extendToTheBeginning();
426-
}
427-
428440
m_sourceBufferPrivate.didReceiveSample(mediaSample.get());
429441
}
430442

@@ -1116,6 +1128,7 @@ static GstPadProbeReturn matroskademuxForceSegmentStartToEqualZero(GstPad*, GstP
11161128
gst_event_copy_segment(event, &segment);
11171129

11181130
segment.start = 0;
1131+
segment.time = 0;
11191132

11201133
GRefPtr<GstEvent> newEvent = adoptGRef(gst_event_new_segment(&segment));
11211134
gst_event_replace(reinterpret_cast<GstEvent**>(&info->data), newEvent.get());

0 commit comments

Comments
 (0)