diff mbox series

[FFmpeg-devel] movenc: add movie_timescale option instead of hardcoding 1000

Message ID CABLWnS96xLwZ4Yx-LdjGPZUpyWfwGOqrPf+b9uNxqxK6MRfYBQ@mail.gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] movenc: add movie_timescale option instead of hardcoding 1000 | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Vittorio Giovara April 27, 2021, 9:35 p.m. UTC 
From: Justin Ruggles <justin.ruggles@gmail.com>

There are cases where using 1000 as the MP4 timescale is not
accurate enough, for example when one needs sample-accurate audio
handling.

This adds a new AVOption to the MOV/MP4 muxer to override the
movie timescale, but it still defaults to 1000 to maintain current
default behavior.

Comments

Gyan Doshi April 28, 2021, 4:25 a.m. UTC | #1 
On 2021-04-28 03:05, Vittorio Giovara wrote:
> From: Justin Ruggles <justin.ruggles@gmail.com>
>
> There are cases where using 1000 as the MP4 timescale is not
> accurate enough, for example when one needs sample-accurate audio
> handling.
>
> This adds a new AVOption to the MOV/MP4 muxer to override the
> movie timescale, but it still defaults to 1000 to maintain current
> default behavior.

Please resend with patch inlined. Patchwork will then trigger fate tests.

Regards,
Gyan
Andriy Gelman April 28, 2021, 4:41 a.m. UTC | #2 
On Wed, 28. Apr 09:55, Gyan Doshi wrote:
> 
> 
> On 2021-04-28 03:05, Vittorio Giovara wrote:
> > From: Justin Ruggles <justin.ruggles@gmail.com>
> > 
> > There are cases where using 1000 as the MP4 timescale is not
> > accurate enough, for example when one needs sample-accurate audio
> > handling.
> > 
> > This adds a new AVOption to the MOV/MP4 muxer to override the
> > movie timescale, but it still defaults to 1000 to maintain current
> > default behavior.
> 
> Please resend with patch inlined. Patchwork will then trigger fate tests.
> 

Patchwork can also accept patches as attachments, but they need to have
text/x-diff or text/x-patch mime type.

I manually changed the mime type on this email and pushed it to patchwork.
https://patchwork.ffmpeg.org/project/ffmpeg/patch/CABLWnS96xLwZ4Yx-LdjGPZUpyWfwGOqrPf+b9uNxqxK6MRfYBQ@mail.gmail.com/
Kevin Wheatley April 30, 2021, 9:02 a.m. UTC | #3 
This is quite similar to my patch suggestion from last year:

http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/261088.html

mine was missing some docs updates, and I followed the existing naming
scheme of prefixing the argument as 'mov_' rather than movie, but
otherwise is quite similar

Kevin
Vittorio Giovara May 3, 2021, 1:49 p.m. UTC | #4 
On Fri, Apr 30, 2021 at 12:02 PM Kevin Wheatley <kevin.j.wheatley@gmail.com>
wrote:

> This is quite similar to my patch suggestion from last year:
>
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/261088.html
>
> mine was missing some docs updates, and I followed the existing naming
> scheme of prefixing the argument as 'mov_' rather than movie, but
> otherwise is quite similar
>
> Kevin
>

Yeah 1/3 + 2/3 are basically the same, was there any reason why it wasn't
merged?

If there are no strong objections, I'd like to push this tomorrow.

Thanks for the help in getting this email to work in patchwork.
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 8e6ed81..93f93e4 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -107,6 +107,7 @@  static const AVOption options[] = {
     { "wallclock", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = MOV_PRFT_SRC_WALLCLOCK}, 0, 0, AV_OPT_FLAG_ENCODING_PARAM, "prft"},
     { "pts", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = MOV_PRFT_SRC_PTS}, 0, 0, AV_OPT_FLAG_ENCODING_PARAM, "prft"},
     { "empty_hdlr_name", "write zero-length name string in hdlr atoms within mdia and minf atoms", offsetof(MOVMuxContext, empty_hdlr_name), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, AV_OPT_FLAG_ENCODING_PARAM},
+    { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
     { NULL },
 };
 
@@ -3039,7 +3040,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->movie_timescale, track->timescale,
                                       AV_ROUND_UP);
     int version = duration < INT32_MAX ? 0 : 1;
     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
@@ -3188,7 +3189,7 @@  static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
                               MOVTrack *track)
 {
     int64_t duration = av_rescale_rnd(calc_samples_pts_duration(mov, track),
-                                      MOV_TIMESCALE, track->timescale,
+                                      mov->movie_timescale, track->timescale,
                                       AV_ROUND_UP);
     int version = duration < INT32_MAX ? 0 : 1;
     int entry_size, entry_count, size;
@@ -3207,7 +3208,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->movie_timescale,
                            track->timescale, AV_ROUND_DOWN);
     version |= delay < INT32_MAX ? 0 : 1;
 
@@ -3242,8 +3243,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 movie timescale units. */
+        av_assert0(av_rescale_rnd(start_dts, mov->movie_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
@@ -3477,7 +3478,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->movie_timescale,
                                                 mov->tracks[i].timescale,
                                                 AV_ROUND_UP);
             if (max_track_len < max_track_len_temp)
@@ -3506,7 +3507,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->movie_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 */
@@ -6115,7 +6116,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->movie_timescale;
     track->par = avcodec_parameters_alloc();
     if (!track->par)
         return AVERROR(ENOMEM);
@@ -6179,8 +6180,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->movie_timescale});
+        pkt->pts = pkt->dts = av_rescale_q(c->start, c->time_base, (AVRational){1,mov->movie_timescale});
         pkt->duration = end - pkt->dts;
 
         if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
@@ -6737,7 +6738,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->movie_timescale;
         }
         if (!track->height)
             track->height = st->codecpar->height;
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index cdbc407..af1ea0b 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -237,6 +237,7 @@  typedef struct MOVMuxContext {
     int write_tmcd;
     MOVPrftBox write_prft;
     int empty_hdlr_name;
+    int movie_timescale;
 } MOVMuxContext;
 
 #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)