[FFmpeg-devel] [PATCH] png parser

[Peter Holik]

Michael Niedermayer schrieb:
> On Tue, Jun 23, 2009 at 08:51:12AM +0200, Peter Holik wrote:
>> Michael Niedermayer schrieb:
> [...]
>
>> +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;
>> +    uint32_t *state_ptr;
>> +    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_ptr = &ppc->pc.state64;
>> +
>> +        for (; i < buf_size; i++) {
>> +            *state64_ptr = (*state64_ptr << 8) | buf[i];
>
> using a local state64 likely is faster than the repeated pointer deref
> i suspect gcc fails to do that optim itself ...

done.

>> +            if (*state64_ptr == PNGSIG || *state64_ptr == MNGSIG) {
>> +                i++;
>> +                if (ppc->pc.index) {
>> +                    i -= 8;
>> +                    next = i;
>> +                } else {
>> +                    ppc->pc.frame_start_found = 1;
>> +                    ppc->chunk_index = 1;
>> +                    if (i == 8)
>> +                        break;
>> +                    else if (i > 8) {
>> +                        buf += i - 8;
>> +                        buf_size = 8;
>> +                    } else /* if (i < 8) */
>> +                        buf_size = i;
>> +                }
>> +                ff_combine_frame(&ppc->pc, next, &buf, &buf_size);
>> +                return i;
>
> could you explain why this is not just looking like lets say mpeg4:
>
>     if(!vop_found){
>         for(i=0; i<buf_size; i++){
>             state= (state<<8) | buf[i];
>             if(state == 0x1B6){
>                 i++;
>                 vop_found=1;
>                 break;
>             }
>         }
>     }

png signature is 8 Bytes


> what is the point of all the special cases?


Because this png parser should filter out good png images.

1. Case
Usually you get a buf starting with a png signature, therefor "i" would be 8
and i skip any special handling.

2. Case
There are some headerbytes at the beginning, that i will drop:

        else if (i > 8) {
              buf += i - 8;
              buf_size = 8;

        ff_combine_frame(&ppc->pc, next, &buf, &buf_size);
        return i;

here i copy the read signature into my buffer with ff_combine_frame
(and modified buf to start from signature start)

3. Case
The signature starts in the next buf:

the first buf is copied to my buffer with ff_combine_frame

for the next one i know it - ppc->pc.index != NULL

        if (ppc->pc.index) {
               i -= 8;
               next = i;

        ff_combine_frame(&ppc->pc, next, &buf, &buf_size);
        return i;

and finish my buffer and return, that i have read till to the start of png signature

the next time png_parse is called with a buf starting with a png signature
like 1. Case

4. Case
The signature ends in the next buf and starts in the first buf:

the first buf is copied to my buffer with ff_combine_frame

for the next one i know it - ppc->pc.index != NULL

        if (ppc->pc.index) {
               i -= 8;
               next = i;

        ff_combine_frame(&ppc->pc, next, &buf, &buf_size);
        return i;

But now "next is NEGATIVE"!

i finish my buffer with ff_combine_frame and return a minus value
(that is set to 0 in av_parser_parse2)

now the magic of "ff_combine_frame" - a negative next means "overread"

ff_combine_frame does

for(;next < 0; next++) pc->overread++;


next time png_parse is called with the same buf and "i" would be < 8

 } else /* if (i < 8) */
    buf_size = i;

  ff_combine_frame(&ppc->pc, next, &buf, &buf_size);
  return i;

this time ff_combine_frame fills my buffer with parts of the overread png signature

/* Copy overread bytes from last frame into buffer. */
    for(; pc->overread>0; pc->overread--){
        pc->buffer[pc->index++]= pc->buffer[pc->overread_index++];
    }

and copies the rest of the signature into my buffer.

next time png_parse is called buf starts after the png signature

cu Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: png_parser.diff
Type: text/x-diff
Size: 5779 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090624/bc91c45e/attachment.diff>