[FFmpeg-devel] [PATCH] png parser

[Jai Menon]

On Mon, Jun 29, 2009 at 6:39 AM, Peter Holik<peter at holik.at> wrote:
> Peter Holik schrieb:
>> 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
>
> ping, pleas apply

Michael, is the patch okay to apply?

-- 
Regards,

Jai