[FFmpeg-devel] [PATCH] RoQ encoder

[Diego Biurrun]

On Fri, May 04, 2007 at 10:50:04PM +0200, Vitor wrote:
> Diego Biurrun wrote:
> >The license header is wrong and your patch contains tabs and trailing
> >whitespace.
> Fixed. I also changed some function names and plugged a memory leak.

Thanks.  While we're at the nitpicking stage..

> --- libavcodec/roqaudioenc.c	(revision 0)
> +++ libavcodec/roqaudioenc.c	(revision 0)
> @@ -0,0 +1,256 @@
> +/*
> + * RoQ audio encoder
> + *
> + * Copyright (c) 2005 Eric Lasota
> + *    Based on RoQ specs (c)2001 Tim Ferguson
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include "avcodec.h"

Please leave a blank line between the header and the #include, looks
less cramped :)

> +    // Create a table of quick DPCM values

I think there is generally a slight preference for C-style /* */
comments..

> +    for(i=0;i<MAX_DPCM;i++)
> +    {
> +        if(diff < 0)
> +            diff = -diff;
> +        if(diff2 < 0)
> +            diff2 = -diff2;
> +    }

Please put a space between for/if and (, makes things more readable.

Diego