On Wed, Jan 04, 2012 at 10:23:15PM -0600, Daniel Huang wrote:
> 

> From aba286b8f9b5b60dd4a3ef24c7083a13187d1f7c Mon Sep 17 00:00:00 2001
> From: Daniel <daniel@daniel-SX2803.(none)>
> Date: Wed, 4 Jan 2012 21:43:20 -0600
> Subject: [PATCH] cosmetics cleanup mjpegdec.c

Your git is not set up correctly; fix your name.


> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -42,40 +42,41 @@
>  
> -static int build_vlc(VLC *vlc, const uint8_t *bits_table, const uint8_t 
> *val_table,
> -                      int nb_codes, int use_static, int is_ac)
> +static int build_vlc(VLC *vlc, const uint8_t *bits_table, const uint8_t 
> *val_table, 
> +                     int nb_codes, int use_static, int is_ac)
>  {
> -    uint8_t huff_size[256];
> -    uint16_t huff_code[256];
> -    uint16_t huff_sym[256];
> -    int i;
> +    uint8_t    huff_size[256];
> +    uint16_t   huff_code[256];
> +    uint16_t   huff_sym[256];
> +    int        i;

No need for so many spaces.

> -    if(is_ac) huff_sym[0]= 16*256;
> +    if (is_ac) huff_sym[0] = 16 * 256;

Break this line.

> -static void build_basic_mjpeg_vlc(MJpegDecodeContext * s) {
> -    build_vlc(&s->vlcs[0][0], ff_mjpeg_bits_dc_luminance,
> -              ff_mjpeg_val_dc, 12, 0, 0);
> -    build_vlc(&s->vlcs[0][1], ff_mjpeg_bits_dc_chrominance,
> -              ff_mjpeg_val_dc, 12, 0, 0);
> -    build_vlc(&s->vlcs[1][0], ff_mjpeg_bits_ac_luminance,
> -              ff_mjpeg_val_ac_luminance, 251, 0, 1);
> -    build_vlc(&s->vlcs[1][1], ff_mjpeg_bits_ac_chrominance,
> -              ff_mjpeg_val_ac_chrominance, 251, 0, 1);
> -    build_vlc(&s->vlcs[2][0], ff_mjpeg_bits_ac_luminance,
> -              ff_mjpeg_val_ac_luminance, 251, 0, 0);
> -    build_vlc(&s->vlcs[2][1], ff_mjpeg_bits_ac_chrominance,
> -              ff_mjpeg_val_ac_chrominance, 251, 0, 0);
> +static void build_basic_mjpeg_vlc(MJpegDecodeContext *s) 
> +{
> +    build_vlc(&s->vlcs[0][0], ff_mjpeg_bits_dc_luminance, ff_mjpeg_val_dc, 
> 12, 0, 0);
> +    
> +    build_vlc(&s->vlcs[0][1], ff_mjpeg_bits_dc_chrominance, ff_mjpeg_val_dc, 
> 12, 0, 0);
> +    
> +    build_vlc(&s->vlcs[1][0], ff_mjpeg_bits_ac_luminance, 
> ff_mjpeg_val_ac_luminance, 251, 0, 1);
> +    
> +    build_vlc(&s->vlcs[1][1], ff_mjpeg_bits_ac_chrominance, 
> ff_mjpeg_val_ac_chrominance, 251, 0, 1);
> +    
> +    build_vlc(&s->vlcs[2][0], ff_mjpeg_bits_ac_luminance, 
> ff_mjpeg_val_ac_luminance, 251, 0, 0);
> +    
> +    build_vlc(&s->vlcs[2][1], 
> ff_mjpeg_bits_ac_chrominance,ff_mjpeg_val_ac_chrominance, 251, 0, 0);

Merging those lines was not an improvement.

> @@ -194,17 +192,15 @@ int ff_mjpeg_decode_dht(MJpegDecodeContext *s)
>  
>          /* build VLC and flush previous vlc if present */
>          free_vlc(&s->vlcs[class][index]);
> -        av_log(s->avctx, AV_LOG_DEBUG, "class=%d index=%d nb_codes=%d\n",
> +        av_log(s->avctx, AV_LOG_DEBUG, "class=%d index=%d nb_codes=%d\n", 

Trailing whitespace added - check the rest of your patch for more
instances.  Indeed many more remain:

biurrun@passion:~/src/priv/libav $ git am -s cosmetics-cleanup-mjpegdec.c.patch
Applying: cosmetics cleanup mjpegdec.c
/home/biurrun/src/priv/libav/.git/rebase-apply/patch:15: trailing whitespace.
static int build_vlc(VLC *vlc, const uint8_t *bits_table, const uint8_t 
*val_table,
/home/biurrun/src/priv/libav/.git/rebase-apply/patch:41: trailing whitespace.
    return init_vlc_sparse(vlc, 9, nb_codes, huff_size, 1, 1,
/home/biurrun/src/priv/libav/.git/rebase-apply/patch:58: trailing whitespace.
static void build_basic_mjpeg_vlc(MJpegDecodeContext *s)
/home/biurrun/src/priv/libav/.git/rebase-apply/patch:61: trailing whitespace.

/home/biurrun/src/priv/libav/.git/rebase-apply/patch:63: trailing whitespace.

warning: squelched 20 whitespace errors
warning: 25 lines add whitespace errors.


> -        if(class>0){
> +        if (class>0) {

spaces around <

>              free_vlc(&s->vlcs[2][index]);
> -            if(build_vlc(&s->vlcs[2][index], bits_table, val_table, code_max 
> + 1, 0, 0) < 0){
> -            return -1;
> -            }
> +            if (build_vlc(&s->vlcs[2][index], bits_table, val_table, 
> code_max + 1, 0, 0) < 0)
> +                return -1;
>          }

Removing just the { looks very suspicious.  Did you check compilation?

> @@ -216,12 +212,12 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>  
> -    if(s->pegasus_rct) s->bits=9;
> -    if(s->bits==9 && !s->pegasus_rct) s->rct=1;    //FIXME ugly
> +    if (s->pegasus_rct) s->bits = 9;
> +    if (s->bits == 9 && !s->pegasus_rct) s->rct = 1;    //FIXME ugly

Break these lines and look for more similar instances in the file.

> @@ -265,12 +261,14 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>  
> -    if(s->v_max==1 && s->h_max==1 && s->lossless==1) s->rgb=1;
> +    if(s->v_max == 1 && 
> +       s->h_max == 1 && 
> +       s->lossless == 1) s->rgb=1;

You broke this line at the wrong place; break after ')'.
Spaces after =.

> @@ -314,11 +312,11 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>      if(!(pix_fmt_id & 0x0D0D0D0D))

Here's one 'if(' that you overlooked, check the rest of the file yourself

>          pix_fmt_id-= (pix_fmt_id & 0x0F0F0F0F)>>1;

spaces around '-=' and '<<'.

> @@ -338,7 +336,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>      }
> -    if(s->ls){
> +    if (s->ls) {
>          if(s->nb_components > 1)
>              s->avctx->pix_fmt = PIX_FMT_RGB24;
>          else if(s->bits <= 8)

Two more overlooked 'if('.

I see more not so thorough work below.  Go through the file again,
fix the issues I mentioned and look for the same classes of mistakes.
Then resend your patch.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to