[FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

[James Almer]

On 1/13/2021 4:58 PM, Thierry Foucu wrote:
> HI Lynne
> 
> On Wed, Jan 13, 2021 at 10:30 AM Lynne <dev at lynne.ee> wrote:
> 
>> Jan 13, 2021, 18:46 by tfoucu at gmail.com:
>>
>>> By default the time code scale in a MKV file in millisecond. With this
>>> option we can set the time code scale to microsecond or nanoseconds for
>>> very high frame rate.
>>> ---
>>>   libavformat/matroskaenc.c | 11 +++++++----
>>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>> index 233c472b8f..cfad6a4693 100644
>>> --- a/libavformat/matroskaenc.c
>>> +++ b/libavformat/matroskaenc.c
>>> @@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
>>>   int                 default_mode;
>>>
>>>   uint32_t            segment_uid[4];
>>> +
>>> +    int64_t             timecodescale;
>>>   } MatroskaMuxContext;
>>>
>>>   /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
>>> @@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
>>>   return ret;
>>>   pb = mkv->info.bc;
>>>
>>> -    put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 1000000);
>>> +    put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
>>>   if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
>>>   put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
>>>   if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
>>> @@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
>>>   // after 4k and on a keyframe
>>>   if (IS_SEEKABLE(pb, mkv)) {
>>>   if (mkv->cluster_time_limit < 0)
>>> -            mkv->cluster_time_limit = 5000;
>>> +            mkv->cluster_time_limit = 5*(1000000000/mkv->timecodescale);
>>>   if (mkv->cluster_size_limit < 0)
>>>   mkv->cluster_size_limit = 5 * 1024 * 1024;
>>>   } else {
>>>   if (mkv->cluster_time_limit < 0)
>>> -            mkv->cluster_time_limit = 1000;
>>> +            mkv->cluster_time_limit = 1*(1000000000/mkv->timecodescale);
>>>   if (mkv->cluster_size_limit < 0)
>>>   mkv->cluster_size_limit = 32 * 1024;
>>>   }
>>> @@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
>>>   }
>>>
>>>   // ms precision is the de-facto standard timescale for mkv files
>>> -        avpriv_set_pts_info(st, 64, 1, 1000);
>>> +        avpriv_set_pts_info(st, 64, 1, 1000000000/mkv->timecodescale);
>>>
>>>   if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
>>>   if (mkv->mode == MODE_WEBM) {
>>> @@ -2795,6 +2797,7 @@ static const AVOption options[] = {
>>>   { "infer", "For each track type, mark the first track of disposition
>> default as default; if none exists, mark the first track as default.", 0,
>> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS,
>> "default_mode" },
>>>   { "infer_no_subs", "For each track type, mark the first track of
>> disposition default as default; for audio and video: if none exists, mark
>> the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
>> DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
>>>   { "passthrough", "Use the disposition flag as-is", 0,
>> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
>> "default_mode" },
>>> +    { "timecodescale", "Time code scale for all tracks in nanoseconds",
>> OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 1000000 }, 1,
>> 1000000000, FLAGS },
>>>   { NULL },
>>>   };
>>>
>>
>> This, x10000000000000000!
>> Can we make it 1ns by default? Or maybe autodetect the highest precision
>> timebase of all streams and use it?
>>
> 
> The MKV spec seems to indicate a default value of ms. I did not want to
> change it in this patch.
> 
> 
>> Let's not keep generating garbage files with shitty rounding that players
>> have to work around to recover the original timestamp based on framerate
>> to reduce jitter (mpv does this!).
>>
> 
> +1 on this, but I'm worry, we could break a lot of decoder which may not
> follow the correct timecodescale (Which we found out in our system a while
> back)
> 
> 
>> Also WebM mode should disable this, because the spec is dumb and specifies
>> a 1ms precision for no good reason.
>>
> 
> If we keep the default value of ms, then this will not change any webm
> unless a user decides to generate a Webm file and set this flag.
> Do you want me to log an error saying that using this flag to set something
> other than ms for webm is incorrect?

The webm spec states "The TimecodeScale element SHOULD be set to a 
default of 1.000.000 nanoseconds.", so it's not forbidden per se.
Does libwebm handle files with a different TimecodeScale than the 
default? If so, then it's fine. Otherwise, I agree we should probably 
error out.

> 
> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 
> 
>