[FFmpeg-devel] [PATCH] Apple RPZA encoder
Jai Menon
jmenon86
Tue Mar 24 12:35:12 CET 2009
On 3/24/09, Diego Biurrun <diego at biurrun.de> wrote:
> On Tue, Mar 24, 2009 at 02:08:44PM +0530, Jai Menon wrote:
> > On 3/24/09, Diego Biurrun <diego at biurrun.de> wrote:
> > > On Tue, Mar 24, 2009 at 10:33:19AM +0530, Jai Menon wrote:
> > > >
> > > > Attached patch is a cleaned up version of the original one posted by
> > > > Todd Kirby [1].
> > > > And, yeah, its a gsoc qualification task :)
> > > >
> > > > [1] http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2005-June/001673.html
> > > >
[...]
> > > > + bi.image_width = s->frame_width;
> > > > + bi.image_height = s->frame_height;
> > > > + bi.rowstride = pict->linesize[0];
> > > > +
> > > > + bi.blocks_per_row = (s->frame_width + 3) >> 2;
> > >
> > > align
> >
> > aligned
>
>
> But many more opportunities for alignment exist throughout your patch..
IMHO alignment is only of concern if the code is unreadable without
the extra cosmetic spaces. I feel that the code right now is quite
readable. If you feel otherwise, please mention the relevant section
of the hunk.
> > revised patch attached
>
> This revised patch leaves a few things to be desired. You are expected
> not only to address the issues pointed out, but also to look for similar
> issues in the rest of your code.
I'll review the code again, and wait for other comments while I'm at it.
[...]
> > --- libavcodec/rpzaenc.c (revision 0)
> > +++ libavcodec/rpzaenc.c (revision 0)
> > @@ -0,0 +1,848 @@
>
> > + min_r = FFMIN(block_ptr[(x * PIXELSTRIDE) + 2], min_r);
> > + min_g = FFMIN(block_ptr[(x * PIXELSTRIDE) + 1], min_g);
> > + min_b = FFMIN(block_ptr[(x * PIXELSTRIDE) + 0], min_b);
> > +
> > + max_r = FFMAX(block_ptr[(x * PIXELSTRIDE) + 2], max_r);
> > + max_g = FFMAX(block_ptr[(x * PIXELSTRIDE) + 1], max_g);
> > + max_b = FFMAX(block_ptr[(x * PIXELSTRIDE) + 0], max_b);
>
> possibly superfluous ()
As I said, for readability, but I'll remove these.
[...]
--
Regards,
Jai
More information about the ffmpeg-devel
mailing list