Message ID | 20191215221614.51627-1-martin@martin.st |
---|---|
State | Superseded |
Headers | show |
On Mon, Dec 16, 2019 at 12:16:14AM +0200, Martin Storsjö wrote: > Keep all the existing data fields as they are (there's lots and > lots of nontrivial calculation and heuristics based on them in > their current form), but derive the duration as the difference > between the pts of the first packet to the maximum pts+duration > (not necessarily the last packet); use this duration in any box > where the actual presentation duration is supposed to be. > > Fixes: 8420 > --- > libavformat/movenc.c | 21 ++++++++++++++++----- > tests/ref/lavf/mov | 4 ++-- > tests/ref/lavf/mp4 | 4 ++-- > 3 files changed, 20 insertions(+), 9 deletions(-) This seems to loose duration for this: (not sure which way is more correct) ./ffmpeg -i ~/tickets/2892/MPEG_tbn_test.mov -c:v copy -c:a copy -vtag mx3n -timecode 10:00:00:00 -vframes 3 x.mov && ./ffprobe -v x.mov -show_packets -print_format compact https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2892/ packet|codec_type=data|stream_index=1|pts=0|pts_time=0.000000|dts=0|dts_time=0.000000|duration=3003|duration_time=0.100100|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=36|flags=K_ packet|codec_type=video|stream_index=0|pts=0|pts_time=0.000000|dts=0|dts_time=0.000000|duration=1001|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=125153|pos=40|flags=K_ packet|codec_type=video|stream_index=0|pts=1001|pts_time=0.033367|dts=1001|dts_time=0.033367|duration=1001|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=125153|pos=125193|flags=K_ packet|codec_type=video|stream_index=0|pts=2002|pts_time=0.066733|dts=2002|dts_time=0.066733|duration=1001|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=125153|pos=250346|flags=K_ vs. packet|codec_type=data|stream_index=1|pts=0|pts_time=0.000000|dts=0|dts_time=0.000000|duration=N/A|duration_time=N/A|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=36|flags=KD packet|codec_type=video|stream_index=0|pts=0|pts_time=0.000000|dts=0|dts_time=0.000000|duration=1001|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=125153|pos=40|flags=K_ packet|codec_type=video|stream_index=0|pts=1001|pts_time=0.033367|dts=1001|dts_time=0.033367|duration=1001|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=125153|pos=125193|flags=K_ packet|codec_type=video|stream_index=0|pts=2002|pts_time=0.066733|dts=2002|dts_time=0.066733|duration=1001|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=125153|pos=250346|flags=K_ [...]
On Mon, 16 Dec 2019, Michael Niedermayer wrote: > On Mon, Dec 16, 2019 at 12:16:14AM +0200, Martin Storsjö wrote: >> Keep all the existing data fields as they are (there's lots and >> lots of nontrivial calculation and heuristics based on them in >> their current form), but derive the duration as the difference >> between the pts of the first packet to the maximum pts+duration >> (not necessarily the last packet); use this duration in any box >> where the actual presentation duration is supposed to be. >> >> Fixes: 8420 >> --- >> libavformat/movenc.c | 21 ++++++++++++++++----- >> tests/ref/lavf/mov | 4 ++-- >> tests/ref/lavf/mp4 | 4 ++-- >> 3 files changed, 20 insertions(+), 9 deletions(-) > > This seems to loose duration for this: (not sure which way is more correct) > > ./ffmpeg -i ~/tickets/2892/MPEG_tbn_test.mov -c:v copy -c:a copy -vtag mx3n -timecode 10:00:00:00 -vframes 3 x.mov && ./ffprobe -v x.mov -show_packets -print_format compact Thanks for testing. Seems like there's a special case for how track_duration is updated for tmcd tracks, where it's updated based on another track; I'll update the patch to fetch the effective duration for that other track instead. // Martin
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index dd144ae20a..24878746a1 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -2760,10 +2760,21 @@ static int mov_write_minf_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext return update_size(pb, pos); } +static int64_t calc_pts_duration(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); + } + return track->track_duration; +} + static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *track) { - int version = track->track_duration < INT32_MAX ? 0 : 1; + int64_t duration = calc_pts_duration(track); + int version = duration < INT32_MAX ? 0 : 1; if (track->mode == MODE_ISM) version = 1; @@ -2785,7 +2796,7 @@ static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov, else if (!track->entry) (version == 1) ? avio_wb64(pb, 0) : avio_wb32(pb, 0); else - (version == 1) ? avio_wb64(pb, track->track_duration) : avio_wb32(pb, track->track_duration); /* duration */ + (version == 1) ? avio_wb64(pb, duration) : avio_wb32(pb, duration); /* duration */ avio_wb16(pb, track->language); /* language */ avio_wb16(pb, 0); /* reserved (quality) */ @@ -2835,7 +2846,7 @@ static void write_matrix(AVIOContext *pb, int16_t a, int16_t b, int16_t c, static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *track, AVStream *st) { - int64_t duration = av_rescale_rnd(track->track_duration, MOV_TIMESCALE, + int64_t duration = av_rescale_rnd(calc_pts_duration(track), MOV_TIMESCALE, track->timescale, AV_ROUND_UP); int version = duration < INT32_MAX ? 0 : 1; int flags = MOV_TKHD_FLAG_IN_MOVIE; @@ -2982,7 +2993,7 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track) static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *track) { - int64_t duration = av_rescale_rnd(track->track_duration, MOV_TIMESCALE, + int64_t duration = av_rescale_rnd(calc_pts_duration(track), MOV_TIMESCALE, track->timescale, AV_ROUND_UP); int version = duration < INT32_MAX ? 0 : 1; int entry_size, entry_count, size; @@ -3269,7 +3280,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb, MOVMuxContext *mov) for (i = 0; i < mov->nb_streams; i++) { if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) { - int64_t max_track_len_temp = av_rescale_rnd(mov->tracks[i].track_duration, + int64_t max_track_len_temp = av_rescale_rnd(calc_pts_duration(&mov->tracks[i]), MOV_TIMESCALE, mov->tracks[i].timescale, AV_ROUND_UP); diff --git a/tests/ref/lavf/mov b/tests/ref/lavf/mov index 75a0c4892d..71aad501d6 100644 --- a/tests/ref/lavf/mov +++ b/tests/ref/lavf/mov @@ -1,7 +1,7 @@ -11bd76730274924e02623172b82b5236 *tests/data/lavf/lavf.mov +a0d1d1b41830a46b91d1282ced7e7faf *tests/data/lavf/lavf.mov 357539 tests/data/lavf/lavf.mov tests/data/lavf/lavf.mov CRC=0xbb2b949b -6efa586655e3db043cb29668f5216610 *tests/data/lavf/lavf.mov +46a18d83ae4f9ecfc993f135e62d484d *tests/data/lavf/lavf.mov 366621 tests/data/lavf/lavf.mov tests/data/lavf/lavf.mov CRC=0xa9793231 c80c625ded376602e71d5aa6ac6fdb1c *tests/data/lavf/lavf.mov diff --git a/tests/ref/lavf/mp4 b/tests/ref/lavf/mp4 index 8482812380..74b286e537 100644 --- a/tests/ref/lavf/mp4 +++ b/tests/ref/lavf/mp4 @@ -1,7 +1,7 @@ -ebca72c186a4f3ba9bb17d9cb5b74fef *tests/data/lavf/lavf.mp4 +3a14053a4f655ac77850160f592ded17 *tests/data/lavf/lavf.mp4 312457 tests/data/lavf/lavf.mp4 tests/data/lavf/lavf.mp4 CRC=0x9d9a638a -9944512475d82d2d601f3c96101bdf9c *tests/data/lavf/lavf.mp4 +7568ed0224236ee9731b686708e098e0 *tests/data/lavf/lavf.mp4 321343 tests/data/lavf/lavf.mp4 tests/data/lavf/lavf.mp4 CRC=0xe8130120 7b3e71f294901067046c09f03a426bdc *tests/data/lavf/lavf.mp4