[FFmpeg-devel] Request for review

[Michael Niedermayer]

On Tue, Jan 20, 2009 at 05:52:40PM -0800, Roman V. Shaposhnik wrote:
> Hi Michael!
> 
> Thanks a lot for reviewing #1, #3.
> Btw, are you still reviewing #2, #4 and #5 or do
> you want me to resubmit them with the issues you've
> identifies so far fixed?

I think i would prefer them resubmited but i can also try to look at them
again as they are

> 
> On Sat, 2009-01-17 at 02:45 +0100, Michael Niedermayer wrote:
[...]
> > > -            /* Everything is set up -- now just copy data -> DCT block */
> > > -            if (do_edge_wrap) {  /* Edge wrap copy: 4x16 -> 8x8 */
> > > -                uint8_t* d;
> > > -                DCTELEM *b = block;
> > > -                for (i = 0; i < 8; i++) {
> > > -                   d = data + 8 * linesize;
> > > -                   b[0] = data[0]; b[1] = data[1]; b[2] = data[2]; b[3] = data[3];
> > > -                   b[4] =    d[0]; b[5] =    d[1]; b[6] =    d[2]; b[7] =    d[3];
> > > -                   data += linesize;
> > > -                   b += 8;
> > > +                    if (s->sys->pix_fmt == PIX_FMT_YUV411P && mb_x >= (704 / 8)) {
> > > +                        uint8_t* b = scratch;
> > > +                        for (i = 0; i < 8; i++) {
> > > +                            uint8_t* d = data + 8 * linesize;
> > > +                            b[0] = data[0]; b[1] = data[1]; b[2] = data[2]; b[3] = data[3];
> > > +                            b[4] =    d[0]; b[5] =    d[1]; b[6] =    d[2]; b[7] =    d[3];
> > > +                            data += linesize;
> > > +                            b += 8;
> > > +                        }
> > > +                        data = scratch;
> > > +                        linesize = 8;
> > > +                    }
> > >
> > 
> > I would prefer if you did not mix indention changes with changes that move
> > code around, it makes such a patch hard to review.
> > (and indention corrections are always ok anyway so you dont need to post
> >  any patch for them, just commit indention fixes)
> 
> Sure. Do you want me to resubmit #2 for your review without
> the indentation problems, or are you fine with it as it
> is (provided that I'll take care of your comment and commit
> two separate transasctions)?

It would be easier for me to review if you resubmited without the indention
changes

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

There will always be a question for which you do not know the correct awnser.
-------------- 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/20090123/7aa3746d/attachment.pgp>