[FFmpeg-devel] [PATCH] Rl2 Video decoder

[Sascha Sommer]

Hi,

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

Ah

               if(tag < 0x80)
                    val = tag;
              else if(tag == 0x80)
                    val = 0;
              else
                    val = tag & 0x7F;


==          val = tag & ~0x80

Regards

Sascha