[FFmpeg-devel] [PATCH 2/3] lavf/mkvtimestamp_v2: review implementation to match mkvextract behavior

[Stefano Sabatini]

On date Saturday 2024-04-20 18:47:58 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
> > On date Saturday 2024-04-20 15:18:39 +0200, Andreas Rheinhardt wrote:
> >> Stefano Sabatini:
> >>> Harmonize internal implementation with the mkvextract behavior:
> >>> - print PTS in place of DTS values
> >>> - ignore NOPTS values
> >>> - sort PTS values
> >>> ---
> >>>  libavformat/mkvtimestamp_v2.c | 69 +++++++++++++++++++++++++++++++++--
> >>>  1 file changed, 65 insertions(+), 4 deletions(-)
[...]
> >> 3. I still don't think that this muxer should exist at all.
> > 
> > I tend to agree. But:
> > 
> > We don't really know why this weird muxer was added, today we have
> > better tools for that (we could use ffprobe, or even a bitstream
> > filter similar to showinfo to get the same result), but if it was
> > added probably there was a reason. So my plan is to make the format a
> > bit more useful (DTS => PTS+sort), and possibly deprecate it and point
> > to better available tools and drop this in two major releses.
> > 
> > I don't think the point of the format was to really make the behavior
> > exactly equal to mkvextract, the thing with TimeScale is probably not
> > very important, at least for the original author that was not really
> > an issue, he was probably only looking for some way to dump timestamps
> > and took free inspiration from mkvtoolnix.
> > 
> 
> This thing has been added in 1c5670dbb204369477ee1b5d967f9e8b4f4a33b8. I
> can't find any discussion in the mailing list archives for this, but the
> file description was "extract pts as timecode v2, as defined by
> mkvtoolnix" at the time. Furthermore, its author is the one who started
> the Matroska muxer. So I think its aim was really to mimic mkvextract.
> (I am not certain wrt MKVToolNix handling of fractional millisecond; old
> versions of mkvextract may really have simply rounded/truncated to
> milliseconds.)
> 
> > If this is true, we might point that this format is not exactly
> > equivalent to timestamp_v2 in the doc.
> 
> Which makes this thing even more pointless.

I'm not against removing this muxer, but it is something we should do?
Removing a working component (even if suboptimal) even without
deprecation. Probably if there is agreement about this, especially
given that there are better alternatives at this time.

If not, I can fix the missing bits so we have a better implementation,
but we might want to deprecated and drop later.