[FFmpeg-devel] [PATCH] add colours to warnings and errors
Michael Niedermayer
michaelni
Mon Apr 26 11:28:55 CEST 2010
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
> log.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 65 insertions(+), 5 deletions(-)
> 82232b11a97efd8edc5b01093302b26648fd660a 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,80 @@
> #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. */
> +
> +#ifndef _WIN32
> +#define SetConsoleTextAttribute(x,y) ;
> +#endif
> +
> static int use_ansi_color=-1;
> +static int use_win_color=-1;
>
> #undef fprintf
> static void colored_fputs(int color, const char *str){
> +#ifdef _WIN32
> + static int16_t attr, attr_orig;
> + static HANDLE console;
> + static const int16_t win_color_conv[] = {
> + 0,
> + FOREGROUND_RED,
> + FOREGROUND_GREEN,
> + FOREGROUND_RED |FOREGROUND_GREEN,
> + FOREGROUND_BLUE,
> + FOREGROUND_RED |FOREGROUND_BLUE,
> + FOREGROUND_GREEN|FOREGROUND_BLUE,
> + FOREGROUND_RED |FOREGROUND_GREEN|FOREGROUND_BLUE
> + };
> +
> + if (use_ansi_color<0) {
> + CONSOLE_SCREEN_BUFFER_INFO con_info;
> + char *term = getenv("TERM") ? getenv("TERM") : "";
> +
> + use_ansi_color = isatty(2) && !getenv("NO_COLOR")
> + && ( !strncmp( term, "msys", 4 )
> + || !strncmp( term, "xterm", 5 )
> + || !strncmp( term, "rxvt", 4 )
> + //|| !strncmp( getenv("TERM"), "cygwin", 6 )
> +/* The CYGWIN environment variable makes this hard for native executables.
> +notty -- programs are directly connected to cmd so the Windows API works
> +tty -- programs are not directly connected so ANSI color codes work
> +The default is notty so leaving "cygwin" excluded doesn't cause problems. */
> + );
> +
> + console = GetStdHandle( STD_ERROR_HANDLE );
> + if (console != INVALID_HANDLE_VALUE && !use_ansi_color) {
> + GetConsoleScreenBufferInfo( console, &con_info );
> + attr_orig = con_info.wAttributes;
> +
> + attr = attr_orig&0xF0;
> + attr |= (attr_orig&BACKGROUND_INTENSITY) ? 0 : FOREGROUND_INTENSITY;
> +
> + use_win_color = 1;
> + } else
> + use_win_color = 0;
> + }
> +#else
> 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
> + use_win_color = 0;
> }
>
> +#endif
> if(use_ansi_color){
> fprintf(stderr, "\033[%d;3%dm", color>>4, color&15);
> + } else if (use_win_color) {
> + SetConsoleTextAttribute( console,
> + (color&15)==9 ? attr_orig : attr|win_color_conv[color&15]
> + );
> }
> fputs(str, stderr);
> if(use_ansi_color){
> fprintf(stderr, "\033[0m");
> + } else if (use_win_color) {
> + SetConsoleTextAttribute( console, attr_orig );
> }
if(use_win_color) {
SetConsoleTextAttribute()
fputs(str, stderr);
SetConsoleTextAttribute( console, attr_orig );
return;
}
#endif
this avoid the ifdef for SetConsoleTextAttribute()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100426/c0cfd0d3/attachment.pgp>
More information about the ffmpeg-devel
mailing list