[FFmpeg-devel] [PATCH] mlp_parser: fetch a new timestamp when major sync is found

[Jan Ekström]

On Mon, Mar 7, 2022 at 7:10 PM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Mon, Mar 07, 2022 at 12:32:59AM +0200, Jan Ekström wrote:
> > From: Hendrik Leppkes <h.leppkes at gmail.com>
> >
> > Decoding can only start on a major sync
>
> > and the parser discards
> > any data received until that point.
>
> Thats a bug and its inconsistant
> For example video codec parsers do not drop frames until a keyframe
> And droping that in a video codec would if someone suggested it
> lead to people opposing as that would drop actual data
> that can partly be decoded. Its better to drop it in the decoder
> if its unwanted.
> So why is this considered to be "ok" in audio codecs?
> Because we dont visually see that data is lost ?
>

For the record, I am not advocating for this behavior in this parser,
nor is this patch adding the dropping. The dropping is and has already
been there.

Rather this is just the result of the following steps:

1. I improve certain parser calling code in an API client (mpv) to
actually utilize the returned timestamps due to certain formats such
as E-AC-3 having buffering/delay.
2. Some of the samples I have at hand were badly cut matroska files
from early 2010s where the muxer expected all packets to be random
access points.
3. I notice that while the E-AC-3 samples' timestamps are now correct,
the TrueHD samples' timestamps are now such that you get a jump
between the first returned data, and the rest of the returned data.
4. After staring at the matroska packets' sizes and timestamps on the
container level, I figure out that the parser is just dropping packets
until the first random access point packet is found, and then failing
to update the returned timestamp.
5. Thus I open https://trac.ffmpeg.org/ticket/9428 .
6. nev is nice enough to figure out how to make the parser actually
update the returned timestamp.
7. After the copyinkf stuff gets applied to master, I write a test for
it and reword the commit message, send the fix to the mailing list.

I do not have experience with the parser, but at least this fixes an
existing bug that I noticed when properly utilizing the parsing API :)
.

> So let me show it vissually :)
> i deleted frame 1, we have 3 decoder outputs
> 1. complete data
> 2. no frame 1 but instead frame 17 duplicated in frame 1s place
> 3. no frame 1
>
> The decoder produces a few errors in case 2 but there seems more
> data coming out which seems resembling the data that is droped
> its not 1:1 identical but it looks like there is recoverable data
> see attached screenshoot from audacity
> Maybe its a coincidence
>
> but i dont think we should just build on top of this droping
> logic without some argumentation why this droped data can really
> not be used ever in any way (it may become harder to fix it the
> more is build on top)
>
> thx
>

I'm OK with the parser being changed in stead, but unfortunately I do
not have the brain power/time to start looking into it myself.

For now I was happy enough to get a fix for the returned timestamps
posted on the mailing list, and have written a test for it.

Jan