[FFmpeg-devel] [PATCH] png parser

[Måns Rullgård]

"Peter Holik" <peter at holik.at> writes:

>>> On Wed, May 27, 2009 at 02:32:40PM +0200, Peter Holik wrote:
>>>>
>>>> This patch adds a png parser to ffmpeg.
>>>
>>> There are two patches that should really be one.
>>>
>>>> --- /dev/null
>>>> +++ b/libavcodec/png_parser.c
>>>> @@ -0,0 +1,95 @@
>>>> +/**
>>>> + * @file libavcodec/png_parser.c
>>>> + * PNG parser.
>>>
>>> Drop the period.
>>>
>>>> +/**
>>>> + * finds the end of the current frame in the bitstream.
>>>
>>> Capitalize.
>>>
>>>> + * @return the position of the first byte of the next frame, or -1
>>>> + */
>>>> +static int find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size){
>>>> +    int i=0;
>>>> +    uint64_t state= pc->state64;
>>>> +
>>>> +    if(!pc->frame_start_found){
>>>> +        for(i=0; i<buf_size; i++){
>>>> +            state= (state<<8) | buf[i];
>>>> +            if(state == PNGSIG || state == MNGSIG){
>>>> +                i++;
>>>> +                pc->frame_start_found=1;
>>>> +                break;
>>>
>>> K&R coding style please, i.e. space between if/for/.. and ( and between
>>> ) and {, spaces around operators and the { on the next line for function
>>> declarations.
>>>
>>>> +static int png_parse(AVCodecParserContext *s,
>>>> +                           AVCodecContext *avctx,
>>>> +                           const uint8_t **poutbuf, int *poutbuf_size,
>>>> +                           const uint8_t *buf, int buf_size)
>>>
>>> weird indentation
>>
>> Ok, I've taken this format out of other ffmpeg source files (unluckily)
>>
>> For Makefile i was unsure, because usually configure creates
>> Makefiles and therefor a separated patchfile.

Only in autohell, and FFmpeg is makefile heaven.

> diff --git a/libavcodec/png_parser.c b/libavcodec/png_parser.c
> new file mode 100644
> index 0000000..1293ef2
> --- /dev/null
> +++ b/libavcodec/png_parser.c
> @@ -0,0 +1,95 @@
> +/*
> + * PNG parser
> + * Copyright (c) 2009 Peter Holik
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file libavcodec/png_parser.c
> + * PNG parser
> + */
> +
> +#include "parser.h"
> +
> +#define PNGSIG 0x89504e470d0a1a0a
> +#define MNGSIG 0x8a4d4e470d0a1a0a
> +
> +/**
> + * Finds the end of the current frame in the bitstream.
> + * @return the position of the first byte of the next frame, or -1
> + */
> +static int find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size)
> +{
> +    int i = 0;
> +    uint64_t state = pc->state64;
> +
> +    if (!pc->frame_start_found) {
> +        for (i = 0; i < buf_size; i++) {
> +            state = (state << 8) | buf[i];
> +            if (state == PNGSIG || state == MNGSIG) {
> +                i++;
> +                pc->frame_start_found = 1;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (pc->frame_start_found) {
> +        /* EOF considered as end of frame */
> +        if (buf_size == 0)
> +            return 0;
> +        for(; i < buf_size; i++){
> +            state = (state << 8) | buf[i];
> +            if (state == PNGSIG || state == MNGSIG) {
> +                pc->frame_start_found = 0;
> +                pc->state64 = 0;
> +                return i - 7;
> +            }
> +        }
> +    }

Wouldn't it be faster to parse the PNG file structure, skipping chunks
by size?  Reading the entire data a byte at a time is slow in itself,
and the 64-bit shifts are unfriendly on 32-bit machines (I don't trust
gcc to be sane).

-- 
M?ns Rullg?rd
mans at mansr.com