[FFmpeg-devel] [PATCH] Rl2 Video decoder

[Sascha Sommer]

Hi,

> > +typedef struct Rl2Context {
> > +    AVCodecContext *avctx;
> > +    AVFrame frame;
> > +
> > +    unsigned short video_base; ///< initial drawing offset
> >
> > +    int base_x;                ///< x in the current line
> > +    int base_y;                ///< how often the stride needs to be
> > incremented
>
> I think they would fit better as local variables of rl2_rle_decode(), after
> all there is no other function useing them.
>

Changed.

> > +    unsigned int clr_count;    ///< number of used colors (currently
> > unused) +    unsigned char* back_frame; ///< background frame
> > +    unsigned int palette[AVPALETTE_COUNT];
> > +} Rl2Context;
> > +
> > +/**
> > + * Run Length Decode a single 320x200 frame
> > + * @param s rl2 context
> > + * @param buf input buffer
> > + * @param size input buffer size
> > + * @param out ouput buffer
> > + * @param stride stride of the output buffer
> > + * @param ignore_base ignore video_base
> > + */
> > +static void rl2_rle_decode(Rl2Context *s,const unsigned char* in,int
> > size, +                               unsigned char* out,int stride,int
> > ignore_base){
>
> Instead of passing ignore_base you could pass base itself and set it to 0
> for the ignore case.
>

Changed.

> > +     int stride_adj = stride - s->avctx->width;
> > +     const unsigned char* back_frame = s->back_frame;
> > +     const unsigned char* in_end = in + size;
> > +     const unsigned char* out_end = out + stride * s->avctx->height;
> > +     unsigned char* line_end = out + s->avctx->width;
> > +
> >
> >
> > +     /** copy start of the background frame */
> > +     if(s->back_frame){
> > +         int i;
> > +         for(i=0;i<s->base_y;i++){
> > +             memcpy(out,back_frame,s->avctx->width);
> > +             out += stride;
> > +             back_frame += s->avctx->width;
> > +         }
> > +         if(s->base_x){
> > +             memcpy(out, back_frame, s->base_x);
> > +             back_frame += s->base_x;
> > +         }
> > +         line_end = out + s->avctx->width;
> > +         out += s->base_x;
> > +     }else if(!ignore_base){
> > +         line_end = out + s->base_y * stride + s->avctx->width;
> > +         out += s->video_base + s->base_y * stride_adj;
> > +         back_frame += s->video_base;
> > +     }
>
> for(i=0;i<=base_y;i++){
>     if(s->back_frame)
>         memcpy(out,back_frame,s->avctx->width);
>     out += stride;
>     back_frame += s->avctx->width;
> }
> back_frame += base_x - s->avctx->width;
> line_end = out - stride_adj;
> out += base_x - stride;
>

Changed.

> > +
> > +     /** decode the variable part of the frame */
> > +     while(in < in_end){
> > +         unsigned char val = *in++;
> > +         int len = 1;
> > +         if(val >= 0x80){
> > +             if(in >= in_end)
> > +                 break;
> > +             len = *in++;
> > +             if(!len)
> > +                 break;
> > +         }
> > +
> >
> > +         if(out >= out_end - len)
>
> if(len >= out_end - out)
>
> out_end - len could theoretically overflow
>

Fixed.


> > +             *out++ = *back_frame++;
> > +             if(out == line_end){
> > +                  out += stride_adj;
> > +                  line_end += stride;
> > +             }
>
> following might be faster, but ignore it if its not
>
> memcpy(out, back_frame, line_end - out);
> back_frame += line_end - out;
> out= line_end + stride_adj;
> line_end += stride;
>

It is faster.


> patch looks ok except these
>

So I can commit this now?

Regards

Sascha


-------------- next part --------------
A non-text attachment was scrubbed...
Name: rl2_decoder_try11.patch
Type: text/x-diff
Size: 8706 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080321/29263b0f/attachment.patch>