[FFmpeg-devel] [PATCH] SIFF demuxer and decoder

[Benoit Fouet]

Hi,

my 2 cents...

Kostya wrote:
> Index: libavcodec/vb.c
> ===================================================================
>
> +static void vb_decode_palette(VBDecContext *c, uint8_t *buf)
> +{
> +    int start, size, i;
> +    int r, g, b;
> +
> +    start = *buf++;
> +    size = ((*buf++) - 1) & 0xFF;
>   

superflous parentheses
and is &0xFF needed ?

> +    for(i = start; i <= start + size; i++){
> +        r = *buf++;
> +        g = *buf++;
> +        b = *buf++;
> +        c->pal[i] = (r << 16) | (g << 8) | b;
>   

you could use bytestream_get_be24

> +    }
> +}
> +
> +static int vb_decode_framedata(VBDecContext *c, uint8_t *buf, int offset)
> +{
> +    uint8_t *prev, *cur;
> +    int blk, blocks, t, blk2;
> +    int blocktypes = 0;
> +    int x, y, a, b;
> +    int pattype, pattern;
> +    const int width = c->avctx->width;
> +    
> +    prev = c->prev_frame + offset;
> +    cur = c->frame;
> +    
> +    blocks = (c->avctx->width >> 2) * (c->avctx->height >> 2);
> +    blk2 = 0;
>   

could be initialized when declaring it

> +static int decode_frame(AVCodecContext *avctx, void *data, int *data_size, uint8_t *buf, int buf_size)
> +{
> +    VBDecContext * const c = avctx->priv_data;
> +    uint8_t *outptr, *srcptr;
> +    int i, j;
> +    int flags;
> +    uint32_t size;
> +    int rest = buf_size;
> +    int offset = 0;
> +    
> +    flags = AV_RL16(buf);
> +    buf += 2;
>   

bytestream_get_le16 ?

> +    rest -= 2;
> +    
> +    if(c->pic.data[0])
> +        avctx->release_buffer(avctx, &c->pic);
> +
> +    if(flags & VB_HAS_GMC){
> +        i = (int16_t)AV_RL16(buf  );
> +        j = (int16_t)AV_RL16(buf+2);
> +        offset = i + j * avctx->width;
> +        buf += 4;
>   

ditto

> Index: libavformat/siff.c
> ===================================================================
> --- libavformat/siff.c	(revision 0)
> +++ libavformat/siff.c	(revision 0)
> @@ -0,0 +1,255 @@
> +/*
> + * Beam Software SIFF demuxer
> + * Copyright (c) 2007 Konstantin Shishkov.
> + *
> + * 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
> + */
> +
> +#include "avformat.h"
> +#include "riff.h"
> +
> +enum SIFFTags{
> +    TAG_SIFF = MKTAG('S', 'I', 'F', 'F'),
> +    TAG_BODY = MKTAG('B', 'O', 'D', 'Y'),
> +    TAG_VBHD = MKTAG('V', 'B', 'H', 'D'),
> +    TAG_SHDR = MKTAG('S', 'H', 'D', 'R'),
> +    TAG_VBV1 = MKTAG('V', 'B', 'V', '1'),
> +    TAG_SOUN = MKTAG('S', 'O', 'U', 'N'),
> +};
> +
> +enum VBFlags{
> +    VB_HAS_GMC     = 0x01,
> +    VB_HAS_AUDIO   = 0x04,
> +    VB_HAS_VIDEO   = 0x08,
> +    VB_HAS_PALETTE = 0x10,
> +    VB_HAS_LENGTH  = 0x20
> +};
>   

duplicated from vb.c

-- 
Ben
Purple Labs S.A.
www.purplelabs.com