mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2024-12-31 12:38:48 +00:00
Code Issues 1 Projects Releases Wiki Activity

qtdemux: Fix premature EOS when some files are played in push mode

Browse source 
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/2771

This EOS branch exists so that if a seek with a stop is made, qtdemux
stops accepting bytes from the sink after the entire requested playback
range is demuxed, as otherwise we could keep download content that is
not being used.

This patch fixes two flaws that were present in that EOS check:

1) A comparison was made between track time and movie time without conversion.
This made the check trigger early in files with edit lists. This patch fixes
this by converting the track PTS to movie PTS (stream time) for the check.

2) To avoid sending a EOS prematurely when the segment stop is within a GOP and
B-frames are present, the check for EOS should only be done for keyframes. I
gather this was already the intention with the existing code, but because it
used `stream->on_keyframe` instead of the local variable `keyframe` the old
code was checking if the *previous* frame was a keyframe.

It's interesting to note that these two flaws in the old code mask each other
in most cases: the track PTS will have reached the movie end PTS, but EOS would
only be sent if the previous frame was a keyframe. A simple case where they
wouldn't mask each other, reproducing the bug, is a sequence of 3 frame GOPs
with structure I-B-P.

The following validateflow tests have been added to future-proof the
fix:

 * validate.test.mp4.qtdemux_ibpibp_non_frag_pull.default
 * validate.test.mp4.qtdemux_ibpibp_non_frag_push.default

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5021>
This commit is contained in:
Alicia Boya García 2023-07-12 12:37:34 +02:00 committed by GStreamer Marge Bot
parent 8dddb9ad20
commit 5fd3c8a16c
7 changed files with 84 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
subprojects/gst-integration-testsuites/medias

@ -1 +1 @@
Subproject commit 4325f3ae114e8e5d160c34cd02634fb4f36f52db
Subproject commit 121f8909493df214564c3db7fbba1dcb9217348e

2
subprojects/gst-integration-testsuites/testsuites/validate.testslist
View file

@ -940,6 +940,8 @@ validate.test.interlace.interlace_deinterlace
validate.test.interlace.interlace_deinterlace_alternate
validate.test.matroska.demux_flush_within_cluster.default
validate.test.mp4.qtdemux_change_edit_list.default
validate.test.mp4.qtdemux_ibpibp_non_frag_pull.default
validate.test.mp4.qtdemux_ibpibp_non_frag_push.default
validate.test.mp4.qtdemux_reverse_playback_full_gop.reverse_playback_full_gop
validate.test.mp4.redirect.play_15s
validate.test.mse.segment_future_past_mse

11
subprojects/gst-integration-testsuites/testsuites/validate/mp4/qtdemux_ibpibp_non_frag_pull.default.validatetest Normal file
View file

@ -0,0 +1,11 @@
set-globals, media_dir="$(test_dir)/../../../medias/"
meta,
seek=false,
handles-states=false,
args = {
"filesrc location=\"$(media_dir)/edit-lists/simple/ibpibp-non-frag.mp4\" ! qtdemux ! fakesink async=false",
},
configs = {
"$(validateflow), pad=fakesink0:sink, record-buffers=true, logged-event-types={segment}",
"change-issue-severity, issue-id=event::eos-has-wrong-seqnum, new-severity=ignore",
}

7
subprojects/gst-integration-testsuites/testsuites/validate/mp4/qtdemux_ibpibp_non_frag_pull.default/flow-expectations/log-fakesink0-sink-expected Normal file
View file

@ -0,0 +1,7 @@
event segment: format=TIME, start=0:00:00.333333333, offset=0:00:00.000000000, stop=0:00:02.333333333, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.333333333
buffer: dts=0:00:00.000000000, pts=0:00:00.333333333, dur=0:00:00.333333333, flags=discont
buffer: dts=0:00:00.333333333, pts=0:00:01.000000000, dur=0:00:00.333333333, flags=delta-unit
buffer: dts=0:00:00.666666666, pts=0:00:00.666666666, dur=0:00:00.333333334, flags=delta-unit
buffer: dts=0:00:01.000000000, pts=0:00:01.333333333, dur=0:00:00.333333333
buffer: dts=0:00:01.333333333, pts=0:00:02.000000000, dur=0:00:00.333333333, flags=delta-unit
buffer: dts=0:00:01.666666666, pts=0:00:01.666666666, dur=0:00:00.333333334, flags=delta-unit

15
subprojects/gst-integration-testsuites/testsuites/validate/mp4/qtdemux_ibpibp_non_frag_push.default.validatetest Normal file
View file

