[FFmpeg-soc] BFI Decoder

[Sisir Koppaka]

updated decoder attached.

On Thu, Apr 17, 2008 at 4:21 PM, Michael Niedermayer <michaelni at gmx.at>
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/40656348@N00/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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bfi.c
Type: text/x-csrc
Size: 5954 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080417/f5124dc6/attachment.c>