[FFmpeg-devel] [PATCH] FLAC parser

Michael Niedermayer michaelni
Wed Aug 18 19:29:25 CEST 2010


On Wed, Aug 18, 2010 at 12:32:08PM +0200, Michael Chinen wrote:
> Hi,
> 
> On Mon, Aug 16, 2010 at 11:40 PM, Justin Ruggles
> <justin.ruggles at gmail.com> wrote:
> [...]
> > Your latest patch looks good. ?I think we might want to wait until the
> > "fix seeking and index generation" patch is applied before committing
> > though. ?One of the main purposes of the FLAC parser is seeking in raw
> > FLAC, which will not work properly without that fix.
> 
> Thanks, understood.
> 
> Also with the new sample accurate seeking tests (commited to soc svn)
> I found some (most) seeks were off by one frame but didn't cause
> complaints since seeking was still to frame boundaries.  The updated
> patch fixes this.
> 
> Michael

>  Makefile      |    1 
>  allcodecs.c   |    1 
>  flac.c        |   87 ++++++++++
>  flac.h        |   23 ++
>  flac_parser.c |  488 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  flacdec.c     |  229 ++++-----------------------
>  6 files changed, 640 insertions(+), 189 deletions(-)
> b335b5896e8dfffd6daa05db96c6f035623c6ee0  0001-Add-FLAC-Parser.patch
> From 0345d12f52966c1c7e064d760ca00515e25b7118 Mon Sep 17 00:00:00 2001
> From: Michael Chinen <mchinen at gmail.com>
> Date: Sun, 8 Aug 2010 17:26:07 +0200
> Subject: [PATCH 1/2] 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 Justin Ruggles' changes from a patch in the same discussion.
> 
> Since the first submission these changes were made:
> fix off by one error where headers that landed at edges of buffers got ignored
> return all buffers including junk frames.
> Removing option to disallow tracking of overlapping sequences as per Justin Ruggles' suggestion.


> Putting in MN's suggestion to concatenate sequential valid CRCs instead of checking them twice.

elaborate on what you do here please


[...]
> +static const int sample_size_table[] = { 0, 8, 12, 0, 16, 20, 24, 0 };
> +
> +static int64_t get_utf8(GetBitContext *gb)
> +{
> +    int64_t val;
> +    GET_UTF8(val, get_bits(gb, 8), return -1;)
> +    return val;
> +}

> -static const int sample_size_table[] =
> -{ 0, 8, 12, 0, 16, 20, 24, 0 };
> -
> -static int64_t get_utf8(GetBitContext *gb)
> -{
> -    int64_t val;
> -    GET_UTF8(val, get_bits(gb, 8), return -1;)
> -    return val;
> -}
> -

code moving should be seperate from functional changes
also the table would fit in uint8_t


[...]
> +/**
> + * Score a header.
> + *
> + * Give FLAC_HEADER_BASE_SCORE points to a frame for existing.
> + * If it has children, (subsequent frames of which the preceding CRC footer
> + * validates against this one,) then take the maximum score of the children,
> + * with a penalty of FLAC_HEADER_CHANGED_PENALTY applied for each change to
> + * bps, sample rate, channels, but not decorrelation mode, or blocksize,
> + * because it can change often.
> + **/
> +static int score_header(FLACHeaderMarker *header)
> +{
> +    FLACHeaderMarker *child;
> +    int dist = 0;
> +    int child_score;
> +
> +    if (header->max_score != FLAC_HEADER_NOT_SCORED_YET)
> +        return header->max_score;
> +
> +    header->max_score = FLAC_HEADER_BASE_SCORE;
> +
> +    /* Check and compute the children's scores. */
> +    child = header->next;
> +    while (1 << dist <= header->crc_valid) {
> +        if (1 << dist++ & header->crc_valid) {

thats the same as 
1 << dist++ == header->crc_valid

and this somehow doesnt look correct


> +            /* Look at the child's frame header info and penalize suspicious
> +               changes between the headers. */
> +            child_score = score_header(child) -
> +                check_header_mismatch(NULL, &header->fi, &child->fi);
> +
> +            if (FLAC_HEADER_BASE_SCORE + child_score > header->max_score) {
> +                /* Keep the child because the frame scoring is dynamic. */
> +                header->best_child = child;
> +                header->max_score  = FLAC_HEADER_BASE_SCORE + child_score;
> +            }
> +        }
> +        child = child->next;
> +    }
> +    return header->max_score;
> +}
> +
> +static void score_sequences(FLACParseContext *fpc)
> +{
> +    FLACHeaderMarker *curr;
> +    int best_score = FLAC_HEADER_NOT_SCORED_YET;
> +    /* First pass to clear all old scores. */
> +    curr = fpc->headers;
> +    while (curr) {
> +        curr->max_score = FLAC_HEADER_NOT_SCORED_YET;
> +        curr            = curr->next;
> +    }

> +    /* Do a second pass to score them all. */
> +    curr = fpc->headers;
> +    while (curr) {

for(curr = fpc->headers; curr; curr = curr->next)


[...]
> +static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
> +                      const uint8_t **poutbuf, int *poutbuf_size,
> +                      const uint8_t *buf, int buf_size)
> +{
> +    FLACParseContext *fpc = s->priv_data;
> +    FLACHeaderMarker *curr;
> +    int nb_headers;
> +    void* new_buffer;
> +
> +    if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
> +        FLACFrameInfo fi;
> +        if (frame_header_is_valid(buf, &fi))
> +            avctx->frame_size = fi.blocksize;
> +        *poutbuf      = buf;
> +        *poutbuf_size = buf_size;
> +        return buf_size;
> +    }
> +
> +    if (fpc->best_header_valid)
> +        return get_best_header(fpc, avctx, poutbuf, poutbuf_size);
> +
> +    /* If a best_header was found last call remove it with the buffer data. */
> +    if (fpc->best_header && fpc->best_header->best_child) {
> +        FLACHeaderMarker *temp;
> +        FLACHeaderMarker *best_child = fpc->best_header->best_child;
> +
> +        curr = fpc->headers;
> +        /* Remove headers in list until the end of the best_header. */
> +        while (curr != best_child) {
> +            if (curr != fpc->best_header) {
> +                av_log(avctx, AV_LOG_DEBUG,
> +                       "dropping low score %i frame header from offset %i "
> +                       "to %i\n",
> +                       curr->max_score, curr->offset, curr->next->offset);
> +            }
> +            temp = curr->next;
> +            av_free(curr);
> +            curr = temp;
> +        }

> +        /* Slide the data down the buffer.
> +           TODO: Make into ring buffer or something cleaner. */
> +        fpc->buffer_size = fpc->buffer_size - best_child->offset;
> +        memmove(fpc->buffer, fpc->buffer + best_child->offset,
> +                fpc->buffer_size);

this doesnt look very fast


> +
> +        /* Fix the offset for the headers remaining to match the new buffer. */
> +        curr = best_child->next;
> +        while (curr) {
> +            curr->offset -= best_child->offset;
> +            curr = curr->next;
> +        }
> +        best_child->offset = 0;
> +        fpc->headers       = best_child;
> +        fpc->best_header = NULL;
> +    } else if (fpc->best_header) {
> +        /* No end frame no need to delete the buffer; probably eof */
> +        FLACHeaderMarker *temp;
> +        curr = fpc->headers;

> +        while (curr != fpc->best_header) {
> +            temp = curr->next;
> +            av_free(curr);
> +            curr = temp;
> +        }

duplicate

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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100818/a471aec9/attachment.pgp>



More information about the ffmpeg-devel mailing list