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

[James Almer]

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. Probably should have said 
it here, but i guess neither him or i thought you'd send a new version. 
Sorry for the confusion.

> 
>>
>> I'll apply the previous version later if no one has any comment about 
>> the implementation, or the option name (I'd prefer timestamp_scale 
>> instead, following the element name).
> 
> I chose "timestamp_precision" over "timestamp_scale", as the latter has 
> different meanings when read. "timestamp_precision" directly implies 
> that the option modifies how accurate the time stamps will be, while 
> "timestamp_scale" has no such meaning to readers.

The option name can be timestamp_scale whereas the description can 
remain as you wrote it. But i don't have a strong opinion about it.