On Sat, Dec 08, 2012 at 02:17:04AM +0100, Luca Barbato wrote:
[...]
> -static const uint8_t run_bits[7][16]={
> -    {1,0},
> -    {1,1,0},
> -    {3,2,1,0},
> -    {3,2,1,1,0},
> -    {3,2,3,2,1,0},
> -    {3,0,1,3,2,5,4},
> -    {7,6,5,4,3,2,1,1,1,1,1,1,1,1,1},
> +static const uint8_t run_bits[7][16] = {
> +    { 1, 0 },
> +    { 1, 1, 0},
> +    { 3, 2, 1, 0},
> +    { 3, 2, 1, 1, 0},
> +    { 3, 2, 3, 2, 1, 0},
> +    { 3, 0, 1, 3, 2, 5, 4},
> +    { 7, 6, 5, 4, 3, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1},
>  };

Ending spaces.

> 
>  static VLC coeff_token_vlc[4];
> -static VLC_TYPE coeff_token_vlc_tables[520+332+280+256][2];
> -static const int coeff_token_vlc_tables_size[4]={520,332,280,256};
> +static VLC_TYPE coeff_token_vlc_tables[520 + 332 + 280 + 256][2];
> +static const int coeff_token_vlc_tables_size[4] = { 520, 332, 280, 256 };
> 
>  static VLC chroma_dc_coeff_token_vlc;
>  static VLC_TYPE chroma_dc_coeff_token_vlc_table[256][2];
>  static const int chroma_dc_coeff_token_vlc_table_size = 256;
> -

Why?

>  static VLC chroma422_dc_coeff_token_vlc;
>  static VLC_TYPE chroma422_dc_coeff_token_vlc_table[8192][2];
>  static const int chroma422_dc_coeff_token_vlc_table_size = 8192;
[...]
> -    tprintf(h->s.avctx, "pred_nnz L%X T%X n%d s%d P%X\n", left, top, n, 
> scan8[n], i&31);
> +    tprintf(h->s.avctx, "pred_nnz L%X T%X n%d s%d P%X\n", left, top, n,
> +            scan8[n],
> +            i & 31);

This break looks particularly stupid.

> 
> -    return i&31;
> +    return i & 31;
>  }
> 
> -static av_cold void init_cavlc_level_tab(void){
> +static av_cold void init_cavlc_level_tab(void)
> +{
>      int suffix_length;
>      unsigned int i;
> 
> -    for(suffix_length=0; suffix_length<7; suffix_length++){
> -        for(i=0; i<(1<<LEVEL_TAB_BITS); i++){
> -            int prefix= LEVEL_TAB_BITS - av_log2(2*i);
> +    for (suffix_length = 0; suffix_length < 7; suffix_length++)
> +        for (i = 0; i < (1 << LEVEL_TAB_BITS); i++) {
> +            int prefix = LEVEL_TAB_BITS - av_log2(2 * i);
> 

Removing the '{' of the initial for as well; you should configure
uncrustify better to detect these cases.

> -            if(prefix + 1 + suffix_length <= LEVEL_TAB_BITS){
> +            if (prefix + 1 + suffix_length <= LEVEL_TAB_BITS) {
>                  int level_code = (prefix << suffix_length) +
> -                    (i >> (av_log2(i) - suffix_length)) - (1 << 
> suffix_length);
> -                int mask = -(level_code&1);
> +                                 (i >> (av_log2(i) - suffix_length)) -
> +                                 (1 << suffix_length);
> +                int mask = -(level_code & 1);
>                  level_code = (((2 + level_code) >> 1) ^ mask) - mask;
> -                cavlc_level_tab[suffix_length][i][0]= level_code;
> -                cavlc_level_tab[suffix_length][i][1]= prefix + 1 + 
> suffix_length;
> -            }else if(prefix + 1 <= LEVEL_TAB_BITS){
> -                cavlc_level_tab[suffix_length][i][0]= prefix+100;
> -                cavlc_level_tab[suffix_length][i][1]= prefix + 1;
> -            }else{
> -                cavlc_level_tab[suffix_length][i][0]= LEVEL_TAB_BITS+100;
> -                cavlc_level_tab[suffix_length][i][1]= LEVEL_TAB_BITS;
> +                cavlc_level_tab[suffix_length][i][0] = level_code;
> +                cavlc_level_tab[suffix_length][i][1] = prefix + 1 +
> +                                                       suffix_length;
> +            } else if (prefix + 1 <= LEVEL_TAB_BITS) {
> +                cavlc_level_tab[suffix_length][i][0] = prefix + 100;
> +                cavlc_level_tab[suffix_length][i][1] = prefix + 1;
> +            } else {
> +                cavlc_level_tab[suffix_length][i][0] = LEVEL_TAB_BITS + 100;
> +                cavlc_level_tab[suffix_length][i][1] = LEVEL_TAB_BITS;
>              }
>          }
> -    }
>  }
> 
[...]
>  #ifdef TRACE
> -    print_bin(buf>>(32-log), log);
> -    av_log(NULL, AV_LOG_DEBUG, "%5d %2d %3d lpr @%5d in %s 
> get_level_prefix\n", buf>>(32-log), log, log-1, get_bits_count(gb), __FILE__);
> +    print_bin(buf >> (32 - log), log);
> +    av_log(NULL, AV_LOG_DEBUG, "%5d %2d %3d lpr @%5d in %s 
> get_level_prefix\n",
> +           buf >> (32 - log),
> +           log,
> +           log - 1,
> +           get_bits_count(gb), __FILE__);

Here and a lot more below, these breaks are pretty bad.

>  #endif
> 
>      LAST_SKIP_BITS(re, gb, log);
>      CLOSE_READER(re, gb);
> 
> -    return log-1;
> +    return log - 1;
>  }
> 
>  /**
> @@ -442,171 +462,195 @@ static inline int get_level_prefix(GetBitContext *gb){
>   * @param max_coeff number of coefficients in the block
>   * @return <0 if an error occurred
>   */
> -static int decode_residual(H264Context *h, GetBitContext *gb, DCTELEM 
> *block, int n, const uint8_t *scantable, const uint32_t *qmul, int max_coeff){
> -    MpegEncContext * const s = &h->s;
> -    static const int coeff_token_table_index[17]= {0, 0, 1, 1, 2, 2, 2, 2, 
> 3, 3, 3, 3, 3, 3, 3, 3, 3};
> +static int decode_residual(H264Context *h, GetBitContext *gb, DCTELEM *block,
> +                           int n, const uint8_t *scantable,
> +                           const uint32_t *qmul,
> +                           int max_coeff)
> +{
> +    MpegEncContext *const s = &h->s;
> +    static const int coeff_token_table_index[17] =
> +    { 0, 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3 };

No indent here?

[...]
> +    if (n >= LUMA_DC_BLOCK_INDEX) { \
> +        ((type *)block)[*scantable] = level[0]; \
> +        for (i = 1; i < total_coeff && zeros_left > 0; i++) { \
> +            if (zeros_left < 7) \
> +                run_before = \
> +                    get_vlc2(gb, run_vlc[zeros_left - 1].table, \
> +                             RUN_VLC_BITS, 1); \
>              else \
> -                run_before= get_vlc2(gb, run7_vlc.table, RUN7_VLC_BITS, 2); \
> -            zeros_left -= run_before; \
> -            scantable -= 1 + run_before; \
> -            ((type*)block)[*scantable]= level[i]; \
> +                run_before = get_vlc2(gb, run7_vlc.table, RUN7_VLC_BITS, 2); 
> \
> +            zeros_left                 -= run_before; \
> +            scantable                  -= 1 + run_before; \
> +            ((type *)block)[*scantable] = level[i]; \

Does it really make any sense to do that kind of non-sensical alignments?

[...]
> -            }else{
> -                fill_rectangle(&h->non_zero_count_cache[scan8[16]], 4, 4, 8, 
> 0, 1);
> -                fill_rectangle(&h->non_zero_count_cache[scan8[32]], 4, 4, 8, 
> 0, 1);
> +            } else {
> +                fill_rectangle(&h->non_zero_count_cache[scan8[16]],
> +                               4, 4,
> +                               8, 0,
> +                               1);
> +                fill_rectangle(&h->non_zero_count_cache[scan8[32]],
> +                               4, 4,
> +                               8, 0,
> +                               1);

:/

[...]
> -            }else{
> -                fill_rectangle(&h->non_zero_count_cache[scan8[16]], 4, 4, 8, 
> 0, 1);
> -                fill_rectangle(&h->non_zero_count_cache[scan8[32]], 4, 4, 8, 
> 0, 1);
> +            } else {
> +                fill_rectangle(&h->non_zero_count_cache[scan8[16]],
> +                               4, 4,
> +                               8, 0,
> +                               1);
> +                fill_rectangle(&h->non_zero_count_cache[scan8[32]],
> +                               4, 4,
> +                               8, 0,
> +                               1);

Don't you think the original style is better?

[...]

-- 
Clément B.

Attachment: pgpnkfQM4SQlm.pgp
Description: PGP signature

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

Reply via email to