[FFmpeg-devel] avformat/matroskaenc: Increase time stamp precision to AV_TIME_BASE

From: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>

Increases the time stamp precision to 1 microsecond, the same as
AV_TIME_BASE. This hopefully prevents demuxers from being confused on
the number of frames/samples we actually have per second, and should
mark all future Matroska/WebM content as being constant rate.

Work-around for: 259, 6406, 7927, 8909, 9124.

Signed-off-by: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
---
 libavformat/matroskaenc.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

On 2021-05-19 01:47, michael.dirks@xaymar.com wrote:
> From: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
>
> Increases the time stamp precision to 1 microsecond, the same as
> AV_TIME_BASE. This hopefully prevents demuxers from being confused on
> the number of frames/samples we actually have per second, and should
> mark all future Matroska/WebM content as being constant rate.
>
> Work-around for: 259, 6406, 7927, 8909, 9124.
>
> Signed-off-by: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
> ---
>   libavformat/matroskaenc.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index ad7b0bf2c6..474e44485a 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1917,7 +1917,8 @@ static int mkv_write_header(AVFormatContext *s)
>           return ret;
>       pb = mkv->info_bc;
>   
> -    put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 1000000);
> +    // Set time base to 1 microsecond.
> +    put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 1000000000 / AV_TIME_BASE);
>       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)) {
> @@ -1959,11 +1960,11 @@ static int mkv_write_header(AVFormatContext *s)
>           int64_t metadata_duration = get_metadata_duration(s);
>   
>           if (s->duration > 0) {
> -            int64_t scaledDuration = av_rescale(s->duration, 1000, AV_TIME_BASE);
> +            int64_t scaledDuration = s->duration;
>               put_ebml_float(pb, MATROSKA_ID_DURATION, scaledDuration);
>               av_log(s, AV_LOG_DEBUG, "Write early duration from recording time = %" PRIu64 "\n", scaledDuration);
>           } else if (metadata_duration > 0) {
> -            int64_t scaledDuration = av_rescale(metadata_duration, 1000, AV_TIME_BASE);
> +            int64_t scaledDuration = metadata_duration;
>               put_ebml_float(pb, MATROSKA_ID_DURATION, scaledDuration);
>               av_log(s, AV_LOG_DEBUG, "Write early duration from metadata = %" PRIu64 "\n", scaledDuration);
>           } else {
> @@ -2038,12 +2039,12 @@ static int mkv_write_header(AVFormatContext *s)
>       // after 4k and on a keyframe
>       if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
>           if (mkv->cluster_time_limit < 0)
> -            mkv->cluster_time_limit = 5000;
> +            mkv->cluster_time_limit = 5000000;
>           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 = 1000000;
>           if (mkv->cluster_size_limit < 0)
>               mkv->cluster_size_limit = 32 * 1024;
>       }
> @@ -2724,6 +2725,7 @@ static int webm_query_codec(enum AVCodecID codec_id, int std_compliance)
>   static int mkv_init(struct AVFormatContext *s)
>   {
>       int i;
> +    MatroskaMuxContext *mkv = s->priv_data;
>   
>       if (s->nb_streams > MAX_TRACKS) {
>           av_log(s, AV_LOG_ERROR,
> @@ -2752,10 +2754,13 @@ static int mkv_init(struct AVFormatContext *s)
>       }
>   
>       for (i = 0; i < s->nb_streams; i++) {
> -        // ms precision is the de-facto standard timescale for mkv files
> -        avpriv_set_pts_info(s->streams[i], 64, 1, 1000);
> +        // Use µs precision (same as FFmpeg)
> +        avpriv_set_pts_info(s->streams[i], 64, 1, AV_TIME_BASE);
>       }
>   
> +    // Scale the configured cluster_time_limit to microseconds.
> +    mkv->cluster_time_limit = av_rescale(mkv->cluster_time_limit, 1000, AV_TIME_BASE);
> +
>       return 0;
>   }
>   

This is the patch for release/4.2, I should have added that to the commit message.

On 5/18/2021 8:47 PM, michael.dirks@xaymar.com wrote:
> From: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
> 
> Increases the time stamp precision to 1 microsecond, the same as
> AV_TIME_BASE. This hopefully prevents demuxers from being confused on
> the number of frames/samples we actually have per second, and should
> mark all future Matroska/WebM content as being constant rate.
> 
> Work-around for: 259, 6406, 7927, 8909, 9124.
> 
> Signed-off-by: Michael Fabian 'Xaymar' Dirks <info@xaymar.com>
> ---
>   libavformat/matroskaenc.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index ad7b0bf2c6..474e44485a 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1917,7 +1917,8 @@ static int mkv_write_header(AVFormatContext *s)
>           return ret;
>       pb = mkv->info_bc;
>   
> -    put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 1000000);
> +    // Set time base to 1 microsecond.
> +    put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 1000000000 / AV_TIME_BASE);

