[FFmpeg-devel] [PATCH 01/N] RV30/40 Decoder - Core

Michael Niedermayer michaelni
Fri Dec 14 22:07:45 CET 2007


On Fri, Dec 14, 2007 at 08:48:02PM +0200, Kostya wrote:
> On Thu, Dec 13, 2007 at 01:07:08PM +0100, Michael Niedermayer wrote:
> > On Thu, Dec 13, 2007 at 09:46:33AM +0200, Kostya wrote:
> > > On Sun, Dec 09, 2007 at 02:47:36PM +0100, Michael Niedermayer wrote:
> > > [...]
> > > > > > 
> > > > > > fill_rectangle() should be used here, yes please
> > > > > > move fill_rectangle() into a file from where it can be used, its a very
> > > > > > usefull function
> > > > > 
> > > > > I guess it should be moved to h264.h. I will try to use it where appropriate.
> > > > 
> > > > i dont think it should be moved to h264.h its not h264 specific
> > > 
> > > Then it's up to you to decide where place it.
> > 
> > rectangle.h would do for now
> > 
> > 
> > > 
> > > [...]
> > > 
> > > Here is a new revision which relies on fill_rectangle()
> > > (mostly for availability cache filling).
> > 
> > ok, ill wait until you took care of all review comments before reviewing
> > again
> 
> Should be taken now.

[...]
>     switch(block_type){
>     case RV34_MB_P_16x16:
>     case RV34_MB_P_MIX16x16:
>         avail_index = 5;
>         if(r->avail_cache[avail_index - 2]){
>             c_off = 2;
>             c_index = avail_index - 2;
>         }
>         break;
>     case RV34_MB_P_8x8:
>         avail_index = 5 + (subblock_no & 1) + (subblock_no & 2) * 2;
>         mv_pos += (subblock_no & 1) + (subblock_no >> 1)*s->b8_stride;
>         if(r->avail_cache[avail_index - 3]){
>             c_off = 1;
>             c_index = avail_index - 3;
>         }
>         if(subblock_no == 3){
>             c_off = -1;
>             c_index = avail_index - 5;
>         }
>         break;
>     case RV34_MB_P_16x8:
>         mv_pos += subblock_no*s->b8_stride;
>         avail_index = 5 + subblock_no * 4;
>         if(r->avail_cache[avail_index - 2]){
>             c_off = 2;
>             c_index = avail_index - 2;
>         }
>         break;
>     case RV34_MB_P_8x16:
>         mv_pos += subblock_no;
>         avail_index = 5 + subblock_no;
>         if(r->avail_cache[avail_index - 3]){
>             c_off = 1;
>             c_index = avail_index - 3;
>         }
>         break;
>     default:
>         return;
>     }

static const uint8_t avail_index_tab[4] = {5,6,9,10}
avail_index = avail_index_tab[subblock_no];
mv_pos     += (subblock_no & 1) + (subblock_no >> 1)*s->b8_stride;
c_off= part_sizes_w[block_type];
c_index= avail_index - 4 + c_off;

would be simpler (assumes sensible subblock_no)


[...]

>         if(!r->avail_cache[avail_index - 4] || (!r->avail_cache[avail_index - 1] && !r->rv30)){
>             C[0] = A[0];
>             C[1] = A[1];
>         }else{
>             C[0] = s->current_picture_ptr->motion_val[0][mv_pos-s->b8_stride-1][0];
>             C[1] = s->current_picture_ptr->motion_val[0][mv_pos-s->b8_stride-1][1];
>         }

if(r->avail_cache[avail_index - 4] && (r->avail_cache[avail_index - 1] || r->rv30)){
    C[0] = s->current_picture_ptr->motion_val[0][mv_pos-s->b8_stride-1][0];
    C[1] = s->current_picture_ptr->motion_val[0][mv_pos-s->b8_stride-1][1];
}else{
    C[0] = A[0];
    C[1] = A[1];
}

note, this is just elementary algebra style simplification, the code does look
a little wrong, avail_index - 4 is not mv_pos-s->b8_stride-1 and
c_index in r->avail_cache[c_index] in the original code was -1 thus it accesses
out of the avail_cache array


[...]
> /**
>  * motion vector prediction for B-frames
>  */
> static void rv34_pred_mv_b(RV34DecContext *r, int block_type, int dir)
> {
>     MpegEncContext *s = &r->s;
>     int mb_pos = s->mb_x + s->mb_y * s->mb_stride;
>     int mv_pos = s->mb_x * 2 + s->mb_y * 2 * s->b8_stride;
>     int A[2], B[2], C[2];
>     int has_A = 0, has_B = 0, has_C = 0;
>     int mx, my;
>     int i, j;
>     Picture *cur_pic = s->current_picture_ptr;
>     const int mask = dir ? MB_TYPE_L1 : MB_TYPE_L0;
> 
>     memset(A, 0, sizeof(A));
>     memset(B, 0, sizeof(B));
>     memset(C, 0, sizeof(C));
>     if(r->avail_cache[5-1] && ((cur_pic->mb_type[mb_pos] & cur_pic->mb_type[mb_pos - 1]) & mask)){

if(r->avail_cache[5-1] & mb_type & mask)

that is if you store the mb_type in the cache instead of 1


[...]

> static int rv34_decode_macroblock(RV34DecContext *r, int8_t *intra_types)
> {
>     MpegEncContext *s = &r->s;
>     GetBitContext *gb = &s->gb;
>     int cbp, cbp2;
>     int i, blknum, blkoff;
>     DCTELEM block16[64];
>     int luma_dc_quant;
> 
>     // Calculate which neighbours are available. Maybe it's worth optimizing too.
>     memset(r->avail_cache, 0, sizeof(r->avail_cache));
>     fill_rectangle(r->avail_cache + 5, 2, 2, 4, 1, 1);
>     if(s->mb_x && !(s->first_slice_line && s->mb_x == s->resync_mb_x))
>         r->avail_cache[4] =
>         r->avail_cache[8] = 1;
>     if(!s->first_slice_line)
>         r->avail_cache[1] =
>         r->avail_cache[2] = 1;
>     if(((s->mb_x+1) < s->mb_width) && (!s->first_slice_line || ((s->mb_x+1) == s->resync_mb_x)))
>         r->avail_cache[3] = 1;
>     if(s->mb_x && !s->first_slice_line && ((s->mb_y-1) > s->resync_mb_y || s->mb_x > s->resync_mb_x))
>         r->avail_cache[0] = 1;

following is simpler

dist= (s->mb_x - s->resync_mb_x) + (s->mb_y - s->resync_mb_y)* s->mb_width;

if(s->mb_x && dist)
    ...
if(dist >= s->mb_width)
    ...
if(s->mb_x+1 < s->mb_width && dist >= s->mb_width - 1)
    ...
if(s->mb_x && dist > s->mb_width)
    ...

[...]

> //    if(r->bits > get_bits_count(gb) && show_bits(gb, r->bits-get_bits_count(gb)))
> //        return -1;
>     if(s->mb_y >= s->mb_height && s->mb_x)
>         return 1;
>     ff_er_add_slice(s, s->resync_mb_x, s->resync_mb_y, s->mb_x-1, s->mb_y, AC_END|DC_END|MV_END);

why is above commented out?
also this is still not correct damaged slices should be added with
AC_ERROR|DC_ERROR|MV_ERROR

[...]
-- 
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/20071214/0343bcbe/attachment.pgp>



More information about the ffmpeg-devel mailing list