[FFmpeg-devel] [PATCH] avutil/log: in colored_fputs, convert string from utf8 to CP_ACP charset on Windows

[Timothy Gu]

On Feb 19, 2014 9:21 PM, "周泽华" <zhouzehua at gmail.com> wrote:
>
> Thanks for your reply.

First, do not top post. See http://www.idallen.com/topposting.html.

>
> Let me first show my situation. I have a mp3 file, in which the metadata
is in
> CP936 charset (CP_ACP is system dependent and is CP936 on my Windows). I
issue
> 'ffprobe path_to_file' to see the information of the file. In the
> 'prepare_app_arguments' function of cmdutils.c , the arguments of ffprobe
are
> converted to utf8, so 'path_to_file' is stored in utf8 in ffmpeg, while
the
> metadata is stored in CP936. Both 'path_to_file' and metadata are sent to
log
> via 'colored_fputs'. So we can only detect the charset of input string at
> runtime, not build time. I searched the internet and did not find any
better
> way to detect the charset.

You can put these comments into the commit log of you want.

>
> I think the commit is an improvement. Before the commit, 'path_to_file' is
> always garbled. If the metadata of a mp3 file is in utf8 charset, the log
> output is also garbled. After the commit, the log output is garbled when
> string in CP_ACP is converted 'from utf8 to CP_ACP successfully', which
> is wrong and happens rarely as I observe.
>

> About the unneeded code: I thought setting pointers to original value or
null
> is good. Anyway, a patch without these unneeded code is attached.

It is very important to keep a patch free from changes not related to the
main change.

However, you didn't answer Michael's request: please add a CONFIG_* or
HAVE_* macro and related detection code into ./configure. See his original
mail:

> > however exactly this is supposed to be detected, it should probably be
> > done in configure and set some appropriate define like maybe
> > HAVE_CP_ACP or something
> > maybe theres also a cleaner way to detect this i dont know

This has the advantage of not duplicating defined(_WIN32) &&
!defined(__MINGW32CE__) all the time, and support for other OS's which
needs this code can be easily added in configure script.

Timothy