@ -0,0 +1,15 @@
set-globals, media_dir="$(test_dir)/../../../medias/"
meta,
seek=false,
handles-states=false,
args = {
"appsrc ! qtdemux ! fakesink async=false",
},
configs = {
"$(validateflow), pad=fakesink0:sink, record-buffers=true, logged-event-types={segment, eos}",
"change-issue-severity, issue-id=event::eos-has-wrong-seqnum, new-severity=ignore",
}
# Note that sending the file this way ensures qtdemux doesn't receive an EOS
# from appsrc, so we can test that the EOS comes from qtdemux.
appsrc-push, target-element-name=appsrc0, file-name="$(media_dir)/edit-lists/simple/ibpibp-non-frag.mp4"

8
subprojects/gst-integration-testsuites/testsuites/validate/mp4/qtdemux_ibpibp_non_frag_push.default/flow-expectations/log-fakesink0-sink-expected Normal file
View file

@ -0,0 +1,8 @@
event segment: format=TIME, start=0:00:00.333333333, offset=0:00:00.000000000, stop=0:00:02.333333333, time=0:00:00.000000000, base=0:00:00.000000000, position=0:00:00.333333333
buffer: dts=0:00:00.000000000, pts=0:00:00.333333333, dur=0:00:00.333333333, flags=discont tag-memory
buffer: dts=0:00:00.333333333, pts=0:00:01.000000000, dur=0:00:00.333333333, flags=delta-unit tag-memory
buffer: dts=0:00:00.666666666, pts=0:00:00.666666666, dur=0:00:00.333333334, flags=delta-unit tag-memory
buffer: dts=0:00:01.000000000, pts=0:00:01.333333333, dur=0:00:00.333333333, flags=tag-memory
buffer: dts=0:00:01.333333333, pts=0:00:02.000000000, dur=0:00:00.333333333, flags=delta-unit tag-memory
buffer: dts=0:00:01.666666666, pts=0:00:01.666666666, dur=0:00:00.333333334, flags=delta-unit tag-memory
event eos: (no structure)

42
subprojects/gst-plugins-good/gst/isomp4/qtdemux.c
View file

@ -7882,6 +7882,42 @@ gst_qtdemux_chain (GstPad * sinkpad, GstObject * parent, GstBuffer * inbuf)
return gst_qtdemux_process_adapter (demux, FALSE);
}
static guint64
gst_segment_to_stream_time_clamped (const GstSegment * segment,
guint64 position)
{
guint64 segment_stream_time_start;
guint64 segment_stream_time_stop = GST_CLOCK_TIME_NONE;
guint64 stream_pts_unsigned;
int ret;
g_return_val_if_fail (segment != NULL, GST_CLOCK_TIME_NONE);
g_return_val_if_fail (segment->format == GST_FORMAT_TIME,
GST_CLOCK_TIME_NONE);
segment_stream_time_start = segment->time;
if (segment->stop != GST_CLOCK_TIME_NONE)
segment_stream_time_stop =
gst_segment_to_stream_time (segment, GST_FORMAT_TIME, segment->stop);
ret =
gst_segment_to_stream_time_full (segment, GST_FORMAT_TIME, position,
&stream_pts_unsigned);
/* ret == 0 if the segment is invalid (either position, segment->time or the segment start are -1). */
g_return_val_if_fail (ret != 0, GST_CLOCK_TIME_NONE);
if (ret == -1 || stream_pts_unsigned < segment_stream_time_start) {
/* Negative or prior to segment start stream time, clamp to segment start. */
return segment_stream_time_start;
} else if (segment_stream_time_stop != GST_CLOCK_TIME_NONE
&& stream_pts_unsigned > segment_stream_time_stop) {
/* Clamp to segment end. */
return segment_stream_time_stop;
} else {
return stream_pts_unsigned;
}
}
static GstFlowReturn
gst_qtdemux_process_adapter (GstQTDemux * demux, gboolean force)
{
@ -8290,7 +8326,7 @@ gst_qtdemux_process_adapter (GstQTDemux * demux, gboolean force)
case QTDEMUX_STATE_MOVIE:{
QtDemuxStream *stream = NULL;
QtDemuxSample *sample;
GstClockTime dts, pts, duration;
GstClockTime dts, pts, stream_pts, duration;
gboolean keyframe;
gint i;
@ -8402,12 +8438,14 @@ gst_qtdemux_process_adapter (GstQTDemux * demux, gboolean force)
dts = QTSAMPLE_DTS (stream, sample);
pts = QTSAMPLE_PTS (stream, sample);
stream_pts =
gst_segment_to_stream_time_clamped (&stream->segment, pts);
duration = QTSAMPLE_DUR_DTS (stream, sample, dts);
keyframe = QTSAMPLE_KEYFRAME (stream, sample);
/* check for segment end */
if (G_UNLIKELY (demux->segment.stop != -1
&& demux->segment.stop <= pts && stream->on_keyframe)
&& demux->segment.stop <= stream_pts && keyframe)
&& !(demux->upstream_format_is_time && demux->segment.rate < 0)) {
GST_DEBUG_OBJECT (demux, "we reached the end of our segment.");
stream->time_position = GST_CLOCK_TIME_NONE; /* this means EOS */