[FFmpeg-devel] BFI video decoder

[Sisir Koppaka]

Updated decoder attached. (Works!!! :) thanks)

On Fri, Apr 18, 2008 at 12:48 AM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Fri, Apr 18, 2008 at 12:23:47AM +0530, Sisir Koppaka wrote:
> > Updated decoder attached.
> >
> > I haven't used the refactored version of the length bound checking
> because
> > this was a bit clearer.
>
> You can factorize the length check using:
>
> static const uint8_t lentab[4]={0,2,0,1};
>
> if (dst + (length<<lentab[code]) > frame_end)
>    break;
>
> Nice :) using it now...

>
>
> [...]
> > 3 3 3 3 1 3 3 3 3 3 3 3 3 3 1 3 3 3 3 3 3 3 3 1 3 3 3 3 3 3 3 3 3 3 3 1
> 3 3
> > 3 3 3 3 3 3 3 3 3 1 3 1 3 3 3 3 1 3 3 3 3 3 3 3 3 3 3 3 3 3 1 3 3 3 3 3
> 3 3
> > 1 3 3 3 3 1 3 3 3 3 3 3 3 1 3 3 3 3 3 3 3 3 3 3 3 1
> > [DECODER] Finished decoding the frame.
> > ------------------------------
> > It seems to be breaking the bounds too frequently...what do you think?
>
> I think there is a bug in your decoder, read the part about
> "Video compression" in the spec again! There is something you forgot.
>
Ahhhhhhhhhhhhhhhhhh!!!!
I put an extra bytestream_get_le32(&buf) and spoof, the video started
playing without any hitches :)
Moral: A byte(or 4) is sufficient to destroy your code. :)

>
>
> [...]
>
> > 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 i, j;
> >     if (bfi->frame.data[0])
> >         avctx->release_buffer(avctx, &bfi->frame);
> >     if (avctx->get_buffer(avctx, &bfi->frame) < 0) {
> >         av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> >         return -1;
> >     }
> >     dst = bfi->dst;
>
> >     frame_end = bfi->dst + avctx->width * avctx->height;
>
> declaration and initialization can be merged
>
Done along with similar cosmetics

>
>
> >     if (!avctx->frame_number) {
> >         int shift;
> >         bfi->frame.pict_type = FF_I_TYPE;
> >         bfi->frame.key_frame = 1;
> >         /* Setting the palette */
>
> >         pal = (uint32_t *) bfi->frame.data[1];
> >         for (i = 0; i < avctx->extradata_size / 3; i++) {
>
> >             shift = 16;
>
> int shift = 16;
> is IMHO clearer
>
> I thought the overhead of declaring, initializing and destroying a
variable for every iteration was too much, but anyway, i've change it to
what you said.

>
> >             *pal = 0;
> >             for (j = 0; j < 3; j++, shift -= 8)
> >                 *pal +=
>
> >                     ((avctx->extradata[i * 3 + j] << 2) | (avctx->
> >                                                            extradata[i *
> >                                                                      3 +
> >                                                                      j]
> >>
> >                                                            4)) << shift;
>
> following looks nicer:
>                       ((avctx->extradata[i * 3 + j] << 2) |
>                       (avctx->extradata[i * 3 + j] >> 4)  ) << shift;
>
> Also how large is the pal array? How large is extradata_size? is there a
> possibility later might be larger?
>
pal is only a uint32_t * pointer, extradata_size is currently 768 for all
our purposes, but if we add AVI transmuxing support like you said earlier,
we might need/want to use more so, in that case the palette can be made
larger, though I don't think doing so offers any significant value without
first altering the codec itself.
I'm not sure what you meant by the possibility that the latter might be
larger...can you please explain?

>
>
> >             pal++;
> >         }
>
> Thanks for the help....if the attached file looks ok, I'll send the patch
against the latest SVN.

-----------------
Sisir Koppaka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bfi.c
Type: text/x-csrc
Size: 5266 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080418/3f73fd53/attachment.c>