[FFmpeg-devel] [PATCH] Rl2 Video decoder

[Sascha Sommer]

Hi,

On Sonntag, 16. M?rz 2008, Michael Niedermayer wrote:
> 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.
>

That was my impression, too. So most of the conditions could be moved out of 
the for loop. Does this match your large simplification or do I need to jump 
higher to get back my pencil?

>
> > +    s->frame.data[0] = NULL;
>
> why is this needed?
>

Its not. This is just a copy and paste error...

Regards

Sascha