Message ID | 1616537325-620485-1-git-send-email-andrey.rikunov@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] Increase MOV_TIMESCALE for higher precision | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | fail | Make fate failed |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | warning | Make fate failed |
On 2021-03-24 03:38, Andrey Rikunov wrote: > --- > libavformat/movenc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/movenc.h b/libavformat/movenc.h > index cdbc407..8a152c0 100644 > --- a/libavformat/movenc.h > +++ b/libavformat/movenc.h > @@ -29,7 +29,7 @@ > > #define MOV_FRAG_INFO_ALLOC_INCREMENT 64 > #define MOV_INDEX_CLUSTER_SIZE 1024 > -#define MOV_TIMESCALE 1000 > +#define MOV_TIMESCALE 60000 Instead of hardcoding it, add an option to allow the user to set it. Regards, Gyan
On Wed, Mar 24, 2021 at 7:30 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2021-03-24 03:38, Andrey Rikunov wrote: > > --- > > libavformat/movenc.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/movenc.h b/libavformat/movenc.h > > index cdbc407..8a152c0 100644 > > --- a/libavformat/movenc.h > > +++ b/libavformat/movenc.h > > @@ -29,7 +29,7 @@ > > > > #define MOV_FRAG_INFO_ALLOC_INCREMENT 64 > > #define MOV_INDEX_CLUSTER_SIZE 1024 > > -#define MOV_TIMESCALE 1000 > > +#define MOV_TIMESCALE 60000 > > Instead of hardcoding it, add an option to allow the user to set it. I think if the "Mov requires timescale 1000" assumption is no longer correct, it should be removed and the generic time base based timescale generation utilized instead, which is utilized for MP4-likes. That way by setting time base one can affect it with the standard option. For the general logic (for MP4- likes), I am planning on getting rid of the video time scale option in the following steps: 1. Figure out what the historical reasons for having a video time scale of over 10k are (I think it was mostly to adjust A/V sync with more granularity than frame rate - but we have edit lists for that now?). 2. If those still hold, make a "auto_adjust_timescale" boolean, which would first be on by default. 3. Make video_timescale disable the auto-adjustment of video timescale, and set the time base earlier in the muxer logic (since muxers are allowed to override the AVStreams' time base). 4. Deprecate video_timescale and point people towards -auto_adjust_timescale false/0 -time_base:v "1:NUMBER" I have seen people add various timescale related options to movenc in their trees for audio/subtitles just because it's not clear that we already have time_base :s . Thus this just needs to be cleaned up. And if MOV still needs its timescale override of 1:1000 for some compatibility reasons (?) then "auto_adjust_timescale" can also poke at that. Jan
On 2021-03-24 15:45, Jan Ekström wrote: > On Wed, Mar 24, 2021 at 7:30 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> On 2021-03-24 03:38, Andrey Rikunov wrote: >>> --- >>> libavformat/movenc.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavformat/movenc.h b/libavformat/movenc.h >>> index cdbc407..8a152c0 100644 >>> --- a/libavformat/movenc.h >>> +++ b/libavformat/movenc.h >>> @@ -29,7 +29,7 @@ >>> >>> #define MOV_FRAG_INFO_ALLOC_INCREMENT 64 >>> #define MOV_INDEX_CLUSTER_SIZE 1024 >>> -#define MOV_TIMESCALE 1000 >>> +#define MOV_TIMESCALE 60000 >> Instead of hardcoding it, add an option to allow the user to set it. > if MOV still needs its timescale override of 1:1000 for some To be clear, the MOV_TIMESCALE refers to the timescale written in the mvhd atom in all products of movenc.c. It is distinct from the stream/track timescales. The mvhd value is the basis for the duration field in edit list entries but not media time which is denominated in terms of track timescale ( what video_track_timescale targets but only for video streams). Users in the past have asked for a change (usually to 600) because some Apple products (FCP ?) only work well with 600 for mvhd timescale. There is no strict inter-relation or constraint across mvhd and the trak timescales, so auto_adjust_timescale shouldn't force it in any hardcoded manner. Regards, Gyan
Gyan, thanks for clarification! It is all exactly as you described. I thought to set MOV_TIMESCALE to either 60kHz or 90kHz as both are divisible by most common fps values. And x10 increase is for further precision in conversion to seconds. Say it's needed to write segmentDuration=289.4892 sec in elst to start presentation time from. Using timescale of 1000 we lose 0,0002 sec which turns into pts drift by 6 points for video track timescale of 30kHz. IMHO there is no need to pull MOV_TIMESCALE up to cli arg because there is no need to control it. The only requirement is best precision. What do you think? On Wed, Mar 24, 2021 at 2:42 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > On 2021-03-24 15:45, Jan Ekström wrote: > > On Wed, Mar 24, 2021 at 7:30 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >> > >> > >> On 2021-03-24 03:38, Andrey Rikunov wrote: > >>> --- > >>> libavformat/movenc.h | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/libavformat/movenc.h b/libavformat/movenc.h > >>> index cdbc407..8a152c0 100644 > >>> --- a/libavformat/movenc.h > >>> +++ b/libavformat/movenc.h > >>> @@ -29,7 +29,7 @@ > >>> > >>> #define MOV_FRAG_INFO_ALLOC_INCREMENT 64 > >>> #define MOV_INDEX_CLUSTER_SIZE 1024 > >>> -#define MOV_TIMESCALE 1000 > >>> +#define MOV_TIMESCALE 60000 > >> Instead of hardcoding it, add an option to allow the user to set it. > > if MOV still needs its timescale override of 1:1000 for some > > To be clear, the MOV_TIMESCALE refers to the timescale written in the > mvhd atom in all products of movenc.c. It is distinct from the > stream/track timescales. The mvhd value is the basis for the duration > field in edit list entries but not media time which is denominated in > terms of track timescale ( what video_track_timescale targets but only > for video streams). Users in the past have asked for a change (usually > to 600) because some Apple products (FCP ?) only work well with 600 for > mvhd timescale. There is no strict inter-relation or constraint across > mvhd and the trak timescales, so auto_adjust_timescale shouldn't force > it in any hardcoded manner. > > Regards, > Gyan > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/movenc.h b/libavformat/movenc.h index cdbc407..8a152c0 100644 --- a/libavformat/movenc.h +++ b/libavformat/movenc.h @@ -29,7 +29,7 @@ #define MOV_FRAG_INFO_ALLOC_INCREMENT 64 #define MOV_INDEX_CLUSTER_SIZE 1024 -#define MOV_TIMESCALE 1000 +#define MOV_TIMESCALE 60000 #define RTP_MAX_PACKET_SIZE 1450