diff mbox series

[FFmpeg-devel,2/3] avformat/movenc: Use base container timescale, instead of hard coded default

Message ID 1587405459-29001-3-git-send-email-kevin.j.wheatley@gmail.com
State New
Headers show
Series avformat/movenc: Support for variable timescale in mov containers | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Kevin Wheatley April 20, 2020, 5:57 p.m. UTC
Signed-off-by: Kevin Wheatley <kevin.j.wheatley@gmain.com>
---
 libavformat/movenc.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Marton Balint May 21, 2020, 9:19 p.m. UTC | #1
On Mon, 20 Apr 2020, Kevin Wheatley wrote:

> Signed-off-by: Kevin Wheatley <kevin.j.wheatley@gmain.com>
> ---
> libavformat/movenc.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)

You should squash this patch with the previous.

>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 7d79eca..508fa73 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -2879,7 +2879,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>                               MOVTrack *track, AVStream *st)
> {
>     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
> -                                      MOV_TIMESCALE, track->timescale,
> +                                      mov->mov_timescale, track->timescale,
>                                       AV_ROUND_UP);
>     int version = duration < INT32_MAX ? 0 : 1;
>     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
> @@ -3027,7 +3027,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>                               MOVTrack *track)
> {
>     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
> -                                      MOV_TIMESCALE, track->timescale,
> +                                      mov->mov_timescale, track->timescale,
>                                       AV_ROUND_UP);
>     int version = duration < INT32_MAX ? 0 : 1;
>     int entry_size, entry_count, size;
> @@ -3046,7 +3046,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>         }
>     }
> 
> -    delay = av_rescale_rnd(start_dts + start_ct, MOV_TIMESCALE,
> +    delay = av_rescale_rnd(start_dts + start_ct, mov->mov_timescale,
>                            track->timescale, AV_ROUND_DOWN);
>     version |= delay < INT32_MAX ? 0 : 1;
> 
> @@ -3081,8 +3081,8 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>         /* Avoid accidentally ending up with start_ct = -1 which has got a
>          * special meaning. Normally start_ct should end up positive or zero
>          * here, but use FFMIN in case dts is a small positive integer
> -         * rounded to 0 when represented in MOV_TIMESCALE units. */
> -        av_assert0(av_rescale_rnd(start_dts, MOV_TIMESCALE, track->timescale, AV_ROUND_DOWN) <= 0);
> +         * rounded to 0 when represented in mov->mov_timescale units. */
> +        av_assert0(av_rescale_rnd(start_dts, mov->mov_timescale, track->timescale, AV_ROUND_DOWN) <= 0);
>         start_ct  = -FFMIN(start_dts, 0);
>         /* Note, this delay is calculated from the pts of the first sample,
>          * ensuring that we don't reduce the duration for cases with
> @@ -3316,7 +3316,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb, MOVMuxContext *mov)
>         if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) {
>             int64_t max_track_len_temp = av_rescale_rnd(
>                                                 calc_pts_duration(mov, &mov->tracks[i]),
> -                                                MOV_TIMESCALE,
> +                                                mov->mov_timescale,
>                                                 mov->tracks[i].timescale,
>                                                 AV_ROUND_UP);
>             if (max_track_len < max_track_len_temp)
> @@ -3345,7 +3345,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb, MOVMuxContext *mov)
>         avio_wb32(pb, mov->time); /* creation time */
>         avio_wb32(pb, mov->time); /* modification time */
>     }
> -    avio_wb32(pb, MOV_TIMESCALE);
> +    avio_wb32(pb, mov->mov_timescale);
>     (version == 1) ? avio_wb64(pb, max_track_len) : avio_wb32(pb, max_track_len); /* duration of longest track */
>
>     avio_wb32(pb, 0x00010000); /* reserved (preferred rate) 1.0 = normal */
> @@ -5921,7 +5921,7 @@ static int mov_create_chapter_track(AVFormatContext *s, int tracknum)
>
>     track->mode = mov->mode;
>     track->tag = MKTAG('t','e','x','t');
> -    track->timescale = MOV_TIMESCALE;
> +    track->timescale = mov->mov_timescale;
>     track->par = avcodec_parameters_alloc();
>     if (!track->par)
>         return AVERROR(ENOMEM);
> @@ -5982,8 +5982,8 @@ static int mov_create_chapter_track(AVFormatContext *s, int tracknum)
>         AVChapter *c = s->chapters[i];
>         AVDictionaryEntry *t;
> 
> -        int64_t end = av_rescale_q(c->end, c->time_base, (AVRational){1,MOV_TIMESCALE});
> -        pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base, (AVRational){1,MOV_TIMESCALE});
> +        int64_t end = av_rescale_q(c->end, c->time_base, (AVRational){1,mov->mov_timescale});
> +        pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base, (AVRational){1,mov->mov_timescale});
>         pkt.duration = end - pkt.dts;
>
>         if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
> @@ -6518,7 +6518,7 @@ static int mov_init(AVFormatContext *s)
>         } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
>             track->timescale = st->time_base.den;
>         } else {
> -            track->timescale = MOV_TIMESCALE;
> +            track->timescale = mov->mov_timescale;

I think this should remain MOV_TIMESCALE. This is a stream with unknown 
stream type, we have no idea what track timescale to use, and in the next 
patch I think you should determine the mov timescale based on the track 
timescales and not time bases.

Regards,
Marton
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 7d79eca..508fa73 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2879,7 +2879,7 @@  static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
                               MOVTrack *track, AVStream *st)
 {
     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
-                                      MOV_TIMESCALE, track->timescale,
+                                      mov->mov_timescale, track->timescale,
                                       AV_ROUND_UP);
     int version = duration < INT32_MAX ? 0 : 1;
     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
