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