[FFmpeg-devel] [PATCH] FLAC parser

[Justin Ruggles]

Hi,

Michael Chinen wrote:

> Hi,
> 
> This FLAC parser takes the suggestions from a thread from another FLAC
> parser patch submitted by Jason Ruggles in March 2009[1].
> Currently it stores 20 headers (8 bit crc verified) and finds all
> possible (16 bit footer) crc-verified sequences within a neighbor
> distance of 4.
> It penalizes sequences that have changes in sample rate, bit depth,
> and channel arrangement.
> The settings probably need some twiddling.
> 
> Seeking seems to work with it (with my av_build_index version in soc
> svn as well).
> 
> I used the modifications to flacdec.c (to remove the bytestream and
> make a function extern) from Jason's patch, but flac_parser.c is new.
> 
> The second patch is just to make ffplay keep calling av_read_frame
> after the ByteIOContext has reach EOF.
> 
> Michael
> 
> [1] http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2009-March/066533.html

> From 6134833f75c83beb306936b62e789c62a85ac2a2 Mon Sep 17 00:00:00 2001
> From: Michael Chinen <mchinen at gmail.com>
> Date: Wed, 14 Jul 2010 02:58:53 +0200
> Subject: [PATCH 1/4] Add FLAC parser
>  Based on mailing list discussion from march 09.
>  flac_parser.c verifies all possible chains within a certain header radius and scores them based on changes in sample rate, bit depth, channels.
>  flacdec.c uses Jason Ruggles' changes from a patch in the same discussion.

My name is Justin Ruggles. :)

> ---
>  libavcodec/Makefile      |    1 +
>  libavcodec/allcodecs.c   |    1 +
>  libavcodec/flac.h        |   12 ++
>  libavcodec/flac_parser.c |  362 ++++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/flacdec.c     |  110 +++-----------
>  5 files changed, 398 insertions(+), 88 deletions(-)
>  create mode 100644 libavcodec/flac_parser.c


Some general comments:

1) I haven't gone too far into why this happens, but it doesn't seem to
work reliably.  Using 20 headers there should be no reason why any
frames are not detected properly, but in 2 random single-song samples I
tested I got instances of multiple frames being joined together (i.e.
skipping a valid header).  The decoder can handle this, but it should
not be occuring.

2) It looks like your parser is potentially skipping data.  Parsers
should pass-through all data.  If your next best header is not at the
start of the buffer, output up to that point, then return your good
frame in the next call.  It could mean a damaged stream or partial
stream or whatever, but the decoder should decide what to do with it.

3) Regarding the optional disallowing of overlapping sequences, the
parser should always evaluate all sequences and choose the one with the
best score.  "Fake frames" will very likely have different parameters. 2
fake frames in a row or a fake frame joining into a real frame is
extremely unlikely, 2 sequential fake frames with matching parameters
even more so.  So I don't see where there would be much danger for
misdetection if you use appropriate scoring.

4) I'll save style comments until the other issues are addressed.


Thanks for picking this up.

Cheers,
Justin