updated decoder attached.

On Thu, Apr 17, 2008 at 4:21 PM, Michael Niedermayer <[EMAIL PROTECTED]>
wrote:

> On Thu, Apr 17, 2008 at 02:20:08PM +0530, Sisir Koppaka wrote:
> > Updated decoder attached.
> > -----------------
> > Sisir Koppaka
>
> [...]
>
> > static int bfi_decode_init(AVCodecContext * avctx)
> > {
> >     BFIContext *bfi = avctx->priv_data;
> >     bfi->frame.reference = 1;
>
> >     bfi->frame.buffer_hints =
> >         FF_BUFFER_HINTS_VALID | FF_BUFFER_HINTS_READABLE |
> >         FF_BUFFER_HINTS_PRESERVE | FF_BUFFER_HINTS_REUSABLE;
>
> As you copy the data from an internal buffer to AVFrame these make no
> sense.
>
Removed

>
> [...]
> > static int bfi_decode_frame(AVCodecContext * avctx, void *data,
> >                             int *data_size, const uint8_t * buf,
> >                             int buf_size)
> > {
> >     BFIContext *bfi = avctx->priv_data;
> >     uint8_t *dst;
> >     uint8_t *src;
> >     uint8_t *dst_offset;
> >     uint8_t colour1, colour2;
> >     uint32_t *pal;
> >     uint8_t *frame_end;
> >     unsigned int code, byte, length, offset, height = avctx->height;
> >     int remaining = avctx->width, i, j;
>
> >     const int wrap_to_next_line = bfi->frame.linesize[0];
>
> redundant

Removed

>
> >     if (avctx->reget_buffer(avctx, &bfi->frame)) {
> >         av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
> >         return -1;
> >     }
> >     //avcodec_set_dimensions(avctx, avctx->width, avctx->height);
> //Check this.
> >     dst = bfi->dst;
> >     frame_end = bfi->dst + avctx->width * avctx->height;
> >     if(bfi->toggle) {
> >         bfi->frame.pict_type = FF_I_TYPE;
> >         bfi->frame.key_frame = 1;
>
> >         /* Converts the given 6-bit palette values(0-63) to 8-bit
> values(0-255) */
> >         for (i = 0; i < avctx->extradata_size; i++)
> >             avctx->extradata[i] =
> >                 (avctx->extradata[i] << 2) | (avctx->extradata[i] >> 4);
>
> A decoder cannot write in extradata
>
Now uses a local array of size 768....if size is a concern, then i'll reduce
it to an array of size 3, should I?

>
>
> >         /* Setting the palette */
> >         pal = (uint32_t *) bfi->frame.data[1];
> >         for (i = 0; i < 256; i++)
> >             *pal++ = AV_RB24(&avctx->extradata[i*3]);
> >         bfi->frame.palette_has_changed = 1;
> >         av_log(NULL,AV_LOG_INFO, "\n[DECODER] Palette has been set.");
> >         bfi->toggle = 0;
> >     }
> >     else
> >     {
> >         bfi->frame.pict_type = FF_P_TYPE;
> >         bfi->frame.key_frame = 0;
> >     }
> >
>
> trailing whitespace
>
Deleted

>
>
> >     while (dst != frame_end) {
> >         byte = *buf++;
> >         code = byte >> 6;
> >         length = byte & ~0xC0;
> >         if (length == 0) {
> >            if(code == 1) {
> >                length = bytestream_get_byte(&buf);
> >                offset = bytestream_get_le16(&buf);
> >            }
> >            else {
> >               length = bytestream_get_le16(&buf);
> >               if(code == 2)
> >                   goto finish;
> >            }
> >         }
> >         else {
> >             if(code == 1)
> >                 offset = bytestream_get_byte(&buf);
> >         }
>
> inconsistant indention
>
Changed

>
>
> >         switch (code) {
> >         case 0:                //Normal Chain
> >             if(dst+length>frame_end)
> >                 break;
> >             bytestream_get_buffer(&buf, dst, length);
> >             dst += length;
> >             av_log(NULL,AV_LOG_INFO, "\n[DECODER] Normal Chain.");
> >             break;
> >         case 1:                //Back Chain
> >             dst_offset = dst - offset;
> >             length *= 4;        //Convert dwords to bytes.
> >             if(dst+length>frame_end || dst_offset<bfi->dst)
> >                 break;
>
> >             while(length--)
> >             {
> >                 *dst++ = *dst_offset++;
> >             }
>
> superflous {}
>
Removed

>
> >             av_log(NULL,AV_LOG_INFO, "\n[DECODER] Back Chain.");
> >             break;
> >         case 2:                //Skip Chain
>
> >             if(dst+length>frame_end)
> >                 break;
>
> duplicate
>
I did some refactoring but it seems  to degenerate the video. I think the
question is what to do when we hit one of these cases where length is out of
bounds etc...do we ignore that particular iteration and move on to the next
or because do we ignore the whole frame? if we ignore only that iteration,
then we could possibly be displacing a whole set of pixels, which is what I
think is happening, looking at the video. Some decent cases of this
displacement are at http://www.flickr.com/gp/[EMAIL PROTECTED]/44Z0J8 but after
the refactoring it's become worse for some reason.

>
>
> >             dst += length;
> >             av_log(NULL,AV_LOG_INFO, "\n[DECODER] Skip Chain.");
> >             break;
> >         case 3:                //Fill Chain
> >             colour1 = bytestream_get_byte(&buf);
> >             colour2 = bytestream_get_byte(&buf);
> >             if(dst+length*2>frame_end)
> >                 break;
> >             while(length--)
> >             {
> >                 *dst++ = colour1;
> >                 *dst++ = colour2;
> >             }
> >             av_log(NULL,AV_LOG_INFO, "\n[DECODER] Fill Chain.");
> >             break;
>
> >         default:
> >             av_log(NULL, AV_LOG_INFO,
> >                    "\nOops! Couldn't recognize the 'code'...");
>
> in which case can this be reached?
>
yes it's not possible since 0<=code<=3 and we have all those covered, I
removed it.

-----------------
Sisir Koppaka
/*
 * Brute Force & Ignorance (BFI) video decoder
 * Copyright (c) 2008 Sisir Koppaka
 *
 * 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 bfi.c
 * @brief Brute Force & Ignorance (.bfi) video decoder
 * @author Sisir Koppaka ( sisir.koppaka at gmail dot com )
 * @sa http://wiki.multimedia.cx/index.php?title=BFI
 */

#include "avcodec.h"
#include "common.h"
#include "bytestream.h"

typedef struct BFIContext {
    AVCodecContext *avctx;
    AVFrame frame;
    int toggle;
    uint8_t *dst;
} BFIContext;

static int bfi_decode_init(AVCodecContext * avctx)
{
    BFIContext *bfi = avctx->priv_data;
    bfi->frame.reference = 1;
    avctx->pix_fmt = PIX_FMT_PAL8;
    bfi->toggle = 1;
    bfi->dst = av_mallocz(avctx->width * avctx->height);
    av_log(NULL, AV_LOG_INFO, "\n[DECODER] Initialization done.");
    return 0;
}

static int bfi_decode_frame(AVCodecContext * avctx, void *data,
                            int *data_size, const uint8_t * buf,
                            int buf_size)
{
    BFIContext *bfi = avctx->priv_data;
    uint8_t *dst;
    uint8_t *src;
    uint8_t *dst_offset;
    uint8_t colour1, colour2;
    uint32_t *pal;
    uint8_t *frame_end;
    unsigned int code, byte, length, offset, height = avctx->height;
    int remaining = avctx->width, i, j;
    if (avctx->reget_buffer(avctx, &bfi->frame)) {
        av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
        return -1;
    }
    //avcodec_set_dimensions(avctx, avctx->width, avctx->height); //Check this.
    dst = bfi->dst;
    frame_end = bfi->dst + avctx->width * avctx->height;
    if (bfi->toggle) {
        char palette[768];
        bfi->frame.pict_type = FF_I_TYPE;
        bfi->frame.key_frame = 1;
        /* Converts the given 6-bit palette values(0-63) to 8-bit values(0-255) */
        for (i = 0; i < avctx->extradata_size; i++)
            palette[i] =
                (avctx->extradata[i] << 2) | (avctx->extradata[i] >> 4);
        /* Setting the palette */
        pal = (uint32_t *) bfi->frame.data[1];
        for (i = 0; i < 256; i++)
            *pal++ = AV_RB24(&palette[i * 3]);
        bfi->frame.palette_has_changed = 1;
        av_log(NULL, AV_LOG_INFO, "\n[DECODER] Palette has been set.");
        bfi->toggle = 0;
    } else {
        bfi->frame.pict_type = FF_P_TYPE;
        bfi->frame.key_frame = 0;
    }

    while (dst != frame_end) {
        byte = *buf++;
        code = byte >> 6;
        length = byte & ~0xC0;
        if (length == 0) {
            if (code == 1) {
                length = bytestream_get_byte(&buf);
                offset = bytestream_get_le16(&buf);
            } else {
                length = bytestream_get_le16(&buf);
                if (code == 2)
                    goto finish;
            }
        } else {
            if (code == 1)
                offset = bytestream_get_byte(&buf);
        }

        if (dst + length > frame_end)
            continue;
        if (code == 1 && dst_offset < bfi->dst)
            continue;
/*        if (code == 3 && dst + length * 2 > frame_end) {
            //Garbage, yeah, but we still have to read it to prevent misreading upcoming bytes.
            colour1 = bytestream_get_byte(&buf);
            colour2 = bytestream_get_byte(&buf);
            continue;
        } */

        switch (code) {

        case 0:                //Normal Chain
            bytestream_get_buffer(&buf, dst, length);
            dst += length;
            av_log(NULL, AV_LOG_INFO, "\n[DECODER] Normal Chain.");
            break;

        case 1:                //Back Chain
            dst_offset = dst - offset;
            length *= 4;        //Convert dwords to bytes.
            while (length--)
                *dst++ = *dst_offset++;
            av_log(NULL, AV_LOG_INFO, "\n[DECODER] Back Chain.");
            break;

        case 2:                //Skip Chain
            dst += length;
            av_log(NULL, AV_LOG_INFO, "\n[DECODER] Skip Chain.");
            break;

        case 3:                //Fill Chain
            colour1 = bytestream_get_byte(&buf);
            colour2 = bytestream_get_byte(&buf);
            if (dst + length * 2 > frame_end)
                break;
            while (length--) {
                *dst++ = colour1;
                *dst++ = colour2;
            }
            av_log(NULL, AV_LOG_INFO, "\n[DECODER] Fill Chain.");
            break;
        }
    }
  finish:
    src = bfi->dst;
    dst = bfi->frame.data[0];
    while (height--) {
        memcpy(dst, src, avctx->width);
        src += avctx->width;
        dst += bfi->frame.linesize[0];
    }
    *data_size = sizeof(AVFrame);
    *(AVFrame *) data = bfi->frame;
    return buf_size;
}

static int bfi_decode_close(AVCodecContext * avctx)
{
    BFIContext *bfi = avctx->priv_data;
    if (bfi->frame.data[0])
        avctx->release_buffer(avctx, &bfi->frame);
    av_free(bfi->dst);
    av_log(NULL, AV_LOG_INFO,
           "\n[DECODER] Finished decoding the frame.\n");
    return 0;
}

AVCodec bfi_decoder = {
    .name = "bfi",
    .type = CODEC_TYPE_VIDEO,
    .id = CODEC_ID_BFI,
    .priv_data_size = sizeof(BFIContext),
    .init = bfi_decode_init,
    .close = bfi_decode_close,
    .decode = bfi_decode_frame,
};
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc

Reply via email to