[FFmpeg-devel] [PATCH] K&R style format of submitted code in g729* files (cosmetics)

Diego Biurrun diego
Sat Jun 6 12:35:46 CEST 2009


On Sat, Jun 06, 2009 at 12:53:31PM +0700, Vladimir Voroshilov wrote:
> 2009/6/6 Diego Biurrun <diego at biurrun.de>:
> > On Sat, Jun 06, 2009 at 01:19:02AM +0700, Vladimir Voroshilov wrote:
> >> 2009/6/5 Diego Biurrun <diego at biurrun.de>:
> >> >
> >> > Your function calls still look quite idiosyncratic. ?You could change
> >> >
> >> > ?ff_acelp_weighted_vector_sum(
> >> > ? ? ? ? ?fc + pitch_delay_int[i],
> >> > ? ? ? ? ?fc + pitch_delay_int[i],
> >> > ? ? ? ? ?fc,
> >> > ? ? ? ? ?1 << 14,
> >> > ? ? ? ? ?av_clip(ctx->gain_pitch, SHARP_MIN, SHARP_MAX),
> >> > ? ? ? ? ?0,
> >> > ? ? ? ? ?14,
> >> > ? ? ? ? ?ctx->subframe_size - pitch_delay_int[i]);
> >> >
> >> > to something like
> >> >
> >> > ?ff_acelp_weighted_vector_sum(fc + pitch_delay_int[i],
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fc + pitch_delay_int[i],
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fc, 1 << 14,
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? av_clip(ctx->gain_pitch, SHARP_MIN, SHARP_MAX),
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0, 14,
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ctx->subframe_size - pitch_delay_int[i]);
> >> >
> >> > or similar, depending on where you wish to break the lines.
> >>
> >> Updated patch
> >
> > Ummm, still room for improvement...
> >
> >> --- ffmpeg-r19118.orig/libavcodec/g729dec.c
> >> +++ ffmpeg-r19118.mod/libavcodec/g729dec.c
> >> @@ -98,53 +97,40 @@ static inline int get_parity(uint8_t value)
> >>
> >> - ? ? ? ?ff_acelp_weighted_vector_sum(
> >> - ? ? ? ? ? ? ? ?fc + pitch_delay_int[i],
> >> - ? ? ? ? ? ? ? ?fc + pitch_delay_int[i],
> >> - ? ? ? ? ? ? ? ?fc,
> >> - ? ? ? ? ? ? ? ?1 << 14,
> >> + ? ? ? ?ff_acelp_weighted_vector_sum(fc + pitch_delay_int[i],
> >> + ? ? ? ? ? ? ? ?fc + pitch_delay_int[i],
> >> + ? ? ? ? ? ? ? ?fc, 1 << 14,
> >> ? ? ? ? ? ? ? ? ?av_clip(ctx->gain_pitch, SHARP_MIN, SHARP_MAX),
> >> - ? ? ? ? ? ? ? ?0,
> >> - ? ? ? ? ? ? ? ?14,
> >> + ? ? ? ? ? ? ? ?0, 14,
> >> ? ? ? ? ? ? ? ? ?ctx->subframe_size - pitch_delay_int[i]);
> >
> > You should indent this so that the parameters align, not randomly.
> 
> Sorry, but i don't understand what do you mean.
> I see only one difference with suggested code: 8 spaces instead of 19
> in indented lines. Do you mean that lines should be indented to
> position of opening parenthesis?

I mean that you should not indent by a random amount like 8 spaces, but
indent so that the function arguments align after the opening
parenthesis.  This style is used all over FFmpeg, like this:

  ff_foo_bar(param1, param2,
             param3, param4);

Diego



More information about the ffmpeg-devel mailing list