[FFmpeg-devel] [PATCH] png parser

[Michael Niedermayer]

On Sat, Jun 13, 2009 at 01:54:46PM +0200, Peter Holik wrote:
[...]
> +static int png_parse(AVCodecParserContext *s, AVCodecContext *avctx,
> +                     const uint8_t **poutbuf, int *poutbuf_size,
> +                     const uint8_t *buf, int buf_size)
> +{
> +    ParseContext *pc = s->priv_data;
> +    uint32_t *state_ptr = &pc->state;

> +    uint32_t *chunk_length_ptr = (uint32_t *)&pc->state64;

this looks like you misuse the field


> +    uint32_t *remaining_size_ptr = &pc->overread_index;

thats a very obfuscated way o access a field from the struct


> +    int next = END_NOT_FOUND;
> +    int i = 0;
> +
> +    *poutbuf_size = 0;
> +    if (buf_size == 0)
> +        return 0;
> +
> +    if (!pc->frame_start_found) {
> +        uint64_t *state64_ptr = &pc->state64;
> +
> +        for (; i < buf_size; i++) {
> +            *state64_ptr = (*state64_ptr << 8) | buf[i];
> +            if (*state64_ptr == PNGSIG || *state64_ptr == MNGSIG) {
> +                pc->frame_start_found = 1;
> +                if (++i == 8)
> +                    break;

> +                /* workaround to start packet with a signature */
> +                buf_size = 8;
> +                *state64_ptr = be2me_64(*state64_ptr);
> +                if (ff_combine_frame(pc, next, (const uint8_t **)&state64_ptr, &buf_size) != -1) {

whatever this is supposed to do, it looks very hackish


> +                    next = buf_size;
> +                    goto fail;
> +                }
> +                return i;
> +            }
> +        }
> +    } else
> +        if (*remaining_size_ptr) {
> +            i = FFMIN(*remaining_size_ptr, buf_size);
> +            *remaining_size_ptr -= i;
> +            if (*remaining_size_ptr)
> +                goto flush;
> +            if (pc->frame_start_found == -1) {
> +                next = i;
> +                goto flush;
> +            }
> +        }
> +
> +    for (;pc->frame_start_found && i < buf_size; i++) {
> +        *state_ptr = (*state_ptr << 8) | buf[i];
> +        if (pc->frame_start_found == 4) {
> +                *chunk_length_ptr = AV_RL32(state_ptr) + 4;
> +                if (*chunk_length_ptr > 0x7fffffff) {
> +                    next = buf_size;
> +                    goto fail;
> +                }

i think this discards data?
if so, thats wrong, parsers should not discard any data, just split it the
best they can.
a decoder might very well be able to decode the correct parts of a frame ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090617/8679c714/attachment.pgp>