[FFmpeg-devel] [mpegaudio_parser] Skip trailing junk data when flushing parser.
nfxjfg at googlemail.com
Mon Feb 26 21:58:15 EET 2018
On Mon, 26 Feb 2018 10:30:26 -0800
Dale Curtis <dalecurtis at chromium.org> wrote:
> On Fri, Feb 23, 2018 at 7:01 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> > this goes the wrong direction.
> > Parsers should not discard data by default. The code we have for tags is a
> > hack.
> > There are many better ways to handle this.
> > Something similar to a AV_PKT_FLAG_CORRUPT set be the parser would be an
> > example. This could then optionally be discarded
> It's not just the trailing tags though, skipping bad data is fundamental to
> how this parser works. Even if we change it so that we mark the last packet
> corrupt, midstream the parser is still designed to skip over everything
> that doesn't look like an mp3 frame. I.e., the data between mp3 frames is
> just dropped wholesale.
> Are you proposing we rework the mp3 parser entirely to not drop any data
> anymore or just not drop the trailing data?
It sounds like the idea is that 1. a parser should never drop data,
even if it's useless data, and 2. the non-mp3 data would go into its
own packet (so that concatenating the packet contents would result in
the original byte stream). This packet would be marked as CORRUPT, so
the user can easily skip it.
I think that's reasonable, but has other problems, such as API
compatibility. Also, sometimes you might want to decode partial packets
(think of silly cases like partially downloaded PNG files). These would
be are as CORRUPT too, but don't contain complete junk data. We would
have to set AVFMT_FLAG_DISCARD_CORRUPT by default to keep the old mp3
behavior (for actual tags), but this would change behavior in other
It'd also feel strange if we'd use the CORRUPT flag for trailing junk,
but not leading junk (like id3v2 tags).
So I'd argue just dropping junk is OK, unless we introduce something
like AV_PKT_FLAG_JUNK (and corresponding AVFMT_FLAG_DISCARD_JUKN). The
discard flag could be enabled by default, but then I'd not be sure
about the situation where this should not be set. Depends what exactly
michaelni is thinking of here, I guess.
(Hopefully my post is helpful instead of piling on the bikeshed.)
More information about the ffmpeg-devel