[FFmpeg-devel] [PATCH] add colours to warnings and errors

James Darnley james.darnley
Mon Apr 26 11:57:34 CEST 2010


On 26 April 2010 11:28, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Mon, Apr 26, 2010 at 12:22:08AM +0200, James Darnley wrote:
>> On 25 April 2010 23:59, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Sun, Apr 25, 2010 at 11:57:08PM +0200, James Darnley wrote:
>> >> On 25 April 2010 23:21, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Sun, Apr 25, 2010 at 10:40:19PM +0200, James Darnley wrote:
>> >> >> On 25 April 2010 20:29, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> >> >> ?log.c | ? 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>> >> >> >> ?1 file changed, 71 insertions(+), 7 deletions(-)
>> >> >> >> bbd24cb9a7eb75393a8fb35fc3ca620a0edb0af7 ?colours.diff
>> >> >> >> Index: libavutil/log.c
>> >> >> >> ===================================================================
>> >> >> >> --- libavutil/log.c ? (revision 22960)
>> >> >> >> +++ libavutil/log.c ? (working copy)
>> >> >> >> @@ -24,6 +24,10 @@
>> >> >> >> ? * logging functions
>> >> >> >> ? */
>> >> >> >>
>> >> >> >> +#ifdef _WIN32
>> >> >> >> +#include <windows.h>
>> >> >> >> +#include <string.h>
>> >> >> >> +#endif
>> >> >> >> ?#include <unistd.h>
>> >> >> >> ?#include <stdlib.h>
>> >> >> >> ?#include "avutil.h"
>> >> >> >> @@ -34,24 +38,84 @@
>> >> >> >> ?#endif
>> >> >> >> ?int av_log_level = AV_LOG_INFO;
>> >> >> >>
>> >> >> >> +/* FIXME: On Windows isatty() returns true when ANSI color codes won't work.
>> >> >> >> +Some hack to detect output to other terminals would be good, fixing the other
>> >> >> >> +terminals would be better. One probable exception is when the user has
>> >> >> >> +ANSI.SYS loaded but the Windows API should then still work. */
>> >> >> >> +
>> >> >> >
>> >> >> >> +#if !HAVE_ISATTY
>> >> >> >> +#define isatty(2) 0
>> >> >> >> +#endif
>> >> >> >
>> >> >> > does HAVE_ISATTY && isatty(2) work? if so this is prefered over the ifdeffery
>> >> >> >
>> >> >>
>> >> >> I've got no idea because I don't have a system without it. ?But surely
>> >> >> if you don't have it trying to use isatty() will result in a compile
>> >> >> error. ?I'm just moving your code about there (and adding a not).
>> >> >
>> >> > the compiler should optimize the call away, we use such code elsewhere.
>> >> > If you want to argue this violates ISO C then you have a point but
>> >> > its a point against using this anywhere not a point against this specific
>> >> > case.
>> >> > Thus as it stands please use it or start a thread asking us to remove it
>> >> > everywhere. The intermediate of using this trick at some places and not
>> >> > at others is surely a bad idea.
>> >>
>> >> Sorry, I slightly misread what you typed the first time. ?Yes, my test
>> >> with the following does not result in a compile error only a warning
>> >> of "implicit declaration of function"
>> >> #undef HAVE_ISATTY
>> >> #define HAVE_ISATTY 0
>> >> #define isatty(x) this_is_not_a_function( x )
>> >>
>> >> No, I don't want to argue about correctness. ?I'll leave that to
>> >> people who actually know what is.
>> >>
>> >> Speaking of correctness, can I just get a confirmation if this is an
>> >> acceptable way to erase the use of a function?
>> >> #ifndef _WIN32
>> >> #define SetConsoleTextAttribute(x,y) ;
>> >> #endif
>> >
>> > its not pretty, so if i find an alternative ill likely will ask you to
>> > change it to that :)
>> > if i dont (and i dont see one currently) then its fine
>> >
>>
>> Okay. ?I've attached the patch
>

>> ? ? ?if(use_ansi_color<0){
>> -#if HAVE_ISATTY && !defined(_WIN32)
>> - ? ? ? ?use_ansi_color= getenv("TERM") && !getenv("NO_COLOR") && isatty(2);
>> -#else
>> - ? ? ? ?use_ansi_color= 0;
>> -#endif
>> + ? ? ? ?use_ansi_color= HAVE_ISATTY && isatty(2) && getenv("TERM") && !getenv("NO_COLOR");
>
> as mans pointed out this doesnt work due to configure adding
> -Werror=missing-prototypes
> thus we have to use the less readable variant
>

Should it be defined as 0 at the top of the file, or should I leave
your original #if inside the posix case?



More information about the ffmpeg-devel mailing list