[FFmpeg-devel] [PATCH] FLAC parser

[Michael Niedermayer]

On Fri, Oct 15, 2010 at 12:06:40PM +0200, Michael Chinen wrote:
> Hi,
> 
> On Thu, Oct 14, 2010 at 10:25 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Oct 13, 2010 at 02:26:13AM +0200, Michael Chinen wrote:
[...]
> >
> >
> >
> >> + ? ? ? ?mid->crc_valid |= 1 << distance;
> >> + ? ?} else {
> >> + ? ? ? ?av_log(fpc->avctx, AV_LOG_DEBUG,
> >> + ? ? ? ? ? ? ? "update_sequence CRC failed at dist %i from %i to %i\n",
> >> + ? ? ? ? ? ? ? distance, mid->offset, end->offset);
> >> + ? ?}
> >> +}
> >> +
> >> +static void update_sequences(FLACParseContext *fpc, int start_index,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? int start_distance, FLACHeaderMarker *end)
> >> +{
> >> + ? ?FLACHeaderMarker *mid = fpc->headers;
> >> +
> >> + ? ?while (start_index-- > 0)
> >> + ? ? ? ?mid = mid->next;
> >> +
> >> + ? ?update_sequences_header(fpc, mid, end, start_distance - 1);
> >> +}
> >> +
> >
> >> +static int find_new_headers(FLACParseContext *fpc, int search_start)
> >> +{
> >> + ? ?FLACFrameInfo fi;
> >> + ? ?FLACHeaderMarker *end;
> >> + ? ?int i, search_end, end_offset = -1, size = 0;
> >> + ? ?uint8_t *buf;
> >> + ? ?fpc->nb_headers_found = 0;
> >> +
> >> + ? ?/* Search for a new header of at most 16 bytes. */
> >> + ? ?search_end = av_fifo_size(fpc->fifo_buf) - (MAX_FRAME_HEADER_SIZE - 1);
> >> + ? ?buf = flac_fifo_read(fpc, search_start,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? search_end + (MAX_FRAME_HEADER_SIZE - 1) - search_start,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? &fpc->wrap_buf, &fpc->wrap_buf_allocated_size);
> >> + ? ?for (i = 0; i < search_end - search_start; i++) {
> >> + ? ? ? ?if ((AV_RB16(buf + i) & 0xFFFE) == 0xFFF8 &&
> >
> > This code does not need a flat buffer.
> > only frame_header_is_valid needs a max of 16 bytes flat buffer
> 
> I tried using a 16 bytes max wrap buffer here that is read into inside
> the for loop, but this was much slower.  This for loop gets iterated
> over each byte that goes in so having things inside the for loop has
> penalties.  (This is therefore not included in the patches but is easy to add.)

you should search for a valid start code and then load the 16 byte not load 16
byte for every byte.


[...]
> @@ -140,12 +138,36 @@ static uint8_t* flac_fifo_read(FLACParseContext *fpc, int offset, int len,
>      return *wrap_buf;
>  }
>  
> +/**
> + * return a pointer in the fifo buffer where the offset starts at until
> + * the wrap point or end of request.
> + * len will contain the valid length of the returned buffer.
> + * A second call to flac_fifo_read (with new offset and len) should be called
> + * to get the post-wrap buf if the returned len is less than the requested.
> + **/
> +static uint8_t* flac_fifo_read(FLACParseContext *fpc, int offset, int *len)
> +{
> +    AVFifoBuffer *f   = fpc->fifo_buf;
> +    uint8_t *start    = f->rptr + offset;
> +
> +    if (start > f->end)
> +        start -= f->end - f->buffer;

> +    if (start > f->end) {
> +        av_log(fpc->avctx, AV_LOG_ERROR, "fifo read request size larger than fifo\n");
> +        *len = 0;
> +        return NULL;
> +    }

how can this occur?



> +    *len = FFMIN(*len, f->end - start);
> +    return start;
> +}
> +
>  static void update_sequences_header(FLACParseContext *fpc,
>                                      FLACHeaderMarker *mid,
>                                      FLACHeaderMarker *end, int distance)
>  {
>      FLACHeaderMarker *child = mid;
> -    int dist_from_start = 0;
> +    int dist_from_start = 0, read_len;
> +    uint32_t crc;
>      uint8_t *buf;
>      /* CRC verify the children first to find out where they connect to */
>      if (!mid->next)
> @@ -166,10 +188,17 @@ static void update_sequences_header(FLACParseContext *fpc,
>  
>      /* Set a bit showing the validity at this distance if CRC is ok.
>         (distance variable is really one less than linked list distance) */
> -    buf = flac_fifo_read(fpc, mid->offset, end->offset - mid->offset,
> -                         &fpc->crc_buf, &fpc->crc_buf_allocated_size);
> -    if (!av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0,
> -                buf, end->offset - mid->offset)) {
> +    read_len = end->offset - mid->offset;
> +    buf = flac_fifo_read(fpc, mid->offset, &read_len);
> +    crc = av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf, read_len);
> +    read_len = (end->offset - mid->offset) - read_len;
> +
> +    if (read_len) {
> +        buf = flac_fifo_read(fpc, end->offset - read_len, &read_len);
> +        crc = av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf, read_len);
> +    }

id say, this code doesn work


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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101015/b867f5f3/attachment.pgp>