After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 794858 - qtdemux: edits with duration = 0 don't work on files without a mehd
qtdemux: edits with duration = 0 don't work on files without a mehd
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-03-31 12:38 UTC by Alicia Boya García
Modified: 2018-05-23 12:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
qtdemux: Add support zero duration edit list (2.29 KB, patch)
2018-03-31 13:17 UTC, Seungha Yang
reviewed Details | Review
Patch (2.24 KB, patch)
2018-03-31 15:35 UTC, Alicia Boya García
none Details | Review
qtdemux: fix buggy duration in edits with duration=0 in fragmented files without a mehd (2.50 KB, patch)
2018-05-22 13:39 UTC, Alicia Boya García
committed Details | Review

Description Alicia Boya García 2018-03-31 12:38:33 UTC
The current code for handling edits with duration = 0 is buggy. For instance, it fails when it's used in a fragmented file without mehd due to an addition performed with GST_CLOCK_TIME_NONE.

$ gst-launch-1.0 -v filesrc location=car-20120827-86.mp4 ! qtdemux ! fakesink silent=false 
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: stream-start (10254), GstEventStreamStart, stream-id=(string)d202bac1545cd2bbacbaee3f292c0a7144a5ac36218e05839925b014107bd12f/001, flags=(GstStreamFlags)GST_STREAM_FLAG_NONE, group-id=(uint)0;) 0x55e5cafbbdd0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: caps (12814), GstEventCaps, caps=(GstCaps)"video/x-h264\,\ stream-format\=\(string\)avc\,\ alignment\=\(string\)au\,\ level\=\(string\)3\,\ profile\=\(string\)main\,\ codec_data\=\(buffer\)014d401effe10016674d401ee8805017fcb0800001f480005dc0078b168901000468ebaf20\,\ width\=\(int\)640\,\ height\=\(int\)360\,\ framerate\=\(fraction\)24000/1001\,\ pixel-aspect-ratio\=\(fraction\)1/1";) 0x55e5cafbbe40
/GstPipeline:pipeline0/GstFakeSink:fakesink0.GstPad:sink: caps = video/x-h264, stream-format=(string)avc, alignment=(string)au, level=(string)3, profile=(string)main, codec_data=(buffer)014d401effe10016674d401ee8805017fcb0800001f480005dc0078b168901000468ebaf20, width=(int)640, height=(int)360, framerate=(fraction)24000/1001, pixel-aspect-ratio=(fraction)1/1
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: segment (17934), GstEventSegment, segment=(GstSegment)"GstSegment, flags=(GstSegmentFlags)GST_SEGMENT_FLAG_NONE, rate=(double)1, applied-rate=(double)1, format=(GstFormat)GST_FORMAT_TIME, base=(guint64)0, offset=(guint64)0, start=(guint64)41711110, stop=(guint64)41711110, time=(guint64)0, position=(guint64)41711110, duration=(guint64)18446744073709551615;";) 0x55e5cafbbf20
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: tag (20510), GstTagList-stream, taglist=(taglist)"taglist\,\ video-codec\=\(string\)\"H.264\\\ /\\\ AVC\"\;";) 0x7f17d4010040
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: tag (20510), GstTagList-global, taglist=(taglist)"taglist\,\ datetime\=\(datetime\)2012-08-27T01:01:22Z\,\ container-format\=\(string\)\"ISO\\\ fMP4\"\;";) 0x7f17d40100b0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: eos (28174), ) 0x7f17d4010120
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
Got EOS from element "pipeline0".
Execution ended after 0:00:00.000084010
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...
Comment 2 Seungha Yang 2018-03-31 13:17:42 UTC
Created attachment 370381 [details] [review]
qtdemux: Add support zero duration edit list

See also bug #778426 and bug #793058
Comment 3 Alicia Boya García 2018-03-31 15:35:52 UTC
Created attachment 370389 [details] [review]
Patch
Comment 4 Alicia Boya García 2018-03-31 16:09:56 UTC
Review of attachment 370381 [details] [review]:

The patch I just uploaded fixes the commented issues.

::: gst/isomp4/qtdemux.c
@@ -9087,3 @@
       segment->time = stime;
       /* add non scaled values so we don't cause roundoff errors */
-      if (duration || media_start == GST_CLOCK_TIME_NONE) {

This condition handles a certain edge case in a particular way: an empty edit with duration = 0.

Before, it was added as as an empty of duration = 0.
After this patch, it is treated as an empty edit that extends forever.

According to the spec, the new behavior is incorrect because duration=0 only means "until the end of the track" for non-empty edits (it makes sense to have that limitation because, even if you knew for sure the duration of the track, you could not calculate the duration of the edit because you lack media_start).

The behavior before does not make much sense either, and the same could be said for the edit itself.

In any case, I'd either keep the current behavior or make it ignore these edge edits, but not change it to a different (questionable too) behavior.

@@ +9090,3 @@
+        /* zero duration does not imply media_start == media_stop
+         * but, only specify media_start */
+        segment->duration = stime = GST_CLOCK_TIME_NONE;

When such an edit is found, it should be the last edit. Otherwise the next edit would start on GST_CLOCK_TIME_NONE.
Comment 5 Alicia Boya García 2018-05-22 13:39:16 UTC
Created attachment 372333 [details] [review]
qtdemux: fix buggy duration in edits with duration=0 in fragmented files without a mehd
Comment 6 Thibault Saunier 2018-05-23 12:27:10 UTC
Attachment 372333 [details] pushed as ee78825 - qtdemux: fix buggy duration in edits with duration=0 in fragmented files without a mehd