On 2021-05-21 00:37, James Almer wrote:
On 5/20/2021 7:33 PM, Michael Fabian 'Xaymar' Dirks wrote:
On 2021-05-21 00:30, Michael Fabian 'Xaymar' Dirks wrote:
On 2021-05-21 00:25, James Almer wrote:
On 5/20/2021 7:20 PM, Lynne wrote:
May 20, 2021, 20:08 by jamr...@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.di...@xaymar.com wrote:

From: Michael Fabian 'Xaymar' Dirks <michael.di...@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.di...@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.
   +@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.

1us is incompatible with the default value for cluster_time_limit. It's in fact 
incompatible with any value for it, since it will force one block per cluster.
If the user wants that, then they are free to set 1us, but the default of one 
option should not invalidate the default of another. It's the entire point the 
warning exists for and is printed.


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.

Good catch.

I may be misreading the WebM specification, but it only says it SHOULD be set 
1.000.000 nanoseconds, not MUST, which going by the rest of the wording does 
not mean it is required, just very very recommended. Perhaps a warning for WebM 
would be better than an error-exit?

I was misreading the specification. Shouldn't try to read technical 
documentation when tired. Will add the error-exit, ignore the patch sent before 
Lynnes message arrived here.

No, apparently you were right. In https://www.webmproject.org/docs/container/ there's a 
line that says "The TimecodeScale element SHOULD be set to a default of 1.000.000 
nanoseconds.", which means it's recommended but not required. Or is there another 
source for the spec?

Also, the spec uses TimecodeScale and TimestampScale interchangeably, which 
makes things a bit confusing.

Directly above it:

Muxers should treat all guidelines marked SHOULD in this section as MUST. This 
will foster consistency across WebM files in the real world.

I missed it as well as it just didn't look important at first.



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.

Sorry, i just default to that in general. Will try to not make that mistake 
again.


--
Sincerely | Mit freundlichen Grüßen

Michael Fabian 'Xaymar' Dirks
Software Designer & Engineer

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to