[FFmpeg-devel] Prepwork for DVCPRO HD inclusion

[Michael Niedermayer]

On Sun, Aug 10, 2008 at 01:31:50PM -0700, Roman V. Shaposhnik wrote:
> On Sun, 2008-08-10 at 22:25 +0200, Michael Niedermayer wrote:
> > >  static void dv_build_unquantize_tables(DVVideoContext *s, uint8_t* perm)
> > >  {
> > > -    int i, q, j;
> > > +    int i, q, a; 
> > >  
> > >      /* NOTE: max left shift is 6 */
> > >      for(q = 0; q < 22; q++) {
> > >          /* 88DCT */
> > > -        for(i = 1; i < 64; i++) {
> > > +        i=1;
> > > +        for(a = 0; a<4; a++) {
> > > +            for(; i < dv_quant_areas[a]; i++) {
> > > -            /* 88 table */
> > > +                /* 88 table */
> > > -            j = perm[i];
> > > -            s->dv_idct_shift[0][0][q][j] =
> > > -                dv_quant_shifts[q][dv_88_areas[i]] + 1;
> > > +                s->dv_idct_factor[0][0][q][i] = dv_iweight_88[i]<<(dv_quant_shifts[q][a] + 1);
> > > -            s->dv_idct_shift[1][0][q][j] = s->dv_idct_shift[0][0][q][j] + 1;
> > > +                s->dv_idct_factor[1][0][q][i] = s->dv_idct_factor[0][0][q][i]<<1;
> > > -        }
> > > -
> > > -        /* 248DCT */
> > > -        for(i = 1; i < 64; i++) {
> > > +
> > > +                /* 248 table */
> > > -            /* 248 table */
> > > -            s->dv_idct_shift[0][1][q][i] =
> > > -                dv_quant_shifts[q][dv_248_areas[i]] + 1;
> > > +                s->dv_idct_factor[0][1][q][i] = dv_iweight_248[i]<<(dv_quant_shifts[q][a] + 1);
> > > -            s->dv_idct_shift[1][1][q][i] = s->dv_idct_shift[0][1][q][i] + 1;
> > > +                s->dv_idct_factor[1][1][q][i] = s->dv_idct_factor[0][1][q][i]<<1;
> > > +            }
> > >          }
> > >      }
> > >  }
> > 
> > cosmetic and functional mix, please split the patch in a cosmetic and
> > a functional patch. This is very hard to decypher otherwise
> 
> Which part of this patch have you considered to be cosmetic? The
> indentation changes because of the extra loop (and that's functional)
> and the table name is changed because the table now has nothing
> to do with shifts.

the indention and whitespace change primarely, it makes reviewing hard because
i have to check each line if something changed or not ...

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

I count him braver who overcomes his desires than him who conquers his
enemies for the hardest victory is over self. -- 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/20080810/8a10c46a/attachment.pgp>