GNOME Bugzilla – Bug 764637
qtdemux: Ensure stream duration by using stts box
Last modified: 2018-11-03 15:08:41 UTC
qtdemux: Ensure stream duration by using stts box Some files have incorrect stream duration value in mdhd box So, we need to double check the stream duration.
Created attachment 325420 [details] [review] qtdemux: Ensure stream duration by using stts box
I found a corrupted mp4 file. The only incorrect factor is stream duration in mdhd. Although the file muxing is incorrect, we can support it by calculating the actual stream duration. - NG case 57 0:00:00.084347775 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:8425:qtdemux_parse_segments:<qtdemux0> created dummy segment 0:04:33.052583333 58 0:00:00.084356373 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:8429:qtdemux_parse_segments:<qtdemux0> using 1 segments 59 0:00:00.084360621 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:10601:qtdemux_parse_trak:<qtdemux0> n_streams is now 2 60 0:00:00.084365309 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:11842:qtdemux_parse_udta:<qtdemux0> No XMP_ node found 61 0:00:00.084390758 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:10772:qtdemux_prepare_streams:<qtdemux0> prepare streams 62 0:00:00.084395791 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:10779:qtdemux_prepare_streams:<qtdemux0> stream 0, id 1, fourcc avc1 63 0:00:00.084401392 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:7875:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 64 0:00:00.084406308 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:8082:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp 0:00:00.000000000 65 0:00:00.084411698 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:8137:qtdemux_parse_samples:<qtdemux0> samples at 0 is keyframe 66 0:00:00.084415646 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:10779:qtdemux_prepare_streams:<qtdemux0> stream 1, id 2, fourcc mp4a 67 0:00:00.084420335 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:7875:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 68 0:00:00.084424276 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:8082:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp 0:00:00.000000000 69 0:00:00.084429254 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:8125:qtdemux_parse_samples:<qtdemux0> all samples are keyframes 70 0:00:00.084432692 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:10831:qtdemux_expose_streams:<qtdemux0> exposing streams 71 0:00:00.084436121 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:10839:qtdemux_expose_streams:<qtdemux0> stream 0, id 1, fourcc avc1 72 0:00:00.084462814 5006 0x7f12780f9de0 DEBUG qtdemux qtdemux.c:7234:gst_qtdemux_configure_stream:<qtdemux0> Calculating framerate, timescale 29970 gave fps_n 10000 fps_d 129 - With this patch 57 0:00:00.031100131 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:8449:qtdemux_parse_segments:<qtdemux0> created dummy segment 0:04:33.052583333 58 0:00:00.031108243 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:8453:qtdemux_parse_segments:<qtdemux0> using 1 segments 59 0:00:00.031112396 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:10625:qtdemux_parse_trak:<qtdemux0> n_streams is now 2 60 0:00:00.031117170 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:11866:qtdemux_parse_udta:<qtdemux0> No XMP_ node found 61 0:00:00.031156892 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:10796:qtdemux_prepare_streams:<qtdemux0> prepare streams 62 0:00:00.031161630 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:10803:qtdemux_prepare_streams:<qtdemux0> stream 0, id 1, fourcc avc1 63 0:00:00.031167805 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:7899:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 64 0:00:00.031172732 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:8106:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp 0:00:00.000000000 65 0:00:00.031188804 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:8161:qtdemux_parse_samples:<qtdemux0> samples at 0 is keyframe 66 0:00:00.031192742 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:10803:qtdemux_prepare_streams:<qtdemux0> stream 1, id 2, fourcc mp4a 67 0:00:00.031197304 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:7899:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 68 0:00:00.031201216 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:8106:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp 0:00:00.000000000 69 0:00:00.031209135 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:8149:qtdemux_parse_samples:<qtdemux0> all samples are keyframes 70 0:00:00.031212720 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:10855:qtdemux_expose_streams:<qtdemux0> exposing streams 71 0:00:00.031216188 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:10863:qtdemux_expose_streams:<qtdemux0> stream 0, id 1, fourcc avc1 72 0:00:00.031240344 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:2850:check_update_duration:<qtdemux0> Updating total duration to 0:11:42.602602602 was 0:11:42.549312500 73 0:00:00.031249534 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:2855:check_update_duration:<qtdemux0> qtdemux->segment.duration: 0:11:42.549312500 .stop: 0:11:42.549312500 74 0:00:00.031254995 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:2862:check_update_duration:<qtdemux0> Updated segment.duration and segment.stop 75 0:00:00.031258571 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:2874:check_update_duration:<qtdemux0> Updating stream #0 duration to 0:11:42.602602602 76 0:00:00.031263416 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:2874:check_update_duration:<qtdemux0> Updating stream #1 duration to 0:11:42.602602602 77 0:00:00.031271809 3575 0x7f95a81171e0 DEBUG qtdemux qtdemux.c:7240:gst_qtdemux_configure_stream:<qtdemux0> Calculating framerate, timescale 29970 gave fps_n 30000 fps_d 1001
(In reply to Seungha Yang from comment #2) > I found a corrupted mp4 file. The only incorrect factor is stream duration > in mdhd. Although the file muxing is incorrect, we can support it by > calculating the actual stream duration. > > - NG case > 57 0:00:00.084347775 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:8425:qtdemux_parse_segments:<qtdemux0> created dummy segment > 0:04:33.052583333 > 58 0:00:00.084356373 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:8429:qtdemux_parse_segments:<qtdemux0> using 1 segments > 59 0:00:00.084360621 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:10601:qtdemux_parse_trak:<qtdemux0> n_streams is now 2 > 60 0:00:00.084365309 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:11842:qtdemux_parse_udta:<qtdemux0> No XMP_ node found > 61 0:00:00.084390758 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:10772:qtdemux_prepare_streams:<qtdemux0> prepare streams > 62 0:00:00.084395791 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:10779:qtdemux_prepare_streams:<qtdemux0> stream 0, id 1, fourcc > avc1 > 63 0:00:00.084401392 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:7875:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 > 64 0:00:00.084406308 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:8082:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp > 0:00:00.000000000 > 65 0:00:00.084411698 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:8137:qtdemux_parse_samples:<qtdemux0> samples at 0 is keyframe > 66 0:00:00.084415646 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:10779:qtdemux_prepare_streams:<qtdemux0> stream 1, id 2, fourcc > mp4a > 67 0:00:00.084420335 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:7875:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 > 68 0:00:00.084424276 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:8082:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp > 0:00:00.000000000 > 69 0:00:00.084429254 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:8125:qtdemux_parse_samples:<qtdemux0> all samples are keyframes > 70 0:00:00.084432692 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:10831:qtdemux_expose_streams:<qtdemux0> exposing streams > 71 0:00:00.084436121 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:10839:qtdemux_expose_streams:<qtdemux0> stream 0, id 1, fourcc avc1 > 72 0:00:00.084462814 5006 0x7f12780f9de0 DEBUG qtdemux > qtdemux.c:7234:gst_qtdemux_configure_stream:<qtdemux0> Calculating > framerate, timescale 29970 gave fps_n 10000 fps_d 129 > > - With this patch > 57 0:00:00.031100131 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:8449:qtdemux_parse_segments:<qtdemux0> created dummy segment > 0:04:33.052583333 > 58 0:00:00.031108243 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:8453:qtdemux_parse_segments:<qtdemux0> using 1 segments > 59 0:00:00.031112396 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:10625:qtdemux_parse_trak:<qtdemux0> n_streams is now 2 > 60 0:00:00.031117170 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:11866:qtdemux_parse_udta:<qtdemux0> No XMP_ node found > 61 0:00:00.031156892 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:10796:qtdemux_prepare_streams:<qtdemux0> prepare streams > 62 0:00:00.031161630 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:10803:qtdemux_prepare_streams:<qtdemux0> stream 0, id 1, fourcc > avc1 > 63 0:00:00.031167805 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:7899:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 > 64 0:00:00.031172732 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:8106:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp > 0:00:00.000000000 > 65 0:00:00.031188804 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:8161:qtdemux_parse_samples:<qtdemux0> samples at 0 is keyframe > 66 0:00:00.031192742 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:10803:qtdemux_prepare_streams:<qtdemux0> stream 1, id 2, fourcc > mp4a > 67 0:00:00.031197304 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:7899:qtdemux_parse_samples:<qtdemux0> parsing up to sample 0 > 68 0:00:00.031201216 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:8106:qtdemux_parse_samples:<qtdemux0> sample 0: index 0, timestamp > 0:00:00.000000000 > 69 0:00:00.031209135 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:8149:qtdemux_parse_samples:<qtdemux0> all samples are keyframes > 70 0:00:00.031212720 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:10855:qtdemux_expose_streams:<qtdemux0> exposing streams > 71 0:00:00.031216188 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:10863:qtdemux_expose_streams:<qtdemux0> stream 0, id 1, fourcc avc1 > 72 0:00:00.031240344 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:2850:check_update_duration:<qtdemux0> Updating total duration to > 0:11:42.602602602 was 0:11:42.549312500 > 73 0:00:00.031249534 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:2855:check_update_duration:<qtdemux0> qtdemux->segment.duration: > 0:11:42.549312500 .stop: 0:11:42.549312500 > 74 0:00:00.031254995 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:2862:check_update_duration:<qtdemux0> Updated segment.duration and > segment.stop > 75 0:00:00.031258571 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:2874:check_update_duration:<qtdemux0> Updating stream #0 duration > to 0:11:42.602602602 > 76 0:00:00.031263416 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:2874:check_update_duration:<qtdemux0> Updating stream #1 duration > to 0:11:42.602602602 > 77 0:00:00.031271809 3575 0x7f95a81171e0 DEBUG qtdemux > qtdemux.c:7240:gst_qtdemux_configure_stream:<qtdemux0> Calculating > framerate, timescale 29970 gave fps_n 30000 fps_d 1001 Actual duration is about 11:42 (mvhd has correct value), but mdhd both video and audio have incorrect value (04:33)
Review of attachment 325420 [details] [review]: Adding this has the downside of increasing startup time as we loop over the stts table. I'm not sure it is worth doing this double check. That said, most stts entries are very short as the frames have a constant duration but also most files should have a correct duration field in the track headers. If we do this, perhaps we should set an upper bound to only do this check if the stts is small enough to avoid delaying startup too much? ::: gst/isomp4/qtdemux.c @@ +7646,3 @@ + stream->stts_total_duration += (guint64) (samples * duration); + } + gst_byte_reader_set_pos (&stream->stts, pos); shouldn't this be inside the block as well? Also maybe put this all in a separate function with a clear name to give a hint to code readers on what it is doing.
Created attachment 325860 [details] [review] qtdemux: Ensure stream duration by using stts box
Created attachment 325861 [details] [review] qtdemux: Ensure stream duration by using stts box
Comment on attachment 325860 [details] [review] qtdemux: Ensure stream duration by using stts box >From 4f3184343e7e7a7842c8aa9c7ccab24336251c1e Mon Sep 17 00:00:00 2001 >From: Seungha Yang <sh.yang@lge.com> >Date: Wed, 13 Apr 2016 22:18:15 +0900 >Subject: [PATCH] qtdemux: Ensure stream duration by using stts box > >Some files have incorrect stream duration value in mdhd box >So, we need to double check the stream duration. > >https://bugzilla.gnome.org/show_bug.cgi?id=764637 >--- > gst/isomp4/qtdemux.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 68 insertions(+) > >diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c >index c4aa9c5..1925a44 100644 >--- a/gst/isomp4/qtdemux.c >+++ b/gst/isomp4/qtdemux.c >@@ -362,6 +362,9 @@ struct _QtDemuxStream > guint32 stts_sample_index; > guint64 stts_time; > guint32 stts_duration; >+ guint64 stts_total_duration; /* To ensure stream duration. non-zero value >+ * means we need to use this value for updating >+ * stream duration */ > /* stss */ > gboolean stss_present; > guint32 n_sample_syncs; >@@ -7185,6 +7188,12 @@ gst_qtdemux_configure_protected_caps (GstQTDemux * qtdemux, > static gboolean > gst_qtdemux_configure_stream (GstQTDemux * qtdemux, QtDemuxStream * stream) > { >+ if (stream->stts_total_duration) { >+ /* if stts_total_duration has non-zero value, update duration using it */ >+ check_update_duration (qtdemux, QTSTREAMTIME_TO_GSTTIME (stream, >+ stream->stts_total_duration)); >+ } >+ > if (stream->subtype == FOURCC_vide) { > /* fps is calculated base on the duration of the average framerate since > * qt does not have a fixed framerate. */ >@@ -7595,6 +7604,58 @@ flow_failed: > } > } > >+/* Check and compare duration of @stream with file duration. >+ * If stream duration is needed to update using each stts sample data, >+ * stts_total_duration will be set to non-zero value. >+ * To use this function, byte_reader position of stts should be located at >+ * the first entry of stts entry table. >+ */ >+static void >+qtdemux_ensure_stream_duration (GstQTDemux * qtdemux, QtDemuxStream * stream) >+{ >+ GstClockTime demux_dur, stream_dur, diff; >+ gint i; >+ guint byte_pos; >+ guint32 samples; >+ guint32 duration; >+ >+ stream->stts_total_duration = 0; >+ >+ /* we can't believe stts data of fragmented format */ >+ if (qtdemux->fragmented) >+ return; >+ >+ demux_dur = QTTIME_TO_GSTTIME (qtdemux, qtdemux->duration); >+ stream_dur = QTSTREAMTIME_TO_GSTTIME (stream, stream->duration); >+ >+ if (demux_dur == GST_CLOCK_TIME_NONE || stream_dur == GST_CLOCK_TIME_NONE) >+ return; >+ >+ diff = (demux_dur > stream_dur) ? >+ (demux_dur - stream_dur) : (stream_dur - demux_dur); >+ >+ /* If difference is over 1 second, need to double check stream duration */ >+ if (diff > GST_SECOND) { >+ GST_DEBUG_OBJECT (qtdemux, "duration difference %" GST_TIME_FORMAT >+ " = (demux duration %" GST_TIME_FORMAT " - stream duration %" >+ GST_TIME_FORMAT ")", GST_TIME_ARGS (diff), GST_TIME_ARGS (demux_dur), >+ GST_TIME_ARGS (stream_dur)); >+ >+ /* store position of byte_reader */ >+ byte_pos = gst_byte_reader_get_pos (&stream->stts); >+ >+ for (i = 0; i < stream->n_sample_times; i++) { >+ samples = gst_byte_reader_get_uint32_be_unchecked (&stream->stts); >+ duration = gst_byte_reader_get_uint32_be_unchecked (&stream->stts); >+ >+ stream->stts_total_duration += (guint64) (samples * duration); >+ } >+ >+ /* roll back byte_reader position to stored position */ >+ gst_byte_reader_set_pos (&stream->stts, byte_pos); >+ } >+} >+ > /* initialise bytereaders for stbl sub-atoms */ > static gboolean > qtdemux_stbl_init (GstQTDemux * qtdemux, QtDemuxStream * stream, GNode * stbl) >@@ -7624,6 +7685,13 @@ qtdemux_stbl_init (GstQTDemux * qtdemux, QtDemuxStream * stream, GNode * stbl) > goto corrupt_file; > } > >+ /* Some corrupted files have incorrect stream duration info in mdhd box >+ * If there is difference between file duration (in mvhd) and >+ * stream duration (in mdhd), it's the clue that this file maybe corrupted. >+ * In that case, we need to ensure actual stream duration by using stts. >+ */ >+ qtdemux_ensure_stream_duration (qtdemux, stream); >+ > /* sync sample atom */ > stream->stps_present = FALSE; > if ((stream->stss_present = >-- >1.9.1 >
(In reply to Thiago Sousa Santos from comment #4) > Review of attachment 325420 [details] [review] [review]: > > Adding this has the downside of increasing startup time as we loop over the > stts table. I'm not sure it is worth doing this double check. > > That said, most stts entries are very short as the frames have a constant > duration but also most files should have a correct duration field in the > track headers. > If we do this, perhaps we should set an upper bound to only do this check if > the stts is small enough to avoid delaying startup too much? > I agree with your opinion that this method can cause startup delay. So, I modified that checking stts entry only if a file possibly corrupted. That is, check difference file duration and stream duration. > ::: gst/isomp4/qtdemux.c > @@ +7646,3 @@ > + stream->stts_total_duration += (guint64) (samples * duration); > + } > + gst_byte_reader_set_pos (&stream->stts, pos); > > shouldn't this be inside the block as well? I use byte reader in order to accumulate each sample duration That is to roll back the byte reader position. > > Also maybe put this all in a separate function with a clear name to give a > hint to code readers on what it is doing.
Review of attachment 325861 [details] [review]: Patch looks good and makes sense, but I wouldn't mind someone else double-checking it.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/266.