GNOME Bugzilla – Bug 753484
qtdemux: support edit lists partially in push-mode
Last modified: 2015-11-16 11:54:24 UTC
Currently qtdemux ignores edit list entries when in push-mode. This can cause out-of-sync playback if stream has negative DTS signaled in the edit list and also will ignore gaps at the beginning of the streams. Supporting those basic scenarios should be possible in push-mode.
Created attachment 309031 [details] [review] qtdemux: respect qt segments in push-mode for empty starts In push-mode it is hard to support qt segments overall but it is possible to support when the file isn't heavily edited but just contain a segment to indicate a gap at the beginning. This also allows properly timestamping data that has negative DTS in push-mode. It is relevant to support those for 2 scenarios: 1) fragmented streaming 2) HTTP playback of 'regular' mp4
Created attachment 309032 [details] [review] qtdemux: handle empty segments in seeking adjust If seeking targets an empty segment skip it as there is no media offset to get from it. Instead look for the next one. This doesn't make seeking in push-mode work if you seek to an empty segment but at least won't get you to wrong offsets.
Putting those here but it is a new feature and we can do it after 1.6.
Review of attachment 309032 [details] [review]: Looks good to me. It's not that intrusive though, but I leave it as after-freeze for now.
Review of attachment 309031 [details] [review]: ::: gst/isomp4/qtdemux.c @@ +5756,3 @@ + /* Only support empty segment at the beginning followed by + * one non-empty segment */ + g_assert (i + 1 == stream->n_segments); As it's something we don't want to support, we should maybe cleanly error out. Assertion are better for catching programming error, not for stream errors (or maybe this is only reached if there is a programming error elsewhere ?).
(In reply to Nicolas Dufresne (stormer) from comment #5) > Review of attachment 309031 [details] [review] [review]: > > ::: gst/isomp4/qtdemux.c > @@ +5756,3 @@ > + /* Only support empty segment at the beginning followed by > + * one non-empty segment */ > + g_assert (i + 1 == stream->n_segments); > > As it's something we don't want to support, we should maybe cleanly error > out. Assertion are better for catching programming error, not for stream > errors (or maybe this is only reached if there is a programming error > elsewhere ?). It is an assert because otherwise qtdemux should ignore the edts and just play the whole media as if it had no edts at all (the current behavior for push-mode).
What's the status here?
Review of attachment 309032 [details] [review]: commit df0a31b4eefb6caff82705b8515218d17e0369ae Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Mon Aug 10 18:14:39 2015 -0300 qtdemux: handle empty segments in seeking adjust If seeking targets an empty segment skip it as there is no media offset to get from it. Instead look for the next one. This doesn't make seeking in push-mode work if you seek to an empty segment but at least won't get you to wrong offsets. https://bugzilla.gnome.org/show_bug.cgi?id=753484
commit 142d8e2d23e5602e7382977af1043d621625f8c8 Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Mon Aug 10 11:23:45 2015 -0300 qtdemux: respect qt segments in push-mode for empty starts In push-mode it is hard to support qt segments overall but it is possible to support when the file isn't heavily edited but just contain a segment to indicate a gap at the beginning. This also allows properly timestamping data that has negative DTS in push-mode. It is relevant to support those for 2 scenarios: 1) fragmented streaming 2) HTTP playback of 'regular' mp4 https://bugzilla.gnome.org/show_bug.cgi?id=753484
Comment on attachment 309031 [details] [review] qtdemux: respect qt segments in push-mode for empty starts Merged an slightly different version with better comments/documentation
See bug #758171, this broke seeking in dashdemux.