[FFmpeg-soc] [soc]: r3884 - amr/amrnbdec.c

[Robert Swain]

2008/12/15 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
>> 2008/12/15 Diego Biurrun <diego at 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 at gmail.com>:
>> >> >
>> >> >> 2008/12/15 diego <subversion at 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