[FFmpeg-devel] BFI video decoder

[Michael Niedermayer]

On Fri, Apr 18, 2008 at 02:34:28AM +0530, Sisir Koppaka wrote:
> Updated decoder attached
> 
> On Fri, Apr 18, 2008 at 2:05 AM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> 
> > 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.
> >
> > I could find only the pointer definition of data[] in avcodec.h, if I
> could know the size of data[], then I suppose a check of extradata_size>size
> of data[1] => exit will work?

yes and its size is as large as a palette with 256 entries can be.



[...]

>     /* Set frame parameters and palette, if necessary */
>     if (!avctx->frame_number) {
>         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++) {
>             int shift = 16;
>             *pal = 0;
>             for (j = 0; j < 3; j++, shift -= 8)
>                 *pal +=
>                     ((avctx->extradata[i * 3 + j] << 2) |
>                     (avctx->extradata[i * 3 + j] >> 4)) << shift;
>             pal++;
>         }
>         bfi->frame.palette_has_changed = 1;
>     } else {
>         bfi->frame.pict_type = FF_P_TYPE;
>         bfi->frame.key_frame = 0;
>     }
> 
>     buf += 4; //Unpacked size, not required.
>     
>     while (dst != frame_end) {
>         static const uint8_t lentab[4]={0,2,0,1};

>         unsigned int byte = *buf++, code = byte >> 6, length = byte & ~0xC0, offset;

I think this would be more readable if it where on 4 lines instead of 1


> 
>         /* Get length and offset(if required) */
>         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 && length == 0) {
>                     break;
>                 }

superflous {}


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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/7542e0fc/attachment.pgp>