[FFmpeg-devel] [PATCH] png parser

[Peter Holik]

Michael Niedermayer schrieb:
> On Thu, Jun 25, 2009 at 02:14:14PM +0200, Peter Holik wrote:
>> Michael Niedermayer schrieb:
>> > On Thu, Jun 25, 2009 at 12:59:07PM +0200, Peter Holik wrote:
>> >> Michael Niedermayer schrieb:
>> >> > On Wed, Jun 24, 2009 at 05:54:17PM +0200, Peter Holik wrote:
>> >> >> Michael Niedermayer schrieb:
>> >> > [...]
>> >> >> >> +            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
>> >> >
>> >> > s/state/state64/ in the example, the question stays the same
>> >> >
>> >> >
>> >> >>
>> >> >>
>> >> >> > what is the point of all the special cases?
>> >> >>
>> >> >>
>> >> >> Because this png parser should filter out good png images.
>> >> >
>> >> > thats not the job of a parser, besides good is a fuzzy term, a
>> >> > png with a bit fliped in the signature is pretty good and easy to decode
>> >> > your code would drop it i think.
>> >>
>> >> Following parser also drops:
>> >>
>> >> mpegaudio_parser.c
>> >> mlp_parser.c
>> >> dvbsub_parser.c
>> >> pnm_parser.c
>> >>
>> >> why may my parser not drop?
>> >
>> > i have explained it already
>> > because it drops things that can still be decoded easily in principle
>> > the others you quote may very well be buggy
>> >
>> >
>> >>
>> >> Anyway fighting against windmills is meaningless.
>> >>
>> >> Here now a version without dropping anything and using 32 Bits for
>> >> signature check instead of 64 Bits.
>> >
>> > why 32bit?
>> > isnt 64bit simpler for 64bit sigs?
>>
>> suggested by "Kostya" <kostya.shishkov at gmail.com>
>>
>> I also would prefere 64bits
>>
>> What shall i do now?
>
> i cant see a reason or advantage to use 32bit so use 64bit
> unless someome, kostya? can explain what advantage 32bit would
> have
> (and no it shouldnt matter speewise as normally these loops run only
> an insignificant number of iterations)

now with 64bits

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