diff mbox

[FFmpeg-devel] movenc: Write durations based on pts into mvhd/mdhd/tkhd/elst

Message ID 20191215221614.51627-1-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Dec. 15, 2019, 10:16 p.m. UTC 
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(-)

Comments

Michael Niedermayer Dec. 16, 2019, 7:25 p.m. UTC | #1 
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_


[...]
Martin Storsjö Dec. 17, 2019, 12:47 p.m. UTC | #2 
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 mbox

Patch

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