[FFmpeg-devel] DVCPRO HD: request for review

[Michael Niedermayer]

On Thu, Aug 21, 2008 at 12:01:01PM +0400, Roman V. Shaposhnik wrote:
> Michael Niedermayer ?????:
[...]
> >>@@ -470,61 +498,56 @@ static inline void dv_decode_video_segment(DVVideoContext *s,
> >>         v = *mb_pos_ptr++;
> >>         mb_x = v & 0xff;
> >>         mb_y = v >> 8;
> >>+        /* We work with 720p frames split in half. The odd half-frame (chan==2,3) is displaced :-( */
> >>+        if (s->sys->height == 720 && ((s->buf[1]>>2)&0x3) == 0) {
> >>+               mb_y = (mb_y + (36<<1)) % (45<<1);
> >>+        }
> >>    
> >>
> >
> >performing modulo operations per block is not a good idea
> >  
> >
> 
> The compilers (even the dumb ones) usually take care of loop invariant 
> pretty easily.
> But since we're dealing with gcc here I can move it outside the loop myself.

please do, gcc is dumber than dumb


> 
> >>+            for (i=0; i<2; i++, block += 64, mb++, y_ptr += (1<<log2_blocksize))
> >>    
> >>
> >
> >i think there are too many statements on this line
> >  
> >
> Well, that's the style I like very much. It lets me see clearly what the 
> loop is iterating over instead
> of looking all over the body. Again, this is my personal choice and it 
> DOES make my life
> easier. Now, since there's usually two people in the world who look at 
> dv*.[ch] files in FFmpeg
> close enough to be annoyed by a choice of a style -- that would be you 
> and me. And given that
> I spend much more time with the code than you do, can I, please, have it 
> the way it makes
> my life easier?

yes


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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- 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/20080821/92aedf57/attachment.pgp>