[FFmpeg-devel] [PATCH 05/10] avformat/matroskadec: Remove non-incremental parsing of clusters

Michael Niedermayer michael at niedermayer.cc
Mon Mar 11 21:14:03 EET 2019


On Sun, Mar 10, 2019 at 11:03:00PM +0000, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Fri, Mar 08, 2019 at 10:25:59AM +0100, Andreas Rheinhardt wrote:
> >> When the new incremental parser was introduced, the old parser was
> >> kept, because the new parser was unable to handle the way SSA packets
> >> are put into Matroska. But since 2014 (since
> >> c7d8dbad14ed5fa3c217a4fc1790021d6c0b6416) this is no longer needed, so
> >> that the old parser can be completely removed.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at googlemail.com>
> >> ---
> >>  libavformat/matroskadec.c | 72 ++++++---------------------------------
> >>  1 file changed, 10 insertions(+), 62 deletions(-)
> > 
> > This affects seeking:
> > 
> > libavformat/tests/seek issues/388/Matrix.Reloaded.Trailer-640x346-XviD-1.0beta2-HE_AAC_subtitled.mkv -duration 400 >oldseek
> > 
> > The file appears to be available here:
> > https://samples.ffmpeg.org/Matroska/matrix/Matrix.Reloaded.Trailer-640x346-XviD-1.0beta2-HE_AAC_subtitled.mkv
> > 
> > --- oldseek	2019-03-08 23:08:21.380042329 +0100
> > +++ newseek	2019-03-08 23:08:02.048041745 +0100
> > @@ -8,7 +8,7 @@
> >  ret: 0         st:13 flags:1 dts: 86.750000 pts: 86.750000 pos:     -1 size: 50436
> >  ret:-1         st: 1 flags:0  ts: 250.577000
> >  ret: 0         st: 1 flags:1  ts: 13.471000
> > -ret: 0         st:13 flags:1 dts: 1.776000 pts: 1.776000 pos:     -1 size: 50436
> > +ret: 0         st:13 flags:1 dts: 0.000000 pts: 0.000000 pos:     -1 size: 50436
> >  ret:-1         st: 2 flags:0  ts: 176.365000
> >  ret: 0         st: 2 flags:1  ts: 339.259000
> >  ret: 0         st:13 flags:1 dts: 145.080000 pts: 145.080000 pos:     -1 size: 50436
> > 
> This is not unexpected. The reason is that the old parser always
> parses a whole cluster at once while the new parser parses clusters
> incrementally, i.e. one block (I use block as a general term for
> SimpleBlock or BlockGroup) at a time. The goal of incremental parsing
> was reduced latency (and with my patch #6 one also achieves a
> reduction of memory used and an increase in performance as well).
> 
> With the old parser, the very first cluster gets parsed during
> avformat_find_stream_info() and index entries were created for all the
> keyframes contained therein (the cues only contain entries for the
> main video track, so this only affects the other tracks). The 1.776
> pts corresponds to the block with the highest timestamp for track #1
> (or track #2 in Matroska's counting) in the first cluster (with a
> timestamp of 1776ms).
> 
> With the incremental parser, only one block of the audio track gets
> parsed during avformat_find_stream_info() (it has a timestamp of 0)
> and so only one index entry gets created for it.
> 
> So when the seek based upon the audio track is performed, the used
> index point has a timestamp of either 0ms or 1776ms. Then the
> current_dts is updated based upon this timestamp.
> 
> Now this file has an attached picture which gets translated into its
> own track and upon every seek this picture will be put into the
> raw_packet_buffer from which it will be read by ff_read_packet as the
> first packet after each seek. Given that this packet doesn't have
> timestamps, the timestamp will be set equal to current_dts (in
> compute_pkt_fields()). Hence the discrepancy.
> 
> Btw: Because of things like this, a fate test had to be changed during
> the initial introduction of incremental parsing (in
> 8336eb6f85e4b94b9c198b16bd0ac4178f4dba86).

Sounds like undesirable behavior if not a bug

The seek request is to 13.471000 and the result should not depend on
what has been parsed or not prior to the seek request.
also if a packet can be produced for 1.776000 which adequatly fullfills
the seek reuest that is better than a more distant one as that would
increase latency experienced by the user

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190311/637161dc/attachment.sig>


More information about the ffmpeg-devel mailing list