[libav-devel] [PATCH 2/2] h263dsp: K&R formatting cosmetics

Martin Storsjö martin at martin.st
Thu Nov 7 14:29:24 CET 2013


On Mon, 4 Nov 2013, Diego Biurrun wrote:

> ---
> libavcodec/h263dsp.c |  110 +++++++++++++++++++++++++++++---------------------
> 1 file changed, 63 insertions(+), 47 deletions(-)
>
> diff --git a/libavcodec/h263dsp.c b/libavcodec/h263dsp.c
> index 1166b93..63d0972 100644
> --- a/libavcodec/h263dsp.c
> +++ b/libavcodec/h263dsp.c
> @@ -23,78 +23,94 @@
> #include "config.h"
> #include "h263dsp.h"
>
> -const uint8_t ff_h263_loop_filter_strength[32]={
> -//  0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> -    0, 1, 1, 2, 2, 3, 3, 4, 4, 4, 5, 5, 6, 6, 7, 7, 7, 8, 8, 8, 9, 9, 9,10,10,10,11,11,11,12,12,12

This also removes some commented out code - that's maybe a part of a 
general cosmetic cleanup, but when the topic is "K&R formatting cosmetics" 
I would expect it to be a pure reindentation patch, because that's the 
only thing that K&R says anything about at all.

Please at least mention this in the commit message in some way. (Ideally 
please also dig through the git history to figure out what these commented 
out coefficientes are and why they were kept to begin with and mention 
that in the commit message.)

> +const uint8_t ff_h263_loop_filter_strength[32] = {
> +    0, 1, 1, 2, 2, 3, 3,  4,  4,  4,  5,  5,  6,  6,  7, 7,
> +    7, 8, 8, 8, 9, 9, 9, 10, 10, 10, 11, 11, 11, 12, 12, 12
> };
>
> -static void h263_v_loop_filter_c(uint8_t *src, int stride, int qscale){
> +static void h263_v_loop_filter_c(uint8_t *src, int stride, int qscale)
> +{
>     int x;
> -    const int strength= ff_h263_loop_filter_strength[qscale];
> +    const int strength = ff_h263_loop_filter_strength[qscale];
>
> -    for(x=0; x<8; x++){
> +    for (x = 0; x < 8; x++) {
>         int d1, d2, ad1;
> -        int p0= src[x-2*stride];
> -        int p1= src[x-1*stride];
> -        int p2= src[x+0*stride];
> -        int p3= src[x+1*stride];
> -        int d = (p0 - p3 + 4*(p2 - p1)) / 8;
> -
> -        if     (d<-2*strength) d1= 0;
> -        else if(d<-  strength) d1=-2*strength - d;
> -        else if(d<   strength) d1= d;
> -        else if(d< 2*strength) d1= 2*strength - d;
> -        else                   d1= 0;
> +        int p0 = src[x - 2 * stride];
> +        int p1 = src[x - 1 * stride];
> +        int p2 = src[x + 0 * stride];
> +        int p3 = src[x + 1 * stride];
> +        int d  = (p0 - p3 + 4 * (p2 - p1)) / 8;
> +
> +        if (d < -2 * strength)
> +            d1 = 0;
> +        else if (d < -strength)
> +            d1 = -2 * strength - d;
> +        else if (d < strength)
> +            d1 = d;
> +        else if (d < 2 * strength)
> +            d1 = 2 * strength - d;
> +        else
> +            d1 = 0;

Some might argue that this was more readable in the previous form, but I'm 
ok with it in this form as well, and this obviously matches the general 
formatting style closer. (Keeping things on one line is ok with me in the 
odd cases where it significantly helps readability - in this case I'm not 
sure how significantly it helps.)

// Martin


More information about the libav-devel mailing list