[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