You should make this an user set AVOption, like it was done with movenc, 
keeping the existing value as the default.

Also, in your other mail you mentioned this applies to 4.2. Patches 
should be made for the master branch. Only regression and bug fixes are 
backported to releases, and this is a new feature/behavior change.

>       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)) {
> @@ -1959,11 +1960,11 @@ static int mkv_write_header(AVFormatContext *s)
>           int64_t metadata_duration = get_metadata_duration(s);
>   
>           if (s->duration > 0) {
> -            int64_t scaledDuration = av_rescale(s->duration, 1000, AV_TIME_BASE);
> +            int64_t scaledDuration = s->duration;
>               put_ebml_float(pb, MATROSKA_ID_DURATION, scaledDuration);
>               av_log(s, AV_LOG_DEBUG, "Write early duration from recording time = %" PRIu64 "\n", scaledDuration);
>           } else if (metadata_duration > 0) {
> -            int64_t scaledDuration = av_rescale(metadata_duration, 1000, AV_TIME_BASE);
> +            int64_t scaledDuration = metadata_duration;
>               put_ebml_float(pb, MATROSKA_ID_DURATION, scaledDuration);
>               av_log(s, AV_LOG_DEBUG, "Write early duration from metadata = %" PRIu64 "\n", scaledDuration);
>           } else {
> @@ -2038,12 +2039,12 @@ static int mkv_write_header(AVFormatContext *s)
>       // after 4k and on a keyframe
>       if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
>           if (mkv->cluster_time_limit < 0)
> -            mkv->cluster_time_limit = 5000;
> +            mkv->cluster_time_limit = 5000000;
>           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 = 1000000;
>           if (mkv->cluster_size_limit < 0)
>               mkv->cluster_size_limit = 32 * 1024;
>       }
> @@ -2724,6 +2725,7 @@ static int webm_query_codec(enum AVCodecID codec_id, int std_compliance)
>   static int mkv_init(struct AVFormatContext *s)
>   {
>       int i;
> +    MatroskaMuxContext *mkv = s->priv_data;
>   
>       if (s->nb_streams > MAX_TRACKS) {
>           av_log(s, AV_LOG_ERROR,
> @@ -2752,10 +2754,13 @@ static int mkv_init(struct AVFormatContext *s)
>       }
>   
>       for (i = 0; i < s->nb_streams; i++) {
> -        // ms precision is the de-facto standard timescale for mkv files
> -        avpriv_set_pts_info(s->streams[i], 64, 1, 1000);
> +        // Use µs precision (same as FFmpeg)
> +        avpriv_set_pts_info(s->streams[i], 64, 1, AV_TIME_BASE);
>       }
>   
> +    // Scale the configured cluster_time_limit to microseconds.
> +    mkv->cluster_time_limit = av_rescale(mkv->cluster_time_limit, 1000, AV_TIME_BASE);
> +
>       return 0;
>   }
>   
>

On 2021-05-19 02:06, James Almer wrote:
>
> You should make this an user set AVOption, like it was done with movenc, keeping the existing value as the default.
>
I will adjust it to do that.
> Also, in your other mail you mentioned this applies to 4.2. Patches should be made for the master branch. Only regression and bug fixes are backported to releases, and this is a new feature/behavior change.
>
Thanks for clearing that up. I was unsure if this was a backportable fix, or considered a new feature, since it had no impact on demuxers that were already correct, and only fixed those that failed to detect constant rate from FFmpeg muxed MKV/WebM files.