GNOME Bugzilla – Bug 760781
qtdemux: handling zero segment-duration edit list
Last modified: 2016-04-10 23:41:31 UTC
Based on document ISO_IEC_14496-12, edit list box can have segment duration as zero. It does not imply that media_start equals to media_stop. But, it just indicates a sample which should be presented at the first. This patch derives segment duration using media_time and duration of file. And set derived duration to segment-duration.
Created attachment 319256 [details] [review] handling zero segment-duration edit list
Review of attachment 319256 [details] [review]: ::: gst/isomp4/qtdemux.c @@ +8057,3 @@ + /* zero duration does not imply media_start == media_stop + * but, only specify media_start.*/ + stime = QTTIME_TO_GSTTIME (qtdemux, qtdemux->duration); We might have to check if the duration is known and valid, and otherwise just set stime to GST_CLOCK_TIME_NONE. @@ +8059,3 @@ + stime = QTTIME_TO_GSTTIME (qtdemux, qtdemux->duration); + segment->duration = + stime - QTSTREAMTIME_TO_GSTTIME (stream, media_time);; Doubled semicolon here. Also you might have to check if QTSTREAMTIME_TO_GSTTIME() returns something valid here, i.e. if media_time is valid. Or is that ensured elsewhere already? Otherwise you might calculate with GST_CLOCK_TIME_NONE here.
Created attachment 319580 [details] [review] handling zero segment-duration edit list (patch set #2)
Created attachment 319583 [details] [review] handling zero segment-duration edit list (patch set #2)
Review of attachment 319583 [details] [review]: ::: gst/isomp4/qtdemux.c @@ +8019,3 @@ + stime - QTSTREAMTIME_TO_GSTTIME (stream, media_time); + segment->duration = + segment_dur >= 0 ? segment_dur : GST_CLOCK_TIME_NONE; This check is not doing anything: segment_dur is unsigned, so always >= 0. Instead check before the subtraction if stime >= media_time_as_gst_clock_time. @@ +8021,3 @@ + segment_dur >= 0 ? segment_dur : GST_CLOCK_TIME_NONE; + } else + segment->duration = GST_CLOCK_TIME_NONE; Add {} around this
Created attachment 319939 [details] [review] handling zero segment-duration edit list (patch set #3)
(In reply to Sebastian Dröge (slomo) from comment #5) > Review of attachment 319583 [details] [review] [review]: > > ::: gst/isomp4/qtdemux.c > @@ +8019,3 @@ > + stime - QTSTREAMTIME_TO_GSTTIME (stream, media_time); > + segment->duration = > + segment_dur >= 0 ? segment_dur : GST_CLOCK_TIME_NONE; > > This check is not doing anything: segment_dur is unsigned, so always >= 0. > Instead check before the subtraction if stime >= > media_time_as_gst_clock_time. --> Done in patch set #3 > > @@ +8021,3 @@ > + segment_dur >= 0 ? segment_dur : GST_CLOCK_TIME_NONE; > + } else > + segment->duration = GST_CLOCK_TIME_NONE; > > Add {} around this --> Done in patch set #3 Additionally in patch set #3, I declared new local variable "media_start" in order to avoid double time unit conversion QTSTREAMTIME_TO_GSTTIME (stream, media_time).
commit 0391a93a3519c2ea4c375e9668b481c851e59d08 Author: Seungha Yang <sh.yang@lge.com> Date: Thu Jan 28 22:36:23 2016 +0900 qtdemux: handling zero segment-duration edit list Based on document ISO_IEC_14496-12, edit list box can have segment duration as zero. It does not imply that media_start equals to media_stop. But, it just indicates a sample which should be presented at the first. This patch derives segment duration using media_time and duration of file. And set derived duration to segment-duration. https://bugzilla.gnome.org/show_bug.cgi?id=760781
It looks like this commit breaks playback of some streams, at least this one is not working in 1.8.0, buffers not go out of qtdemux: http://e1.cdnak.neulion.com/nhl/vod/2016/01/27/741/2_741_phi_wsh_1516_h_continuous_1_1600K_16x9.mp4 If I revert it stream works.
(In reply to Maroš Ondrášek from comment #9) > It looks like this commit breaks playback of some streams, at least this one > is not working in 1.8.0, buffers not go out of qtdemux: > http://e1.cdnak.neulion.com/nhl/vod/2016/01/27/741/ > 2_741_phi_wsh_1516_h_continuous_1_1600K_16x9.mp4 > > If I revert it stream works. Thanks for your regression information. I uploaded patch at https://bugzilla.gnome.org/show_bug.cgi?id=764870