[FFmpeg-devel] [PATCH] Rl2 Video decoder

[Michael Niedermayer]

On Sun, Mar 16, 2008 at 10:52:27AM +0100, Sascha Sommer wrote:
> Hi,
> 
> Thanks for the review.
> 
> On Sonntag, 16. M?rz 2008, Michael Niedermayer wrote:
> > On Sat, Mar 15, 2008 at 10:07:20PM +0100, Sascha Sommer wrote:
> > > Hi,
> > >
> > > attached patch adds support for the rl2 video format as described on
> > > http://wiki.multimedia.cx/index.php?title=RL2
> >
> > [...]
> >

> > > +     while(pos_in < size){
> > > +         unsigned char tag = buf[pos_in++];
> > > +         int len = 1;
> > > +         unsigned char val;
> > > +         int i;
> > > +         if(tag >= 0x80){
> > > +             if(pos_in >= size)
> > > +                 break;
> > > +             len = buf[pos_in++];
> > > +             if(!len)
> > > +                 break;
> > > +         }
> > > +         for(i=0;i<len && pos_out < frame_size &&
> > > +                          back_pos < back_frame_size ;i++){
> > > +
> > > +             if(s->back_frame){
> > > +                 if(tag == 0x00 || tag == 0x80)
> > > +                     val = s->back_frame[back_pos];
> > > +                 else
> > > +                     val = tag | 0x80;
> > > +             }else{
> > > +                 if(tag < 0x80)
> > > +                     val = tag;
> > > +                 else if(tag == 0x80)
> > > +                     val = 0;
> > > +                 else
> > > +                     val = tag & 0x7F;
> > > +             }
> > > +             out[pos_out++] = val;
> > > +             ++back_pos;
> > > +             ++x;
> > > +             if(x >= s->avctx->width){
> > > +                 pos_out += row_inc;
> > > +                 x = 0;
> > > +             }
> > > +         }
> > > +    }
> >
> > This can be simplified, i could point at specific things but after all
> > its a qualification task so i wont ...
> >
> >
> 
> Better now?

No, its worse, now you do a division in the innermost loop.
Anyway, you missed the large simplification i meant.


[...]
> > > +
> > > +    /** run length decode */
> > > +   
> > > rl2_rle_decode(s,buf,buf_size,s->frame.data[0],s->frame.linesize[0]);
> >
> > Does doxygen nowadays support such inline comments?
> >
> 
> It can be used to describe the steps of a function: For example the section 

ok

[...]

> +    s->frame.data[0] = NULL;

why is this needed?

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20080316/772d5eeb/attachment.pgp>