[FFmpeg-devel] BFI video decoder

[Michael Niedermayer]

On Fri, Apr 18, 2008 at 01:33:58AM +0530, Sisir Koppaka wrote:
> Updated decoder attached. (Works!!! :) thanks)
[...]
> >
> > >             *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 points to bfi->frame.data[1] which is not infinitly large, if
extradata_size is larger (from bfi in avi for example) then this still
could do nasty things.


[...]
> static int bfi_decode_init(AVCodecContext * avctx)
> {
>     BFIContext *bfi = avctx->priv_data;

>     bfi->frame.reference = 1;

this should be before get_buffer()


[...]
>     bytestream_get_le32(&buf); //Unpacked size, not required.

buf+= 4; 
should do


> 
>     while (dst != frame_end) {
>         static const uint8_t lentab[4]={0,2,0,1};

>         byte = *buf++;
>         code = byte >> 6;
>         length = byte & ~0xC0;

declaration and initialization can be merged for these as well


[...]
>         /* Do boundary check */
>         for(i=0;i<4;i++)
>             if (dst + (length<<lentab[i]) > frame_end)
>                 break;

this does not make sense


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080417/a93b6ef1/attachment.pgp>