[FFmpeg-devel] [PATCH] png parser

[Michael Niedermayer]

On Tue, Jun 30, 2009 at 08:47:48AM +0200, Peter Holik wrote:
> Michael Niedermayer schrieb:
> > On Thu, Jun 25, 2009 at 07:55:08PM +0200, Peter Holik wrote:
> >> Michael Niedermayer schrieb:
> >> > On Thu, Jun 25, 2009 at 02:14:14PM +0200, Peter Holik wrote:
> > [...]
> >
> >
> >> +    for (;ppc->pc.frame_start_found && i < buf_size; i++) {
> >> +        ppc->state = (ppc->state<<8) | buf[i];
> >
> > why is this not using the state variable from ParseContext ?
> 
> ok, now ParseContext state is used

[...]

> +typedef struct PNGParseContext
> +{
> +    ParseContext pc;
> +    uint32_t index;

> +    uint32_t chunk_length;

unneeded, it can be read out of state64, it not needed to be stored in the
context


> +    uint32_t remaining_size;
> +} PNGParseContext;
> +
> +static int png_parse(AVCodecParserContext *s, AVCodecContext *avctx,
> +                     const uint8_t **poutbuf, int *poutbuf_size,
> +                     const uint8_t *buf, int buf_size)
> +{
> +    PNGParseContext *ppc = s->priv_data;
> +    int next = END_NOT_FOUND;
> +    int i = 0;
> +
> +    *poutbuf_size = 0;
> +    if (buf_size == 0)
> +        return 0;
> +
> +    if (!ppc->pc.frame_start_found) {
> +        uint64_t state64 = ppc->pc.state64;
> +        for (; i < buf_size; i++) {
> +            state64 = (state64 << 8) | buf[i];
> +            if (state64 == PNGSIG || state64 == MNGSIG) {
> +                i++;
> +                ppc->pc.frame_start_found = 1;
> +                break;
> +            }
> +        }
> +        ppc->pc.state64 = state64;
> +    } else
> +        if (ppc->remaining_size) {
> +            i = FFMIN(ppc->remaining_size, buf_size);
> +            ppc->remaining_size -= i;

> +            if (ppc->remaining_size)
> +                goto flush;

unneeded, the for loop between here and flush wont execute for i=buf_size


> +            if (ppc->index == -1) {
> +                next = i;
> +                goto flush;
> +            }
> +        }
> +
> +    for (;ppc->pc.frame_start_found && i < buf_size; i++) {
> +        ppc->pc.state = (ppc->pc.state<<8) | buf[i];
> +        if (ppc->index == 3)
> +            ppc->chunk_length = AV_RL32(&ppc->pc.state) + 4;
> +        else if (ppc->index == 7) {
> +            if (AV_RB32(&ppc->pc.state) == MKTAG('I', 'E', 'N', 'D')) {

> +                if (i + ppc->chunk_length >= buf_size) {

i + chunk_length can overflow


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

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20090701/5a0b9813/attachment.pgp>