On Fri, 11 Dec 2020 at 14:07, Martin Storsjö <mar...@martin.st> wrote: > > On Fri, 11 Dec 2020, Josh Allmann wrote: > > > A negative start_dts value (eg, delay from edit lists) typically yields > > a duration larger than end_pts. During edit list processing, the > > delay is removed again, yielding the correct duration within the elst. > > > > However, other duration-carrying atoms (tkhd, mvhd, mdhd) still have > > the delay incorporated into their durations. This is incorrect. > > > > Fix this by withholding delay from the duration if edit lists are used. > > This also simplifies edit-list processing a bit, since the delay > > does not need to be removed from the calculated duration again. > > --- > > > > The mov spec says that the tkhd duration is "derived from the track's > > edits" [1] and the duratons of the other atoms (mvhd, mdhd) are in turn > > taken from the longest track. So it seems that incorporating the delay > > into the track duration is a bug in itself when the edit list has the > > correct duration, and this propagates out tothe other top-level durations. > > > > Unsure of how this change interacts with other modes that may expect > > negative timestamps such as CMAF, so the patch errs on the side of > > caution and only takes effect if edit lists are used. Can loosen that > > up if necessary. > > > > [1] > > https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap2/qtff2.html#//apple_ref/doc/uid/TP40000939-CH204-BBCEIDFA > > > > libavformat/movenc.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > > index 7db2e28840..31441a9f6c 100644 > > --- a/libavformat/movenc.c > > +++ b/libavformat/movenc.c > > @@ -2831,7 +2831,14 @@ static int64_t calc_pts_duration(MOVMuxContext *mov, > > MOVTrack *track) > > if (track->end_pts != AV_NOPTS_VALUE && > > track->start_dts != AV_NOPTS_VALUE && > > track->start_cts != AV_NOPTS_VALUE) { > > - return track->end_pts - (track->start_dts + track->start_cts); > > + int64_t dur = track->end_pts, delay = track->start_dts + > > track->start_cts; > > + /* Note, this delay is calculated from the pts of the first sample, > > + * ensuring that we don't reduce the duration for cases with > > + * dts<0 pts=0. */ > > If you have a stream starting with dts<0 pts=0, you'll have start_pts = > start_dts + start_cts = 0. That gives delay=0 after your modification. But > the comment says "don't reduce the duration for cases with pts=0" - where > the delay variable would be zero anyway? >
I'm not quite sure what you mean - that the comment is outdated? Or that this modification would perhaps not behave as expected? For what it's worth, the cases I'm concerned with have start_pts < 0. > > > I don't manage to follow the reasoning and explanation in the commit > message. To be able to concretely reason about this issue at all, we need > to look at a concrete example. Can you provide a sample input file and a > reproducible command, and point out which exact field in the muxer output > of that case that you consider wrong? > Had to create a trac to find somewhere to host the sample. Tried to put some details there but the formatting seems messed up and I can't figure out how to edit, apologies. So here is some more info - Input sample: https://trac.ffmpeg.org/raw-attachment/ticket/9028/test-timecode.mp4 Run the following for a transmuxed clip from 3s for a 5s duration: ffmpeg -ss 3 -i test-timecode.mp4 -t 5 -c copy out.mp4 Note that the actual cut location is mid-GOP, so there's a 1s pts delay at the beginning of the output file with negative pts. ffprobe shows: ffprobe -show_streams -show_format out.mp4 2>&1 | grep duration= duration=5.166992 # stream duration - correct duration=6.167000 # format duration - incorrect mp4dump'ing out.mp4 gives this: # incorrect: duration should be sum of elst durations [tkhd] size=12+80, flags=3 duration = 6167 # correct [elst] size=12+16 entry_count = 1 entry/segment duration = 5167 # incorrect; derived from track duration (tkhd) [mvhd] size=12+96 timescale = 1000 duration = 6167 > // Martin > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".