[FFmpeg-devel] [PATCH 03/N] RV30/40 Decoder - RV40 decoder

Michael Niedermayer michaelni
Tue Dec 4 12:20:18 CET 2007


Hi

On Mon, Dec 03, 2007 at 09:17:20AM +0200, Kostya wrote:
> On Sun, Dec 02, 2007 at 11:46:36PM +0100, Michael Niedermayer wrote:
> > On Sat, Dec 01, 2007 at 08:08:59PM +0200, Kostya wrote:
> > > The same as RV30 counterpart - without loop filter.
> > 
> > [...]
> > 
> > >     for(i = 0; i < AIC_MODE1_NUM; i++){
> > >         // For some reason every tenth VLC table is empty
> > >         // So skip it for consistency
> > >         // XXX: redo without this hack
> > >         if((i % 10) == 9) continue;
> > >         init_vlc(&aic_mode1_vlc[i], AIC_MODE1_BITS, AIC_MODE1_SIZE,
> > >                  aic_mode1_vlc_bits[i],  1, 1,
> > >                  aic_mode1_vlc_codes[i], 1, 1, INIT_VLC_USE_STATIC);
> > 
> > i assume this %10 hack can be avoided by chaning the tables and using
> > B + C*9 instead of B + C*10, if so it should be done before this reaches
> > svn as otherwise it likely will never be cleaned up ...
> 
> Actually, comment is outdated. These tables are context-dependent, context is
> determined by numbers in range (-1..8), some of -1 should never occur and tables
> corresponding to them are zero.    
>  
> > [...]
> > > static int get_dimension(GetBitContext *gb, const int *dim1, const int *dim2)
> > > {
> > 
> > >     int val, t;
> > > 
> > >     t = get_bits(gb, 3);
> > >     val = dim1[t];
> > 
> > int t   = get_bits(gb, 3);
> > int val = dim1[t];
> > 
> > 
> > >     if(!val && dim2)
> > >         val = dim2[(t*2 | get_bits1(gb)) & 3];
> > 
> > if(val<0)
> >     val= dim[get_bits1(gb) - val];
> > 
> > and with matching change to dim (and removial of dim2)
> 
> done
> Here is corresponding table:
> static const int rv40_standard_heights[]  = { 120, 132, 144, 240, 288, 480, -8, -10, 180, 360, 576, 0};
>  
> > [...]
> > >     if(!si->type || !get_bits1(gb))
> > >         rv40_parse_picture_size(gb, &w, &h);
> > >     si->width  = w;
> > >     si->height = h;
> > 
> > checking w/h for being valid before using them would be a good idea
> 
> It is done in rv34.c. First slice dimensions are checked and decoder changes
> dimensions if they are different from current ones. Other slices are
> checked to have the same dimensions as the first one.
>  
> > [...]
> > >                         if(B == -1 || B == 0 || B == 1)
> > 
> > if(B<2)
> > 
> > 
> > [...]
> > >     int blocks[RV34_MB_TYPES];
> > >     int count = 0;
> > > 
> > >     if(!r->s.mb_skip_run)
> > >         r->s.mb_skip_run = ff_rv34_get_gamma(gb);
> > > 
> > >     if(--r->s.mb_skip_run)
> > >          return RV34_MB_SKIP;
> > > 
> > >     memset(blocks, 0, sizeof(blocks));
> > 
> > int blocks[RV34_MB_TYPES]={0};
> > 
> > 
> > >     if(r->avail[0])
> > >         blocks[r->mb_type[mb_pos - 1]]++;
> > >     if(r->avail[1])
> > >         blocks[r->mb_type[mb_pos - s->mb_stride]]++;
> > >     if(r->avail[1] && r->avail[2])
> > >         blocks[r->mb_type[mb_pos - s->mb_stride + 1]]++;
> > >     if(r->avail[1] && r->avail[3])
> > >         blocks[r->mb_type[mb_pos - s->mb_stride - 1]]++;
> > 
> > if(r->avail[1]) can be factored out
> > 
> > 
> > [...]
> > >     if(s->pict_type == P_TYPE){
> > >         if(prev_type == RV34_MB_SKIP) prev_type = RV34_MB_P_16x16;
> > >         prev_type = block_num_to_ptype_vlc_num[prev_type];
> > 
> > block_num_to_ptype_vlc_num can be changed to avoid the if()
> 
> ok
>  
> > [...]
> > > /**
> > >  * Initialize decoder
> > >  */
> > > static int rv40_decode_init(AVCodecContext *avctx)
> > > {
> > >     RV34DecContext *r = avctx->priv_data;
> > >     static int tables_done = 0;
> > > 
> > >     r->rv30 = 0;
> > >     ff_rv34_decode_init(avctx);
> > >     if(!tables_done){
> > >         rv40_init_tables();
> > >         tables_done = 1;
> > >     }
> > 
> > you could check an entry of one of the tables which would avoid the
> > tables_done var
> 
> done

patch looks ok

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

There will always be a question for which you do not know the correct awnser.
-------------- 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/20071204/fc230c97/attachment.pgp>



More information about the ffmpeg-devel mailing list