Author: diego Date: Mon Dec 15 11:13:50 2008 New Revision: 3884 Log: K&R function declaration and whitespace cosmetics Modified: amr/amrnbdec.c Modified: amr/amrnbdec.c ============================================================================== --- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -76,8 +76,8 @@ typedef struct AMRContext { } AMRContext; -static int amrnb_decode_init(AVCodecContext *avctx) { - +static int amrnb_decode_init(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data; // variables needed for cos table generation @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont * @return Returns the frame mode */ -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{ AMRContext *p = avctx->priv_data; enum Mode mode; int i; @@ -214,7 +215,8 @@ enum Mode decode_bitstream(AVCodecContex * @param min_dist minimum required separation of lsfs */ -static void reorder_lsf(int *lsf, int min_dist) { +static void reorder_lsf(int *lsf, int min_dist) +{ int i; int lsf_min = min_dist; @@ -234,7 +236,8 @@ static void reorder_lsf(int *lsf, int mi * @param lsp a vector of lsps */ -static void lsf2lsp(int *lsf, int *lsp) { +static void lsf2lsp(int *lsf, int *lsp) +{ int i; int index, offset; @@ -252,8 +255,8 @@ static void lsf2lsp(int *lsf, int *lsp) * @param avctx pointer to the AVCodecContext for AMR */ -static void decode_lsf2lsp_3(AVCodecContext *avctx) { - +static void decode_lsf2lsp_3(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data; int lsf1_r[LP_FILTER_ORDER]; // vector of residual lsfs @@ -337,8 +340,8 @@ static void decode_lsf2lsp_3(AVCodecCont * @param avctx pointer to the AVCodecContext for AMR */ -static void decode_lsf2lsp_5(AVCodecContext *avctx) { - +static void decode_lsf2lsp_5(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data; int lsf1_r[LP_FILTER_ORDER], lsf2_r[LP_FILTER_ORDER]; // vectors of residual lsfs @@ -421,7 +424,8 @@ static void decode_lsf2lsp_5(AVCodecCont * @return void */ -static void lsp2poly(int *lsp, int *f) { +static void lsp2poly(int *lsp, int *f) +{ int i, j; f[0] = 1<<24; @@ -458,7 +462,8 @@ static void lsp2poly(int *lsp, int *f) { * @return void */ -static void lsp2lpc(int *lsp, int *lpc_coeffs) { +static void lsp2lpc(int *lsp, int *lpc_coeffs) +{ int f1[6], f2[6]; int temp, i; @@ -501,7 +506,8 @@ static void lsp2lpc(int *lsp, int *lpc_c * @return void */ -static void lpc_interp_13(AVCodecContext *avctx, int **lpc_coeffs) { +static void lpc_interp_13(AVCodecContext *avctx, int **lpc_coeffs) +{ AMRContext *p = avctx->priv_data; int lsp[LP_FILTER_ORDER]; int i; @@ -539,7 +545,8 @@ static void lpc_interp_13(AVCodecContext * @return void */ -static void lpc_interp_123(AVCodecContext *avctx, int **lpc_coeffs) { +static void lpc_interp_123(AVCodecContext *avctx, int **lpc_coeffs) +{ AMRContext *p = avctx->priv_data; int lsp[LP_FILTER_ORDER]; int i; @@ -590,7 +597,9 @@ static void lpc_interp_123(AVCodecContex * @return void */ -static void decode_pitch_lag_3(AVCodecContext *avctx, int pitch_index, int *pitch_lag_int, int *pitch_lag_frac) { +static void decode_pitch_lag_3(AVCodecContext *avctx, int pitch_index, + int *pitch_lag_int, int *pitch_lag_frac) +{ AMRContext *p = avctx->priv_data; int tmp_lag; @@ -659,7 +668,9 @@ static void decode_pitch_lag_3(AVCodecCo * @return void */ -static void decode_pitch_lag_6(AVCodecContext *avctx, int pitch_index, int *pitch_lag_int, int *pitch_lag_frac) { +static void decode_pitch_lag_6(AVCodecContext *avctx, int pitch_index, + int *pitch_lag_int, int *pitch_lag_frac) +{ AMRContext *p = avctx->priv_data; int temp; @@ -707,7 +718,8 @@ static void decode_pitch_lag_6(AVCodecCo * @return void */ -static void decode_pitch_vector(AVCodecContext *avctx, int *excitation) { +static void decode_pitch_vector(AVCodecContext *avctx, int *excitation) +{ AMRContext *p = avctx->priv_data; int i, j, temp; int *excitation_temp; @@ -744,7 +756,9 @@ static void decode_pitch_vector(AVCodecC * @return void */ -static void reconstruct_fixed_code(int *fixed_code, int *pulse_position, int sign, int nr_pulses) { +static void reconstruct_fixed_code(int *fixed_code, int *pulse_position, + int sign, int nr_pulses) +{ int i; // reset the code @@ -765,7 +779,8 @@ static void reconstruct_fixed_code(int * * @return void */ -static void fixed2position(int16_t *fixed_index, int *position_index) { +static void fixed2position(int16_t *fixed_index, int *position_index) +{ int MSBs, LSBs, MSBs0_24, divMSB; // indices from track 1 (7+3 bits) @@ -820,7 +835,9 @@ static void fixed2position(int16_t *fixe * @return void */ -static void decode_2_pulses_9bits(AVCodecContext *avctx, int sign, int fixed_index, int *fixed_code) { +static void decode_2_pulses_9bits(AVCodecContext *avctx, int sign, + int fixed_index, int *fixed_code) +{ AMRContext *p = avctx->priv_data; int pulse_position[2]; int pulse_subset; @@ -848,7 +865,8 @@ static void decode_2_pulses_9bits(AVCode * @return void */ -static void decode_2_pulses_11bits(int sign, int fixed_index, int *fixed_code) { +static void decode_2_pulses_11bits(int sign, int fixed_index, int *fixed_code) +{ int pulse_position[2]; int pulse_subset; @@ -881,7 +899,8 @@ static void decode_2_pulses_11bits(int s * @return void */ -static void decode_3_pulses_14bits(int sign, int fixed_index, int *fixed_code) { +static void decode_3_pulses_14bits(int sign, int fixed_index, int *fixed_code) +{ int pulse_position[3]; int pulse_subset; @@ -912,7 +931,8 @@ static void decode_3_pulses_14bits(int s * @return void */ -static void decode_4_pulses_17bits(int sign, int fixed_index, int *fixed_code) { +static void decode_4_pulses_17bits(int sign, int fixed_index, int *fixed_code) +{ int pulse_position[4]; int pulse_subset; @@ -942,7 +962,8 @@ static void decode_4_pulses_17bits(int s * @return void */ -static void decode_8_pulses_31bits(int16_t *fixed_index, int *fixed_code) { +static void decode_8_pulses_31bits(int16_t *fixed_index, int *fixed_code) +{ int position_index[8]; int i, pos1, pos2, sign; @@ -977,7 +998,8 @@ static void decode_8_pulses_31bits(int16 * @return void */ -static void decode_10_pulses_35bits(int16_t *fixed_index, int *fixed_code) { +static void decode_10_pulses_35bits(int16_t *fixed_index, int *fixed_code) +{ int i, pos1, pos2, sign; // reset the code @@ -1010,7 +1032,8 @@ static void decode_10_pulses_35bits(int1 * @return a-b : the result of the comparison */ -int qsort_compare(const int *a, const int *b) { +int qsort_compare(const int *a, const int *b) +{ return (int)(*a - *b); } @@ -1022,7 +1045,8 @@ int qsort_compare(const int *a, const in * @return Returns the median value */ -static int median(int *values, int n) { +static int median(int *values, int n) +{ int temp[9]; // largest n used for median calculation is 9 memcpy(values, temp, n*sizeof(int)); @@ -1042,7 +1066,8 @@ static int median(int *values, int n) { * @return Returns the pitch gain */ -static int find_pitch_gain(AMRDecoderState *state_ptr) { +static int find_pitch_gain(AMRDecoderState *state_ptr) +{ int temp_median; // find the median of the previous five pitch gains @@ -1064,7 +1089,8 @@ static int find_pitch_gain(AMRDecoderSta * @return Returns the pitch gain */ -static int decode_pitch_gain(enum Mode mode, int index) { +static int decode_pitch_gain(enum Mode mode, int index) +{ int gain; if(mode == MODE_122) { @@ -1086,8 +1112,9 @@ static int decode_pitch_gain(enum Mode m * @param pitch_gain pointer to the pitch gain */ -static void pitch_gain_update(AMRDecoderState *state_ptr, int bad_frame_indicator, - int *pitch_gain) { +static void pitch_gain_update(AMRDecoderState *state_ptr, + int bad_frame_indicator, int *pitch_gain) +{ if(bad_frame_indicator == 0) { if(state_ptr->prev_frame_bad != 0) { // if the previous frame was bad, limit the current pitch gain to @@ -1121,7 +1148,8 @@ static void pitch_gain_update(AMRDecoder * @param avctx pointer to the AVCodecContext for AMR */ -void decode_reset(AVCodecContext *avctx) { +void decode_reset(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data; p->prev_frame_homing = 1; @@ -1131,9 +1159,9 @@ void decode_reset(AVCodecContext *avctx) } -static int amrnb_decode_frame(AVCodecContext *avctx, - void *data, int *data_size, uint8_t *buf, int buf_size) { - +static int amrnb_decode_frame(AVCodecContext *avctx, void *data, + int *data_size, uint8_t *buf, int buf_size) +{ AMRContext *p = avctx->priv_data; // pointer to private data int16_t *outbuffer = data; // pointer to the output data buffer int i; // counter @@ -1281,8 +1309,8 @@ static int amrnb_decode_frame(AVCodecCon } -static int amrnb_decode_close(AVCodecContext *avctx) { - +static int amrnb_decode_close(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data; /* Free allocated memory */ @@ -1295,8 +1323,7 @@ static int amrnb_decode_close(AVCodecCon } -AVCodec amrnb_decoder = -{ +AVCodec amrnb_decoder = { .name = "amrnb", .type = CODEC_TYPE_AUDIO, .id = CODEC_ID_AMR_NB,
2008/12/15 diego <subversion@mplayerhq.hu>:
Author: diego Date: Mon Dec 15 11:13:50 2008 New Revision: 3884
Log: K&R function declaration and whitespace cosmetics
Modified: amr/amrnbdec.c
Modified: amr/amrnbdec.c ============================================================================== --- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -76,8 +76,8 @@ typedef struct AMRContext { } AMRContext;
-static int amrnb_decode_init(AVCodecContext *avctx) { - +static int amrnb_decode_init(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data;
// variables needed for cos table generation @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont * @return Returns the frame mode */
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing. Rob
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Author: diego Date: Mon Dec 15 11:13:50 2008 New Revision: 3884
Log: K&R function declaration and whitespace cosmetics
Modified: amr/amrnbdec.c
Modified: amr/amrnbdec.c ============================================================================== --- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -76,8 +76,8 @@ typedef struct AMRContext { } AMRContext;
-static int amrnb_decode_init(AVCodecContext *avctx) { - +static int amrnb_decode_init(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data;
// variables needed for cos table generation @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont * @return Returns the frame mode */
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :) Rob
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Author: diego Date: Mon Dec 15 11:13:50 2008 New Revision: 3884
Log: K&R function declaration and whitespace cosmetics
Modified: amr/amrnbdec.c
Modified: amr/amrnbdec.c ============================================================================== --- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -76,8 +76,8 @@ typedef struct AMRContext { } AMRContext;
-static int amrnb_decode_init(AVCodecContext *avctx) { - +static int amrnb_decode_init(AVCodecContext *avctx) +{ AMRContext *p = avctx->priv_data;
// variables needed for cos table generation @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont * @return Returns the frame mode */
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
Rob
I do, it is more readable to me. More code per loc. MvH Benjamin Larsson
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Log: K&R function declaration and whitespace cosmetics
--- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer if (condition) statement; over if (condition) statement and similar. But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24: Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace. Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange. Diego
Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Log: K&R function declaration and whitespace cosmetics
--- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Diego
Well you have a few instances in the h264 decoder to take care of then. I still thinks it's abit over the edge to complain about this, but I wont argue any more about it. MvH Benjamin Larsson
On Mon, Dec 15, 2008 at 02:25:22PM +0100, Benjamin Larsson wrote:
Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Log: K&R function declaration and whitespace cosmetics
--- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Well you have a few instances in the h264 decoder to take care of then.
Probably in more places, I'll see if I get to it one of these days. Diego
On Mon, Dec 15, 2008 at 03:08:23PM +0100, baisong@sict.ac.cn wrote: [...] List administrators, please unsubscribe this guy, I'm getting an autoreply for every single mail I send to this mailing list. Diego
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Log: K&R function declaration and whitespace cosmetics
--- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this. If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active. Regards, Rob
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Log: K&R function declaration and whitespace cosmetics
--- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too.
Fair enough, I will. Diego
On Mon, Dec 15, 2008 at 03:12:59PM +0100, Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
> Log: > K&R function declaration and whitespace cosmetics > > --- amr/amrnbdec.c (original) > +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 > @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont > > -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { > - > +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, > + enum Mode *speech_mode) > +{ > Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too.
Fair enough, I will.
Done. Diego
2008/12/27 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 03:12:59PM +0100, Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
> 2008/12/15 diego <subversion@mplayerhq.hu>: > >> Log: >> K&R function declaration and whitespace cosmetics >> >> --- amr/amrnbdec.c (original) >> +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 >> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont >> >> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { >> - >> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, >> + enum Mode *speech_mode) >> +{ >> > Urgh. I'm happy with the line breaks but I don't tend to like the > opening { on a new line. I thought that was a GNU thing not a K&R > thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too.
Fair enough, I will.
Done.
I meant throughout the entirety of FFmpeg. It still makes me wince to see the opening { on a new line. :/ Rob
On Sat, Dec 27, 2008 at 08:01:45PM +0000, Robert Swain wrote:
2008/12/27 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 03:12:59PM +0100, Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote: > 2008/12/15 Robert Swain <robert.swain@gmail.com>: > >> 2008/12/15 diego <subversion@mplayerhq.hu>: >> >>> Log: >>> K&R function declaration and whitespace cosmetics >>> >>> --- amr/amrnbdec.c (original) >>> +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 >>> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont >>> >>> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { >>> - >>> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, >>> + enum Mode *speech_mode) >>> +{ >>> >> Urgh. I'm happy with the line breaks but I don't tend to like the >> opening { on a new line. I thought that was a GNU thing not a K&R >> thing. > > Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too.
Fair enough, I will.
Done.
I meant throughout the entirety of FFmpeg.
.. some other day ..
It still makes me wince to see the opening { on a new line. :/
Start doing it, you'll pick up the habit... Diego
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
Log: K&R function declaration and whitespace cosmetics
--- amr/amrnbdec.c (original) +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
-enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { - +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, + enum Mode *speech_mode) +{
Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon. and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly. Also i think that code should not be reformatted against the maintainers wishes. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact
2008/12/15 Michael Niedermayer <michaelni@gmx.at>:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
> Log: > K&R function declaration and whitespace cosmetics > > --- amr/amrnbdec.c (original) > +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 > @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont > > -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { > - > +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, > + enum Mode *speech_mode) > +{ > Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon.
I think if K&R style was agreed upon then this issue is a subset of that style and so was agreed upon.
and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly.
Seems reasonable.
Also i think that code should not be reformatted against the maintainers wishes.
I felt a bit like this after the change was made, but this would lead to inconsistency of style. Whether consistency of style is paramount or not, I don't know. Rob
On Mon, Dec 15, 2008 at 07:22:26PM +0000, Robert Swain wrote:
2008/12/15 Michael Niedermayer <michaelni@gmx.at>:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
> 2008/12/15 diego <subversion@mplayerhq.hu>: > >> Log: >> K&R function declaration and whitespace cosmetics >> >> --- amr/amrnbdec.c (original) >> +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 >> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont >> >> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { >> - >> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, >> + enum Mode *speech_mode) >> +{ >> > Urgh. I'm happy with the line breaks but I don't tend to like the > opening { on a new line. I thought that was a GNU thing not a K&R > thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon.
I think if K&R style was agreed upon then this issue is a subset of that style and so was agreed upon.
and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly.
Seems reasonable.
Also i think that code should not be reformatted against the maintainers wishes.
I felt a bit like this after the change was made, but this would lead to inconsistency of style. Whether consistency of style is paramount or not, I don't know.
well, iam trying not to piss off maintainers due to a newline or not. If some maintainer feel strongly about the {} placement i think he should be able to place them as he prefers in code he maintains. Most people do not feel that strongly about it and thus most code should be consistant and the rest, well, saying it directly its a matter of unmaintained code vs code with slightly different {} placement. Its not as if we had that many people volunteering for maintaining each part that we should impose cosmetic rules if someone is strongly against them. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire
On Mon, Dec 15, 2008 at 07:59:52PM +0100, Michael Niedermayer wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
2008/12/15 diego <subversion@mplayerhq.hu>:
> Log: > K&R function declaration and whitespace cosmetics > > --- amr/amrnbdec.c (original) > +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 > @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont > > -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { > - > +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, > + enum Mode *speech_mode) > +{ > Urgh. I'm happy with the line breaks but I don't tend to like the opening { on a new line. I thought that was a GNU thing not a K&R thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon.
Set by Fabrice before your time? The coding rules are very clear: The presentation is the one specified by 'indent -i4 -kr -nut'. This includes clear rules about brace placement in function definitions.
and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly.
I think following some well-known style is advisable. Everybody will have to make some compromises for this. Anyway, this is not the best place to discuss this. Diego
On Sat, Dec 27, 2008 at 01:34:35PM +0100, Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 07:59:52PM +0100, Michael Niedermayer wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote:
2008/12/15 Robert Swain <robert.swain@gmail.com>:
> 2008/12/15 diego <subversion@mplayerhq.hu>: > >> Log: >> K&R function declaration and whitespace cosmetics >> >> --- amr/amrnbdec.c (original) >> +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 >> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont >> >> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { >> - >> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, >> + enum Mode *speech_mode) >> +{ >> > Urgh. I'm happy with the line breaks but I don't tend to like the > opening { on a new line. I thought that was a GNU thing not a K&R > thing.
Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon.
Set by Fabrice before your time? The coding rules are very clear:
The presentation is the one specified by 'indent -i4 -kr -nut'.
This includes clear rules about brace placement in function definitions.
it does, but 1. as said { placement of functions hasnt been discussed as far as i know (you arent saying it has been discussed ...) 2. it hasnt been agreed upon, rather above seems more a guideline choosen by fabrice. And in that fabrice did not enforce it AFAIK, so raising some fineprint of a complex guidline now to a strict law that should be followed letter by letter seems a little strange to me.
and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly.
I think following some well-known style is advisable. Everybody will have to make some compromises for this.
following the style that most people working on ffmpeg prefer means fewer compromises than following one out of 3 well known styles. Thats simply because the style of fewest compromises is likely not exactly one of the 3. Besides, we dont enforce the fineprint of "indent -i4 -kr -nut" anyway so for this to matter much we first would have to strictly enforce this and i belive strictly enforcing any style guidline is a bad idea. Simply because a few good developers have their personal preferances on minor details of the coding style and are rather emotional about it. Loosing good developers because of some fineprint in some style seems a bad choice ...
Anyway, this is not the best place to discuss this.
no, feel free to start a thread on ffmpeg-dev if the { placement of functions is that important to you. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin
On Sat, Dec 27, 2008 at 03:03:49PM +0100, Michael Niedermayer wrote:
On Sat, Dec 27, 2008 at 01:34:35PM +0100, Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 07:59:52PM +0100, Michael Niedermayer wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
Robert Swain wrote: > 2008/12/15 Robert Swain <robert.swain@gmail.com>: > >> 2008/12/15 diego <subversion@mplayerhq.hu>: >> >>> Log: >>> K&R function declaration and whitespace cosmetics >>> >>> --- amr/amrnbdec.c (original) >>> +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 >>> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont >>> >>> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { >>> - >>> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, >>> + enum Mode *speech_mode) >>> +{ >>> >> Urgh. I'm happy with the line breaks but I don't tend to like the >> opening { on a new line. I thought that was a GNU thing not a K&R >> thing. > > Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon.
Set by Fabrice before your time? The coding rules are very clear:
The presentation is the one specified by 'indent -i4 -kr -nut'.
This includes clear rules about brace placement in function definitions.
it does, but 1. as said { placement of functions hasnt been discussed as far as i know (you arent saying it has been discussed ...)
2. it hasnt been agreed upon, rather above seems more a guideline choosen by fabrice. And in that fabrice did not enforce it AFAIK, so raising some fineprint of a complex guidline now to a strict law that should be followed letter by letter seems a little strange to me.
Nobody is doing that, the files I edited are not following K&R style strictly, before or after my changes. You have always treated the coding rules as a binding document even though large parts of it have not been agreed upon, much less by most of the developers, some parts it seems, not even by you..
and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly.
I think following some well-known style is advisable. Everybody will have to make some compromises for this.
following the style that most people working on ffmpeg prefer means fewer compromises than following one out of 3 well known styles. Thats simply because the style of fewest compromises is likely not exactly one of the 3.
I doubt it.
Loosing good developers because of some fineprint in some style seems a bad choice ...
I see no risk of this happening now nor in the future. Diego
On Sat, Dec 27, 2008 at 03:20:48PM +0100, Diego Biurrun wrote:
On Sat, Dec 27, 2008 at 03:03:49PM +0100, Michael Niedermayer wrote:
On Sat, Dec 27, 2008 at 01:34:35PM +0100, Diego Biurrun wrote:
On Mon, Dec 15, 2008 at 07:59:52PM +0100, Michael Niedermayer wrote:
On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
2008/12/15 Diego Biurrun <diego@biurrun.de>:
On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote: > Robert Swain wrote: > > 2008/12/15 Robert Swain <robert.swain@gmail.com>: > > > >> 2008/12/15 diego <subversion@mplayerhq.hu>: > >> > >>> Log: > >>> K&R function declaration and whitespace cosmetics > >>> > >>> --- amr/amrnbdec.c (original) > >>> +++ amr/amrnbdec.c Mon Dec 15 11:13:50 2008 > >>> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont > >>> > >>> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) { > >>> - > >>> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, > >>> + enum Mode *speech_mode) > >>> +{ > >>> > >> Urgh. I'm happy with the line breaks but I don't tend to like the > >> opening { on a new line. I thought that was a GNU thing not a K&R > >> thing. > > > > Nope, it is K&R. Hmm, then who likes them on the same line other than me? :) > > I do, it is more readable to me. More code per loc.
With that kind of reasoning, we can also prefer
if (condition) statement;
over
if (condition) statement
and similar.
But this discussion is completely pointless IMO. The rules have been set in http://ffmpeg.org/general.html#SEC24:
Indent size is 4. The presentation is the one specified by 'indent -i4 -kr -nut'. The TAB character is forbidden outside of Makefiles as is any form of trailing whitespace.
Now it's clear that each person will dislike some part of K&R style and prefer to do things in other ways. But the nature of compromises is exactly that: You accept a few things you may not be terribly fond of and you get a uniform style in exchange.
Mmm. There are a fair few instances of { being on the same line as the function declaration so I guess you'll have to do those too. I still don't like it but it is personal preference and if that's what's been agreed, I won't argue about something like this.
If it hadn't been agreed project-wide, I would have preferred to have been consulted about the changes before they were committed considering it's my code. Even if I haven't touched it for a while, I am still active.
i dont remember { placement for functions being discussed or agreed upon.
Set by Fabrice before your time? The coding rules are very clear:
The presentation is the one specified by 'indent -i4 -kr -nut'.
This includes clear rules about brace placement in function definitions.
it does, but 1. as said { placement of functions hasnt been discussed as far as i know (you arent saying it has been discussed ...)
2. it hasnt been agreed upon, rather above seems more a guideline choosen by fabrice. And in that fabrice did not enforce it AFAIK, so raising some fineprint of a complex guidline now to a strict law that should be followed letter by letter seems a little strange to me.
Nobody is doing that, the files I edited are not following K&R style strictly, before or after my changes.
You have always treated the coding rules as a binding document
If by coding rules you mean coding style, then i think you are wrong i did not treat the coding style as strictly binding, rather as a recommandition that people should follow but that exceptions would be no problem if someone feels strongly about something. If OTOH you mean rules related to coding quality or svnlog/history reviewabilty then yes i did and do consider these rules important and reject patches if they are not followed.
even though large parts of it have not been agreed upon, much less by most of the developers, some parts it seems, not even by you..
If you or someone else thinks some part of it is bad, you can start a discussion about it on ffmpeg-dev.
and i prefer them on the same line as well. Though i am not strongly opposed to following K&R, just that if most people prefer them like we do, that following K&R just because of it would be silly.
I think following some well-known style is advisable. Everybody will have to make some compromises for this.
following the style that most people working on ffmpeg prefer means fewer compromises than following one out of 3 well known styles. Thats simply because the style of fewest compromises is likely not exactly one of the 3.
I doubt it.
i see [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I wish the Xiph folks would stop pretending they've got something they do not. Somehow I fear this will remain a wish. -- Måns Rullgård
participants (5)
-
Benjamin Larsson -
diego -
Diego Biurrun -
Michael Niedermayer -
Robert Swain