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

[Andreas Rheinhardt]

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).

- Andreas