[FFmpeg-devel] [PATCH] avformat/matroskaenc: Allow changing the time stamp precision via option

[Lynne]

May 20, 2021, 20:08 by jamrial at gmail.com:

> On 5/20/2021 3:00 PM, Michael Fabian 'Xaymar' Dirks wrote:
>
>> On 2021-05-20 19:26, James Almer wrote:
>>
>>> On 5/20/2021 2:18 PM, michael.dirks at xaymar.com wrote:
>>>
>>>> From: Michael Fabian 'Xaymar' Dirks <michael.dirks at xaymar.com>
>>>>
>>>> Adds "timestamp_precision" to the available option for Matroska/WebM
>>>> muxing. The option enables users and developers to change the precision
>>>> of the time stamps in the Matroska/WebM container up to 1 nanosecond,
>>>> which can aid with the proper detection of constant and variable rate
>>>> content.
>>>>
>>>> Work-around fix for: 259, 6406, 7927, 8909 and 9124.
>>>>
>>>> Signed-off-by: Michael Fabian 'Xaymar' Dirks <michael.dirks at xaymar.com>
>>>> ---
>>>>   doc/muxers.texi           |  8 ++++++++
>>>>   libavformat/matroskaenc.c | 28 ++++++++++++++++++++--------
>>>>   2 files changed, 28 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>>>> index e1c6ad0829..d9f7badae1 100644
>>>> --- a/doc/muxers.texi
>>>> +++ b/doc/muxers.texi
>>>> @@ -1583,6 +1583,14 @@ bitmap is stored bottom-up. Note that this option does not flip the bitmap
>>>>   which has to be done manually beforehand, e.g. by using the vflip filter.
>>>>   Default is @var{false} and indicates bitmap is stored top down.
>>>>   + at item timestamp_precision
>>>> +Sets the timestamp precision up to 1 nanosecond for Matroska/WebM, which can
>>>> +improve detection of constant rate content in demuxers. Note that some poorly
>>>> +implemented demuxers may require a timestamp precision of 1 millisecond, so
>>>> +increasing it past that point may result in playback issues. Higher precision
>>>> +also reduces the maximum possible timestamp significantly.
>>>> +Default is @var{1/1000000000} (1 nanosecond).
>>>>
>>>
>>> Like i said, the default must be the one defined in the spec. This version not only would need FATE test updates, it also like you described makes all new files by default have a lot of overhead from having one block per cluster.
>>>
>>
>> I am aware of what you said and I am also aware of Lynne said. I do not know who has the actual final say in this, all I know is that the maintainers for matroskaenc.c are "David Conrad" and "Andreas Rheinhardt" - both of which have not commented on this yet.
>>
>
> Lynne agreed on IRC that it can remain as 1ms.
>
If there's a chance it could be changed to 1us (the common internal lavf timebase),
I'd like for it to be, but not without a small partial study of compatibility with common
implementations.
Specifically, the one case I've heard where non-1ms timebases had issues was with VLC's
demuxer, where apparently giving an external .mks file as subtitles with a non-1ms timebase
caused the player to OOM. If this is/was still true, for shame, for shame.

Also, the patch is missing some error-checking. WebM hard-requires a 1ms timebase.
You should likely error out if the demuxer acts in WebM mode and the timebase is set
to a different value.

As for the whole "1ms is good enough for everything!", well, it's not for 960fps VFR video
and that's all I'm going to say. Where you get 960fps VFR videos? Smartphones.
Even a cheap one from 5 years ago can shoot at 240fps. They're really common, and
depending on what strategy you use to convert the video, rounding jitter can result in some
horrible slow-mo conversions.


> Probably should have said it here, but i guess neither him
>
she. Try to remember, this hasn't been the first time.