[FFmpeg-devel] BFI video decoder

[Sisir Koppaka]

On Thu, Apr 17, 2008 at 8:24 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Thu, Apr 17, 2008 at 07:55:00PM +0530, Sisir Koppaka wrote:
> > /* Sorry for cross-posting this message on ffmpeg-soc and devel, but I
> sent
> > a mail with a questionably large attachment in the morning to soc and
> since
> > then my mails haven't been appearing in the soc archives and hence, I'm
> not
> > sure if anyone else is getting them. */
> >
> > updated decoder attached.
>


> [...]
> > 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;
> >     }
>
> As nothing is reused from the content of the AVFrame a normal
> get_buffer()/
> release_buffer() could be used.
>
> Done

>
> >     //avcodec_set_dimensions(avctx, avctx->width, avctx->height);
> //Check this.
> >     dst = bfi->dst;
> >     frame_end = bfi->dst + avctx->width * avctx->height;
>
> >     if (bfi->toggle) {
>
> You could use  avctx->frame_number == 0 here and remove the toggle
> variable
>
Changed

>
>
> >         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);
>
> What could happen if extradata_size where larger than 768 ?
>
But we set it to 768 in the demuxer right?

>
>
> [...]
> >     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;
>
> this does not look correct
>
Yes, I'm working on a couple of branches and this must have happened when I
tried merging them. Fixed.

The video has improved a lot now, it's more legible and clear. I'm working
on your remaining suggestions...
-----------------
Sisir Koppaka