@@ -3027,7 +3027,7 @@  static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
                               MOVTrack *track)
 {
     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
-                                      MOV_TIMESCALE, track->timescale,
+                                      mov->mov_timescale, track->timescale,
                                       AV_ROUND_UP);
     int version = duration < INT32_MAX ? 0 : 1;
     int entry_size, entry_count, size;
@@ -3046,7 +3046,7 @@  static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
         }
     }
 
-    delay = av_rescale_rnd(start_dts + start_ct, MOV_TIMESCALE,
+    delay = av_rescale_rnd(start_dts + start_ct, mov->mov_timescale,
                            track->timescale, AV_ROUND_DOWN);
     version |= delay < INT32_MAX ? 0 : 1;
 
@@ -3081,8 +3081,8 @@  static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
         /* Avoid accidentally ending up with start_ct = -1 which has got a
          * special meaning. Normally start_ct should end up positive or zero
          * here, but use FFMIN in case dts is a small positive integer
-         * rounded to 0 when represented in MOV_TIMESCALE units. */
-        av_assert0(av_rescale_rnd(start_dts, MOV_TIMESCALE, track->timescale, AV_ROUND_DOWN) <= 0);
+         * rounded to 0 when represented in mov->mov_timescale units. */
+        av_assert0(av_rescale_rnd(start_dts, mov->mov_timescale, track->timescale, AV_ROUND_DOWN) <= 0);
         start_ct  = -FFMIN(start_dts, 0);
         /* Note, this delay is calculated from the pts of the first sample,
          * ensuring that we don't reduce the duration for cases with
@@ -3316,7 +3316,7 @@  static int mov_write_mvhd_tag(AVIOContext *pb, MOVMuxContext *mov)
         if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) {
             int64_t max_track_len_temp = av_rescale_rnd(
                                                 calc_pts_duration(mov, &mov->tracks[i]),
-                                                MOV_TIMESCALE,
+                                                mov->mov_timescale,
                                                 mov->tracks[i].timescale,
                                                 AV_ROUND_UP);
             if (max_track_len < max_track_len_temp)
@@ -3345,7 +3345,7 @@  static int mov_write_mvhd_tag(AVIOContext *pb, MOVMuxContext *mov)
         avio_wb32(pb, mov->time); /* creation time */
         avio_wb32(pb, mov->time); /* modification time */
     }
-    avio_wb32(pb, MOV_TIMESCALE);
+    avio_wb32(pb, mov->mov_timescale);
     (version == 1) ? avio_wb64(pb, max_track_len) : avio_wb32(pb, max_track_len); /* duration of longest track */
 
     avio_wb32(pb, 0x00010000); /* reserved (preferred rate) 1.0 = normal */
@@ -5921,7 +5921,7 @@  static int mov_create_chapter_track(AVFormatContext *s, int tracknum)
 
     track->mode = mov->mode;
     track->tag = MKTAG('t','e','x','t');
-    track->timescale = MOV_TIMESCALE;
+    track->timescale = mov->mov_timescale;
     track->par = avcodec_parameters_alloc();
     if (!track->par)
         return AVERROR(ENOMEM);
@@ -5982,8 +5982,8 @@  static int mov_create_chapter_track(AVFormatContext *s, int tracknum)
         AVChapter *c = s->chapters[i];
         AVDictionaryEntry *t;
 
-        int64_t end = av_rescale_q(c->end, c->time_base, (AVRational){1,MOV_TIMESCALE});
-        pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base, (AVRational){1,MOV_TIMESCALE});
+        int64_t end = av_rescale_q(c->end, c->time_base, (AVRational){1,mov->mov_timescale});
+        pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base, (AVRational){1,mov->mov_timescale});
         pkt.duration = end - pkt.dts;
 
         if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
@@ -6518,7 +6518,7 @@  static int mov_init(AVFormatContext *s)
         } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
             track->timescale = st->time_base.den;
         } else {
-            track->timescale = MOV_TIMESCALE;
+            track->timescale = mov->mov_timescale;
         }
         if (!track->height)
             track->height = st->codecpar->height;