[MPlayer-dev-eng] [PATCH] ve_lavc cosmetics

Michael Niedermayer michaelni at gmx.at
Sun Feb 12 18:35:57 CET 2006


Hi

On Sun, Feb 12, 2006 at 06:29:50PM +0200, Oded Shimon wrote:
> On Sun, Feb 12, 2006 at 03:52:23PM +0000, Robert Swain wrote:
> > Hello,
> > 
> > A while ago when I was trying to do MPEG-4 profiles in ve_lavc I noticed
> > how ugly libmpcodecs/ve_lavc.c was in terms of mixing styles, mixing
> > tabs and spaces for indentation, irregular indentation and so on. Today
> > I got bored so I decided to go through it and clean it up. So, this is
> > what I have done:
> > 
> > - All indentation is now four spaces per 'level'.
> > - Bad indentation has been fixed.
> > - Many long lines in the main code are now < 80 columns or thereabouts.
> > - There were a number of styles for if(){} paranthesis position and
> > indentation, these have been made uniform.
> > - This is just personal taste, I think, but there was a mix of a=b, a= b
> > and a = b. I prefer the latter so I have made this consistent
> > throughout.
> > - Around (patched) line 556 ['e = sscanf (lavc_param_aspect, "%f",
> > &ratio);'] there were {} but they are unnecessary as there was only one
> > line in the else statement so I removed these. I hope this doesn't have
> > any ABI connotations that I'm unaware of due to being a n00b. :)
> > 
> > Hopefully all this will be OK but if I've done anything idiotic or you
> > guys and gals really don't like any of it, please say.
> > 
> > Warning, the attached patch is difficult to read because of its nature
> > as a cosmetic diff.
> 
> Your patch contains cosmetics. rejected.
> 
> Oh.
> 
> 
> I'm for this patch, I can't go over it, just have to take your word for it 
> that there are absoloutely no functional changes. Is anyone against this? 
> I'd like to commit.

iam against it

i agree with 
* 4space indention
* trailing whitespace removial
* tab removial
* i agree with moving "{" which are on their own line

i do not agree with
* x=y / x = y changes
* breaking lines > 80 columns
* removing {} of if/for if theres just one statement inside
* reindenting lavcopts_conf table, this serves no purpose

[...]
-- 
Michael




More information about the MPlayer-dev-eng mailing list