[Ffmpeg-devel] [PATCH] from DivX, Part 1: cosmectic changes

The Wanderer inverseparadox
Fri Dec 16 07:20:18 CET 2005


Steve Lhomme wrote:

> Hi everyone,
> 
> This is the first patch from DivX in order to make our code as
> similar as the official FFMPEG as possible.
> 
> This patch includes only the cosmetic changes so that after that we
> can proceed with the real juice (otherwise the noise from these
> changes is too big to find the actual fixes).
> 
> It has :
> - tab removal (yes the official code has tabs)

That's definitely a cosmetic change.

> - use av_log instead of fprintf

That's not cosmetic, but it should be done anyway.

> - missing includes (help MSVC at least and won't hurt anyone)

That's not purely cosmetic, although if the code compiles without them
it's arguably close.

> - a few #ifdef to hide code when it's not enabled in the main config
> (to avoid having to fix MSVC portability issues)

That's not cosmetic, although the first example of it I noticed in a
quick look at the patch should be done anyway.

> - more PRId64, PRIx64, etc

That's probably not cosmetic either.

Any change which affects behaviour - either when compiling or when
running - is *not* purely cosmetic. As I understand matters, general
policy is that cosmetic changes and functional changes should not be
made in the same commit, although there have been occasional exceptions
made and this patch (at first glance) doesn't seem to violate the spirit
of the rule.

I'm not saying "patch rejected" (I definitely don't have the authority
to do that, and I'm not sure it would be the right decision anyway);
what I am saying that this is not in any sense a cosmetics-only patch.

(Yow! It certainly is extensive, though!)

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.





More information about the ffmpeg-devel mailing list