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

Michael Niedermayer michaelni
Sun Dec 16 02:13:16 CET 2007


On Sat, Dec 15, 2007 at 07:39:40PM +0200, Kostya wrote:
> On Fri, Dec 14, 2007 at 10:07:45PM +0100, Michael Niedermayer wrote:
[...]
> > [...]
> > 
> > > //    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
> 
> commented code is an old leftover from slice end detection
> added slice with error (but it is set to error by ff_er_start_frame(), isn't it?) 

the error concealment code handles missing slices different from damaged
ones, this improves the resulting quality
dont hesitate to damage a video by flipping a few bits to see how/if it
works


[...]
>     int c_index = avail_index - 4 + c_off;
> 
>     mv_pos += (subblock_no & 1) + (subblock_no >> 1)*s->b8_stride;

>     if(block_type == RV34_MB_P_8x8 && subblock_no == 3){

the block_type == RV34_MB_P_8x8 check is unneeded, there should be no others
which have subblock_no == 3


>         c_off = -1;
>         c_index = avail_index - 5;
>     }
> 
>     if(r->avail_cache[avail_index - 1]){
>         A[0] = s->current_picture_ptr->motion_val[0][mv_pos-1][0];
>         A[1] = s->current_picture_ptr->motion_val[0][mv_pos-1][1];
>     }
>     if(r->avail_cache[avail_index - 4]){
>         B[0] = s->current_picture_ptr->motion_val[0][mv_pos-s->b8_stride][0];
>         B[1] = s->current_picture_ptr->motion_val[0][mv_pos-s->b8_stride][1];
>     }else{
>         B[0] = A[0];
>         B[1] = A[1];
>     }

>     if(!r->avail_cache[c_index]){

c_index here can be replaced by avail_index - 4 + c_off and all other
c_index can be removed


[...]
>     ff_init_block_index(s);
>     while(!check_slice_end(r, s)) {
>         ff_update_block_index(s);
>         s->dsp.clear_blocks(s->block[0]);
> 
>         if(rv34_decode_macroblock(r, r->intra_types + s->mb_x * 4 + 1) < 0)
>             break;
>         if (++s->mb_x == s->mb_width) {
>             s->mb_x = 0;
>             s->mb_y++;
>             ff_init_block_index(s);
> 
>             memmove(r->intra_types_hist, r->intra_types, s->b4_stride * 4 * sizeof(*r->intra_types_hist));
>             memset(r->intra_types, -1, s->b4_stride * 4 * sizeof(*r->intra_types_hist));
>         }
>         if(s->mb_x == s->resync_mb_x)
>             s->first_slice_line=0;
>         s->mb_num_left--;
>     }
>     if(s->mb_y >= s->mb_height && s->mb_x){
>         ff_er_add_slice(s, s->resync_mb_x, s->resync_mb_y, s->mb_x-1, s->mb_y, AC_ERROR|DC_ERROR|MV_ERROR);
>         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);

this is not correct, EVERY error must be added properly
these are
missmatches of the block end and bitstream end
error returns from rv34_decode_macroblock()


[...]

after you took care of the above, got rid of the mc_dir_part() duplication,
mb_type-avail_cache simplification and anything ive missed ...
you can commit it


PS: id like to see a patch for the loopfilter, 1/3 pel MC and any other major
improvments instead of direct commits (unless the code you commit is really
clean, simplified, fast and free of duplications)

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- 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/20071216/6b2ab218/attachment.pgp>



More information about the ffmpeg-devel mailing list