[PATCH]Win32: correct the encoding of filename in console message
1) In Win32, the filename is converted to UTF-8 encoding and stored in memory, but it's not converted back correctly when writing console messages. This patch converts the filename to suitable encoding before output to console. I compiled the code under Ubuntu with mingw, and test under Windows 10 with charset CP936, iconv enabled/disabled. 2) When building Linux x86_64 STATIC version mplayer, I got a link error at the last step and fixed it by adding '-lXau' to config.mak by hand. But I am not sure if this is a platform dependent issue or not (Ubuntu 15.04), anyone can help check and fix it ? I'm not very familiar with config system -.- build commands: ./configure --enable-static make -j6 Error messages: /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../x86_64-linux-gnu/libaudio.a(ConnSvr.o): In function `_AuConnectServer': (.text+0x934): undefined reference to `XauDisposeAuth' /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../x86_64-linux-gnu/libaudio.a(ConnSvr.o): In function `_AuConnectServer': (.text+0x9dc): undefined reference to `XauGetBestAuthByAddr' Here is the patch fixes filename encoding issue: Index: mp_msg.c =================================================================== diff --git a/trunk/mp_msg.c b/trunk/mp_msg.c --- a/trunk/mp_msg.c (revision 37444) +++ b/trunk/mp_msg.c (working copy) @@ -30,6 +30,11 @@ #include <errno.h> #endif +#if defined(__MINGW32__) || defined(__CYGWIN__) +#include <windows.h> +#include "mpcommon.h" +#endif + #include "mp_msg.h" /* maximum message length of mp_msg */ @@ -46,21 +51,19 @@ static iconv_t msgiconv; #endif -const char* filename_recode(const char* filename) +#if defined(CONFIG_ICONV) && defined(MSG_CHARSET) +static const char *convert_charset(const char* filename, const char *to, const char *from) { -#if !defined(CONFIG_ICONV) || !defined(MSG_CHARSET) - return filename; -#else static iconv_t inv_msgiconv = (iconv_t)(-1); static char recoded_filename[MSGSIZE_MAX]; size_t filename_len, max_path; char* precoded; - if (!mp_msg_charset || - !strcasecmp(mp_msg_charset, MSG_CHARSET) || - !strcasecmp(mp_msg_charset, "noconv")) + if (!from || !to || + !strcasecmp(from, to) || + !strcasecmp(from, "noconv")) return filename; if (inv_msgiconv == (iconv_t)(-1)) { - inv_msgiconv = iconv_open(MSG_CHARSET, mp_msg_charset); + inv_msgiconv = iconv_open(to, from); if (inv_msgiconv == (iconv_t)(-1)) return filename; } @@ -67,7 +70,7 @@ filename_len = strlen(filename); max_path = MSGSIZE_MAX - 4; precoded = recoded_filename; - if (iconv(inv_msgiconv, &filename, &filename_len, + if (iconv(inv_msgiconv, (char **)&filename, &filename_len, &precoded, &max_path) == (size_t)(-1) && errno == E2BIG) { precoded[0] = precoded[1] = precoded[2] = '.'; precoded += 3; @@ -74,9 +77,70 @@ } *precoded = '\0'; return recoded_filename; +} + #endif + +#if defined(__MINGW32__) || defined(__CYGWIN__) + +// convert filename to system default charset +static const char * win_default_charset(const char* filename) +{ + int cnt; + wchar_t fname_w[MSGSIZE_MAX]; + static char recoded_filename[MSGSIZE_MAX]; + typedef int WINAPI (*PWC2MB)(UINT, DWORD, LPCWSTR, int, LPSTR, int, LPCSTR, LPBOOL); + typedef int WINAPI (*PMB2WC)(UINT, DWORD, LPCSTR, int, LPWSTR, int); + PWC2MB wc2mb; + PMB2WC mb2wc; + + HMODULE WINAPI kernel32 = GetModuleHandle("Kernel32.dll"); + if (!kernel32) goto fallback; + + mb2wc = (PMB2WC)GetProcAddress(kernel32, "MultiByteToWideChar"); + wc2mb = (PWC2MB)GetProcAddress(kernel32, "WideCharToMultiByte"); + if (!mb2wc || !wc2mb) goto fallback; + cnt = mb2wc(CP_UTF8, MB_ERR_INVALID_CHARS, filename, -1, fname_w, sizeof(fname_w) / sizeof(*fname_w)); + if (cnt <= 0) goto fallback; + + cnt = wc2mb(CP_ACP, 0, fname_w, -1, recoded_filename, sizeof(recoded_filename) / sizeof(char), NULL, NULL); + if (cnt <= 0) goto fallback; + return recoded_filename; + +fallback: + return filename; } +#endif +// if have iconv and msg_charset, ensure the filename is 'msg_charset' encoding +// if no iconv nor msg_charset, keep it as system default encoding +const char* filename_recode(const char* filename) +{ +#if !defined(CONFIG_ICONV) || !defined(MSG_CHARSET) + +#if defined(__MINGW32__) || defined(__CYGWIN__) + if (win32_use_utf8) + return win_default_charset(filename); +#endif + return filename; + +#else // CONFIG_ICONV && MSG_CHARSET + +#if defined(__MINGW32__) || defined(__CYGWIN__) + if (win32_use_utf8) { + if (!mp_msg_charset || !strcasecmp(mp_msg_charset, "noconv")) { + return win_default_charset(filename); + } + if (!strcasecmp(MSG_CHARSET, "UTF-8") || !strcasecmp(MSG_CHARSET, "UTF8")) + return filename; + return convert_charset(filename, MSG_CHARSET, "UTF-8"); + } +#endif // fallback if not using utf8 filename for win32 + + return convert_charset(filename, MSG_CHARSET, mp_msg_charset); +#endif +} + void mp_msg_init(void){ int i; char *env = getenv("MPLAYER_VERBOSE"); @@ -198,7 +262,8 @@ tmp[MSGSIZE_MAX-1] = 0; #if defined(CONFIG_ICONV) && defined(MSG_CHARSET) - if (mp_msg_charset && strcasecmp(mp_msg_charset, "noconv")) { + if (mp_msg_charset && strcasecmp(mp_msg_charset, "noconv") + && strcasecmp(mp_msg_charset, MSG_CHARSET)) { char tmp2[MSGSIZE_MAX]; size_t inlen = strlen(tmp), outlen = MSGSIZE_MAX; char *in = tmp, *out = tmp2; Index: mpcommon.c =================================================================== diff --git a/trunk/mpcommon.c b/trunk/mpcommon.c --- a/trunk/mpcommon.c (revision 37444) +++ b/trunk/mpcommon.c (working copy) @@ -466,7 +466,10 @@ {NULL, NULL, 0, 0, 0, 0, NULL} }; -#ifdef __MINGW32__ +#if defined(__MINGW32__) || defined(__CYGWIN__) + +int win32_use_utf8 = 0; + static int get_win32_cmdline(int *argc_ptr, char **argv_ptr[]) { int i; @@ -524,6 +527,9 @@ *argc_ptr = argc_n; *argv_ptr = argv_n; LocalFree(argv_w); + + win32_use_utf8 = 1; + return 0; err_out: Index: mpcommon.h =================================================================== diff --git a/trunk/mpcommon.h b/trunk/mpcommon.h --- a/trunk/mpcommon.h (revision 37444) +++ b/trunk/mpcommon.h (working copy) @@ -70,6 +70,10 @@ extern m_config_t *mconfig; extern const m_option_t noconfig_opts[]; +#if defined(__MINGW32__) || defined(__CYGWIN__) +extern int win32_use_utf8; +#endif + void print_version(const char* name); void init_vo_spudec(struct stream *stream, struct sh_video *sh_video, struct sh_sub *sh_sub); void update_subtitles(struct sh_video *sh_video, double refpts, demux_stream_t *d_dvdsub, int reset); Index: stream/stream_file.c =================================================================== diff --git a/trunk/stream/stream_file.c b/trunk/stream/stream_file.c --- a/trunk/stream/stream_file.c (revision 37444) +++ b/trunk/stream/stream_file.c (working copy) @@ -26,9 +26,10 @@ #if HAVE_SETMODE #include <io.h> #endif -#ifdef __MINGW32__ +#if defined(__MINGW32__) || defined(__CYGWIN__) #include <windows.h> #include <share.h> +#include "mpcommon.h" #endif #include "mp_msg.h" @@ -120,7 +121,7 @@ return STREAM_UNSUPPORTED; } -#ifdef __MINGW32__ +#if defined(__MINGW32__) || defined(__CYGWIN__) static int win32_open(const char *fname, int m, int omode) { int cnt; @@ -127,7 +128,11 @@ int fd = -1; wchar_t fname_w[MAX_PATH]; int WINAPI (*mb2wc)(UINT, DWORD, LPCSTR, int, LPWSTR, int) = NULL; - HMODULE kernel32 = GetModuleHandle("Kernel32.dll"); + HMODULE kernel32; + + if (!win32_use_utf8) goto fallback; + + kernel32 = GetModuleHandle("Kernel32.dll"); if (!kernel32) goto fallback; mb2wc = GetProcAddress(kernel32, "MultiByteToWideChar"); if (!mb2wc) goto fallback;
On Sun, 9 Aug 2015 02:20:57 +0800 (CST) 朱海 <zhuhai.mail@163.com> wrote: hello! sorry, your patch was stuck in the moderation queue.
- if (!mp_msg_charset || - !strcasecmp(mp_msg_charset, MSG_CHARSET) || - !strcasecmp(mp_msg_charset, "noconv")) + if (!from || !to || + !strcasecmp(from, to) || + !strcasecmp(from, "noconv")) return filename; if (inv_msgiconv == (iconv_t)(-1)) { - inv_msgiconv = iconv_open(MSG_CHARSET, mp_msg_charset); + inv_msgiconv = iconv_open(to, from);
changing functions to "from" and "to" looks weird to me.
- HMODULE kernel32 = GetModuleHandle("Kernel32.dll"); + HMODULE kernel32; + + if (!win32_use_utf8) goto fallback; + + kernel32 = GetModuleHandle("Kernel32.dll");
why the kernel32 change? -compn
On 2015/08/16 01:34, compn wrote:
On Sun, 9 Aug 2015 02:20:57 +0800 (CST) Zhu Hai <zhuhai.mail@163.com> wrote:
hello! sorry, your patch was stuck in the moderation queue.
- if (!mp_msg_charset || - !strcasecmp(mp_msg_charset, MSG_CHARSET) || - !strcasecmp(mp_msg_charset, "noconv")) + if (!from || !to || + !strcasecmp(from, to) || + !strcasecmp(from, "noconv")) return filename; if (inv_msgiconv == (iconv_t)(-1)) { - inv_msgiconv = iconv_open(MSG_CHARSET, mp_msg_charset); + inv_msgiconv = iconv_open(to, from);
changing functions to "from" and "to" looks weird to me. The parameter "from" and "to" comes from declaration of iconv_open:
iconv_t iconv_open(const char *tocode, const char *fromcode);
- HMODULE kernel32 = GetModuleHandle("Kernel32.dll"); + HMODULE kernel32; + + if (!win32_use_utf8) goto fallback; + + kernel32 = GetModuleHandle("Kernel32.dll"); why the kernel32 change?
just want to avoid unnecessary call of 'GetModuleHandle' and also avoid compile warning like : "... ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] " if put the 'if' statement before declaration of kernel32.
-compn
Hello, First only on the second part: On Sun, Aug 09, 2015 at 02:20:57AM +0800, 朱海 wrote:
2) When building Linux x86_64 STATIC version mplayer, I got a link error at the last step and fixed it by adding '-lXau' to config.mak by hand. But I am not sure if this is a platform dependent issue or not (Ubuntu 15.04), anyone can help check and fix it ? I'm not very familiar with config system -.- build commands: ./configure --enable-static make -j6 Error messages: /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../x86_64-linux-gnu/libaudio.a(ConnSvr.o): In function `_AuConnectServer': (.text+0x934): undefined reference to `XauDisposeAuth' /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../x86_64-linux-gnu/libaudio.a(ConnSvr.o): In function `_AuConnectServer': (.text+0x9dc): undefined reference to `XauGetBestAuthByAddr'
Look at the echocheck "NAS" part. Brute-force solution: add -lXau in the lines below. A better solution would be to change the code to properly use a test function so that linking is tested, compare e.g. the "statement_check esd.h 'esd_get_latency(0)'" a few lines above. That would have discovered the issue and disabled that feature instead of failing to link.
On Sun, Aug 09, 2015 at 02:20:57AM +0800, 朱海 wrote:
+ mb2wc = (PMB2WC)GetProcAddress(kernel32, "MultiByteToWideChar"); + wc2mb = (PWC2MB)GetProcAddress(kernel32, "WideCharToMultiByte");
FFmpeg already uses these unconditionally, so I think we can just remove the address lookup and use them directly.
+// if have iconv and msg_charset, ensure the filename is 'msg_charset' encoding +// if no iconv nor msg_charset, keep it as system default encoding +const char* filename_recode(const char* filename) +{ +#if !defined(CONFIG_ICONV) || !defined(MSG_CHARSET) + +#if defined(__MINGW32__) || defined(__CYGWIN__)
I think we use #ifdef WIN32 or so for this usually. But as mentioned later I am not sure it should apply to cygwin, I think it's not unlikely if using cygwin a console with UTF-8 support will be used, too.
+ if (win32_use_utf8) + return win_default_charset(filename); +#endif + return filename; + +#else // CONFIG_ICONV && MSG_CHARSET + +#if defined(__MINGW32__) || defined(__CYGWIN__) + if (win32_use_utf8) { + if (!mp_msg_charset || !strcasecmp(mp_msg_charset, "noconv")) { + return win_default_charset(filename); + }
Why is this case inside the CONFIG_ICONV ifdefs, and does it really make sense for the two cases to be different? Why should "noconv" result in re-encoding if iconv is not available? I think the intent is for it to ensure no charset conversion is ever done for messages.
+ if (!strcasecmp(MSG_CHARSET, "UTF-8") || !strcasecmp(MSG_CHARSET, "UTF8")) + return filename;
As far as I can tell this is already checked inside convert_charset. And as this is a define we should only ever allow one variant, i.e. MSG_CHARSET should never ever be UTF8, utf8 or utf-8 but always UTF-8.
+ return convert_charset(filename, MSG_CHARSET, "UTF-8"); + } +#endif // fallback if not using utf8 filename for win32 + + return convert_charset(filename, MSG_CHARSET, mp_msg_charset); +#endif +} + void mp_msg_init(void){ int i; char *env = getenv("MPLAYER_VERBOSE"); @@ -198,7 +262,8 @@ tmp[MSGSIZE_MAX-1] = 0;
#if defined(CONFIG_ICONV) && defined(MSG_CHARSET) - if (mp_msg_charset && strcasecmp(mp_msg_charset, "noconv")) { + if (mp_msg_charset && strcasecmp(mp_msg_charset, "noconv") + && strcasecmp(mp_msg_charset, MSG_CHARSET)) {
This seems unrelated and should be in a separate patch.
Index: stream/stream_file.c =================================================================== diff --git a/trunk/stream/stream_file.c b/trunk/stream/stream_file.c --- a/trunk/stream/stream_file.c (revision 37444) +++ b/trunk/stream/stream_file.c (working copy) @@ -26,9 +26,10 @@ #if HAVE_SETMODE #include <io.h> #endif -#ifdef __MINGW32__ +#if defined(__MINGW32__) || defined(__CYGWIN__) #include <windows.h> #include <share.h> +#include "mpcommon.h" #endif
#include "mp_msg.h" @@ -120,7 +121,7 @@ return STREAM_UNSUPPORTED; }
-#ifdef __MINGW32__ +#if defined(__MINGW32__) || defined(__CYGWIN__) static int win32_open(const char *fname, int m, int omode) { int cnt;
I suspect cygwin was left out on purpose, as it has its own hacks. Either way I really think this belongs at least in a separate patch.
@@ -127,7 +128,11 @@ int fd = -1; wchar_t fname_w[MAX_PATH]; int WINAPI (*mb2wc)(UINT, DWORD, LPCSTR, int, LPWSTR, int) = NULL; - HMODULE kernel32 = GetModuleHandle("Kernel32.dll"); + HMODULE kernel32; + + if (!win32_use_utf8) goto fallback; + + kernel32 = GetModuleHandle("Kernel32.dll"); if (!kernel32) goto fallback; mb2wc = GetProcAddress(kernel32, "MultiByteToWideChar"); if (!mb2wc) goto fallback;
I believe this breaks the intended behaviour. It should be possible to pass file names as UTF-8 to MPlayer and have them opened properly. Coupling it to whether the arguments were passed as wide args break that.
participants (4)
-
compn -
Reimar Döffinger -
zhuhai -
朱海