[FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Nil Admirari]

These functions are going to be used in libavformat/avisynth.c
and fftools/cmdutils.c to remove MAX_PATH limit.
---
 libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index 90f0824..c0e5d47 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -40,6 +40,57 @@ static inline int utf8towchar(const char *filename_utf8, wchar_t **filename_w)
     MultiByteToWideChar(CP_UTF8, 0, filename_utf8, -1, *filename_w, num_chars);
     return 0;
 }
+
+av_warn_unused_result
+static inline int wchartocp(unsigned int code_page, const wchar_t *filename_w,
+                            char **filename)
+{
+    DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
+    int num_chars = WideCharToMultiByte(code_page, flags, filename_w, -1,
+                                        NULL, 0, NULL, NULL);
+    if (num_chars <= 0) {
+        *filename = NULL;
+        return 0;
+    }
+    *filename = av_calloc(num_chars, sizeof(char));
+    if (!*filename) {
+        errno = ENOMEM;
+        return -1;
+    }
+    WideCharToMultiByte(code_page, flags, filename_w, -1,
+                        *filename, num_chars, NULL, NULL);
+    return 0;
+}
+
+av_warn_unused_result
+static inline int wchartoutf8(const wchar_t *filename_w, char **filename)
+{
+    return wchartocp(CP_UTF8, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int wchartoansi(const wchar_t *filename_w, char **filename)
+{
+    return wchartocp(CP_ACP, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int utf8toansi(const char *filename_utf8, char **filename)
+{
+    wchar_t *filename_w = NULL;
+    int ret = -1;
+    if (utf8towchar(filename_utf8, &filename_w))
+        return -1;
+
+    if (!filename_w) {
+        *filename = NULL;
+        return 0;
+    }
+
+    ret = wchartoansi(filename_w, filename);
+    av_free(filename_w);
+    return ret;
+}
 #endif
 
 #endif /* AVUTIL_WCHAR_FILENAME_H */
-- 
2.34.1



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v12 2/4] libavformat/avisynth.c: Remove MAX_PATH limit

[Nil Admirari]

---
 libavformat/avisynth.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 8ba2bde..f7bea8c 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -34,6 +34,7 @@
 /* Platform-specific directives. */
 #ifdef _WIN32
   #include "compat/w32dlfcn.h"
+  #include "libavutil/wchar_filename.h"
   #undef EXTERN_C
   #define AVISYNTH_LIB "avisynth"
 #else
@@ -810,8 +811,7 @@ static int avisynth_open_file(AVFormatContext *s)
     AVS_Value arg, val;
     int ret;
 #ifdef _WIN32
-    char filename_ansi[MAX_PATH * 4];
-    wchar_t filename_wc[MAX_PATH * 4];
+    char *filename_ansi = NULL;
 #endif
 
     if (ret = avisynth_context_create(s))
@@ -819,10 +819,12 @@ static int avisynth_open_file(AVFormatContext *s)
 
 #ifdef _WIN32
     /* Convert UTF-8 to ANSI code page */
-    MultiByteToWideChar(CP_UTF8, 0, s->url, -1, filename_wc, MAX_PATH * 4);
-    WideCharToMultiByte(CP_THREAD_ACP, 0, filename_wc, -1, filename_ansi,
-                        MAX_PATH * 4, NULL, NULL);
+    if (utf8toansi(s->url, &filename_ansi)) {
+        ret = AVERROR_UNKNOWN;
+        goto fail;
+    }
     arg = avs_new_value_string(filename_ansi);
+    av_free(filename_ansi);
 #else
     arg = avs_new_value_string(s->url);
 #endif
-- 
2.34.1



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v12 3/4] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW

[Nil Admirari]

---
 compat/w32dlfcn.h | 80 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
index 52a94ef..6b0dd7d 100644
--- a/compat/w32dlfcn.h
+++ b/compat/w32dlfcn.h
@@ -22,9 +22,31 @@
 #ifdef _WIN32
 #include <windows.h>
 #include "config.h"
-#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
 #include "libavutil/wchar_filename.h"
-#endif
+
+static inline wchar_t *get_module_filename(HMODULE module)
+{
+    wchar_t *path = NULL, *new_path = NULL;
+    DWORD path_size = 0, path_len = 0;
+
+    do {
+        path_size = path_size ? 2 * path_size : MAX_PATH;
+        new_path = av_realloc_array(path, path_size, sizeof *path);
+        if (!new_path) {
+            av_free(path);
+            return NULL;
+        }
+        path = new_path;
+        path_len = GetModuleFileNameW(module, path, path_size);
+    } while (path_len && path_size <= 32768 && path_size <= path_len);
+
+    if (!path_len) {
+        av_free(path);
+        return NULL;
+    }
+    return path;
+}
+
 /**
  * Safe function used to open dynamic libs. This attempts to improve program security
  * by removing the current directory from the dll search path. Only dll's found in the
@@ -34,29 +56,53 @@
  */
 static inline HMODULE win32_dlopen(const char *name)
 {
+    wchar_t *name_w = NULL;
+    if (utf8towchar(name, &name_w))
+        name_w = NULL;
 #if _WIN32_WINNT < 0x0602
     // Need to check if KB2533623 is available
     if (!GetProcAddress(GetModuleHandleW(L"kernel32.dll"), "SetDefaultDllDirectories")) {
         HMODULE module = NULL;
-        wchar_t *path = NULL, *name_w = NULL;
-        DWORD pathlen;
-        if (utf8towchar(name, &name_w))
+        wchar_t *path = NULL, *new_path = NULL;
+        DWORD pathlen, pathsize, namelen;
+        if (!name_w)
             goto exit;
-        path = (wchar_t *)av_calloc(MAX_PATH, sizeof(wchar_t));
+        namelen = wcslen(name_w);
         // Try local directory first
-        pathlen = GetModuleFileNameW(NULL, path, MAX_PATH);
-        pathlen = wcsrchr(path, '\\') - path;
-        if (pathlen == 0 || pathlen + wcslen(name_w) + 2 > MAX_PATH)
+        path = get_module_filename(NULL);
+        if (!path)
             goto exit;
-        path[pathlen] = '\\';
+        new_path = wcsrchr(path, '\\');
+        if (!new_path)
+            goto exit;
+        pathlen = new_path - path;
+        pathsize = pathlen + namelen + 2;
+        new_path = av_realloc_array(path, pathsize, sizeof *path);
+        if (!new_path)
+            goto exit;
+        path = new_path;
         wcscpy(path + pathlen + 1, name_w);
         module = LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
         if (module == NULL) {
             // Next try System32 directory
-            pathlen = GetSystemDirectoryW(path, MAX_PATH);
-            if (pathlen == 0 || pathlen + wcslen(name_w) + 2 > MAX_PATH)
+            pathlen = GetSystemDirectoryW(path, pathsize);
+            if (!pathlen)
                 goto exit;
-            path[pathlen] = '\\';
+            // Buffer is not enough in two cases:
+            // 1. system directory + \ + module name
+            // 2. system directory even without module name.
+            if (pathlen + namelen + 2 > pathsize) {
+                pathsize = pathlen + namelen + 2;
+                new_path = av_realloc_array(path, pathsize, sizeof *path);
+                if (!new_path)
+                    goto exit;
+                path = new_path;
+                // Query again to handle case #2.
+                pathlen = GetSystemDirectoryW(path, pathsize);
+                if (!pathlen)
+                    goto exit;
+            }
+            path[pathlen] = L'\\';
             wcscpy(path + pathlen + 1, name_w);
             module = LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
         }
@@ -73,15 +119,17 @@ exit:
 #   define LOAD_LIBRARY_SEARCH_SYSTEM32        0x00000800
 #endif
 #if HAVE_WINRT
-    wchar_t *name_w = NULL;
     int ret;
-    if (utf8towchar(name, &name_w))
+    if (!name_w)
         return NULL;
     ret = LoadPackagedLibrary(name_w, 0);
     av_free(name_w);
     return ret;
 #else
-    return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
+    /* filename may be be in CP_ACP */
+    if (!name_w)
+        return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
+    return LoadLibraryExW(name_w, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
 #endif
 }
 #define dlopen(name, flags) win32_dlopen(name)
-- 
2.34.1



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v12 4/4] fftools/cmdutils.c: Remove MAX_PATH limit

[Nil Admirari]

---
 fftools/cmdutils.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 5d7cdc3..d42bb04 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -50,6 +50,7 @@
 #include "opt_common.h"
 #ifdef _WIN32
 #include <windows.h>
+#include "compat/w32dlfcn.h"
 #endif
 
 AVDictionary *sws_dict;
@@ -812,6 +813,9 @@ FILE *get_preset_file(char *filename, size_t filename_size,
 {
     FILE *f = NULL;
     int i;
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+    char *datadir = NULL;
+#endif
     const char *base[3] = { getenv("FFMPEG_DATADIR"),
                             getenv("HOME"),
                             FFMPEG_DATADIR, };
@@ -821,19 +825,31 @@ FILE *get_preset_file(char *filename, size_t filename_size,
         f = fopen(filename, "r");
     } else {
 #if HAVE_GETMODULEHANDLE && defined(_WIN32)
-        char datadir[MAX_PATH], *ls;
+        wchar_t *datadir_w = get_module_filename(NULL);
         base[2] = NULL;
 
-        if (GetModuleFileNameA(GetModuleHandleA(NULL), datadir, sizeof(datadir) - 1))
+        if (wchartoansi(datadir_w, &datadir))
+            datadir = NULL;
+        av_free(datadir_w);
+
+        if (datadir)
         {
-            for (ls = datadir; ls < datadir + strlen(datadir); ls++)
+            char *ls;
+            for (ls = datadir; *ls; ls++)
                 if (*ls == '\\') *ls = '/';
 
             if (ls = strrchr(datadir, '/'))
             {
-                *ls = 0;
-                strncat(datadir, "/ffpresets",  sizeof(datadir) - 1 - strlen(datadir));
-                base[2] = datadir;
+                ptrdiff_t datadir_len = ls - datadir;
+                size_t desired_size = datadir_len + strlen("/ffpresets") + 1;
+                char *new_datadir = av_realloc_array(
+                    datadir, desired_size, sizeof *datadir);
+                if (new_datadir) {
+                    datadir = new_datadir;
+                    datadir[datadir_len] = 0;
+                    strncat(datadir, "/ffpresets",  desired_size - 1 - datadir_len);
+                    base[2] = datadir;
+                }
             }
         }
 #endif
@@ -853,6 +869,9 @@ FILE *get_preset_file(char *filename, size_t filename_size,
         }
     }
 
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+    av_free(datadir);
+#endif
     return f;
 }
 
-- 
2.34.1



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Martin Storsjö]

On Sun, 5 Jun 2022, Nil Admirari wrote:

> These functions are going to be used in libavformat/avisynth.c
> and fftools/cmdutils.c to remove MAX_PATH limit.
> ---
> libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)

This patchset looks good to me too, thanks! If there's no more comments on 
it, I'll push it later.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Thursday, June 9, 2022 12:10 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
> whcartoutf8, wchartoansi and utf8toansi
> 
> On Sun, 5 Jun 2022, Nil Admirari wrote:
> 
> > These functions are going to be used in libavformat/avisynth.c
> > and fftools/cmdutils.c to remove MAX_PATH limit.
> > ---
> > libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> 
> This patchset looks good to me too, thanks! If there's no more comments on
> it, I'll push it later.

I missed to take another look at it.

I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
whether it won't fail then, when a path contains non-ANSI chars.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Soft Works
> Sent: Thursday, June 9, 2022 6:47 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
> whcartoutf8, wchartoansi and utf8toansi
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> > Storsjö
> > Sent: Thursday, June 9, 2022 12:10 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
> > whcartoutf8, wchartoansi and utf8toansi
> >
> > On Sun, 5 Jun 2022, Nil Admirari wrote:
> >
> > > These functions are going to be used in libavformat/avisynth.c
> > > and fftools/cmdutils.c to remove MAX_PATH limit.
> > > ---
> > > libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 51 insertions(+)
> >
> > This patchset looks good to me too, thanks! If there's no more comments on
> > it, I'll push it later.
> 
> I missed to take another look at it.
> 
> I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
> whether it won't fail then, when a path contains non-ANSI chars.

@Nil - would you mind rebasing your patchset, it doesn't apply anymore.

Thanks,
sw
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Martin Storsjö]

On Thu, 9 Jun 2022, Soft Works wrote:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>> Storsjö
>> Sent: Thursday, June 9, 2022 12:10 PM
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
>> whcartoutf8, wchartoansi and utf8toansi
>>
>> On Sun, 5 Jun 2022, Nil Admirari wrote:
>>
>>> These functions are going to be used in libavformat/avisynth.c
>>> and fftools/cmdutils.c to remove MAX_PATH limit.
>>> ---
>>> libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 51 insertions(+)
>>
>> This patchset looks good to me too, thanks! If there's no more comments on
>> it, I'll push it later.
>
> I missed to take another look at it.
>
> I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
> whether it won't fail then, when a path contains non-ANSI chars.

Yes, that's a preexisting problem there. That patch gets rid of the fixed 
path lengths without touching the rest of the charset handling there.

There's paths from two sources; getenv() and from GetModuleFileName(). 
These are passed to plain fopen() (which uses ANSI filenames).

To make it entirely unicode compliant, we'd need to replace getenv() with 
a wrapper that uses _wgetenv() and converts that to utf8, then convert the 
GetModuleFileName() output to utf8 too, and switch the fopen() to 
fopen_utf8.

So the patch in itself is fine - it fixes one aspect, and doesn't make the 
other aspect worse.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[nil-admirari]

>> I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
>> whether it won't fail then, when a path contains non-ANSI chars.
>
> Yes, that's a preexisting problem there. That patch gets rid of the fixed 
> path lengths without touching the rest of the charset handling there.
>
> There's paths from two sources; getenv() and from GetModuleFileName(). 
> These are passed to plain fopen() (which uses ANSI filenames).
>
> To make it entirely unicode compliant, we'd need to replace getenv() with 
> a wrapper that uses _wgetenv() and converts that to utf8, then convert the 
> GetModuleFileName() output to utf8 too, and switch the fopen() to 
> fopen_utf8.

Actually utf8toansi() was created for AviSynth, which doesn't have a Unicode API.

getenv() and plain fopen() are used in cmdutils.c, which is the only file that
haven't transitioned to avpriv_fopen_utf8() yet, but utf8toansi() is not used in cmdutils.c.



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Thursday, June 9, 2022 9:09 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
> whcartoutf8, wchartoansi and utf8toansi
> 
> On Thu, 9 Jun 2022, Soft Works wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> >> Storsjö
> >> Sent: Thursday, June 9, 2022 12:10 PM
> >> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h:
> Add
> >> whcartoutf8, wchartoansi and utf8toansi
> >>
> >> On Sun, 5 Jun 2022, Nil Admirari wrote:
> >>
> >>> These functions are going to be used in libavformat/avisynth.c
> >>> and fftools/cmdutils.c to remove MAX_PATH limit.
> >>> ---
> >>> libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 51 insertions(+)
> >>
> >> This patchset looks good to me too, thanks! If there's no more comments on
> >> it, I'll push it later.
> >
> > I missed to take another look at it.
> >
> > I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
> > whether it won't fail then, when a path contains non-ANSI chars.
> 
> Yes, that's a preexisting problem there. That patch gets rid of the fixed
> path lengths without touching the rest of the charset handling there.
> 
> There's paths from two sources; getenv() and from GetModuleFileName().
> These are passed to plain fopen() (which uses ANSI filenames).

What I meant is the use of wchartoansi() in patch 4/4.

The current code calls GetModuleFileNameA()
the new code calls GetModuleFileNameW() and then converts the output
file name with wchartoansi().

Can we be sure that this is always the same? 
Especially when the path contains non-ANSI chars?





That's something quite different. GetModuleFileNameA() will return a 
valid and usable path (but it could be an 8.3 path or have some 
replacement chars)

Same applies to GetModuleFileNameW()
when using the wchar output - but not when using a string conversion to
an Ansi string.

> > So the patch in itself is fine - it fixes one aspect, and doesn't make the
> other aspect worse.

Wouldn't it make it worse in the case above?


Thanks,
sw


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Martin Storsjö]

On Thu, 9 Jun 2022, Soft Works wrote:

>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>> Storsjö
>> Sent: Thursday, June 9, 2022 9:09 PM
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
>> whcartoutf8, wchartoansi and utf8toansi
>>
>> On Thu, 9 Jun 2022, Soft Works wrote:
>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>>>> Storsjö
>>>> Sent: Thursday, June 9, 2022 12:10 PM
>>>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>>>> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h:
>> Add
>>>> whcartoutf8, wchartoansi and utf8toansi
>>>>
>>>> On Sun, 5 Jun 2022, Nil Admirari wrote:
>>>>
>>>>> These functions are going to be used in libavformat/avisynth.c
>>>>> and fftools/cmdutils.c to remove MAX_PATH limit.
>>>>> ---
>>>>> libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 51 insertions(+)
>>>>
>>>> This patchset looks good to me too, thanks! If there's no more comments on
>>>> it, I'll push it later.
>>>
>>> I missed to take another look at it.
>>>
>>> I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
>>> whether it won't fail then, when a path contains non-ANSI chars.
>>
>> Yes, that's a preexisting problem there. That patch gets rid of the fixed
>> path lengths without touching the rest of the charset handling there.
>>
>> There's paths from two sources; getenv() and from GetModuleFileName().
>> These are passed to plain fopen() (which uses ANSI filenames).
>
> What I meant is the use of wchartoansi() in patch 4/4.
>
> The current code calls GetModuleFileNameA()
> the new code calls GetModuleFileNameW() and then converts the output
> file name with wchartoansi().
>
> Can we be sure that this is always the same?
> Especially when the path contains non-ANSI chars?
>
> That's something quite different. GetModuleFileNameA() will return a
> valid and usable path (but it could be an 8.3 path or have some
> replacement chars)

Right, I wasn't aware that the -A version would return a 
guaranteed-ansi-compatible version of the filename. If that's really the 
case, this patch would indeed be a minor step backwards.

// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[nil-admirari]

> Right, I wasn't aware that the -A version would return a 
> guaranteed-ansi-compatible version of the filename. If that's really the 
> case, this patch would indeed be a minor step backwards.

Two options are available:
1. fopen() is replaced with avpriv_fopen_utf8(), getenv() is made Unicode-aware
    on Windows, and wide version of get_module_filename() is used as it is now.
2. A wrapper around GetModuleFilenameA() created specifically for this case,
    only to be replaced later by option one, when avpriv_fopen_utf8() gets merged.

If option one is chosen, the patch will have to wait for avpriv_fopen_utf8() patches.



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Martin Storsjö]

On Thu, 9 Jun 2022, nil-admirari@mailo.com wrote:

>> Right, I wasn't aware that the -A version would return a
>> guaranteed-ansi-compatible version of the filename. If that's really the
>> case, this patch would indeed be a minor step backwards.
>
> Two options are available:
> 1. fopen() is replaced with avpriv_fopen_utf8(), getenv() is made Unicode-aware
>    on Windows, and wide version of get_module_filename() is used as it is now.
> 2. A wrapper around GetModuleFilenameA() created specifically for this case,
>    only to be replaced later by option one, when avpriv_fopen_utf8() gets merged.
>
> If option one is chosen, the patch will have to wait for avpriv_fopen_utf8() patches.

For that, the first option sounds better - that sounds to me more like a 
direction forward, not backwards.

The patches for *_fopen_utf8 have already been merged - within the 
libraries, you can use avpriv_fopen_utf8, and fftools has got a separate 
"fopen_utf8" function it can use.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[nil-admirari]

>> ...
>> 1. fopen() is replaced with avpriv_fopen_utf8(), getenv() is made Unicode-aware
>> on Windows, and wide version of get_module_filename() is used as it is now.
> ...
> For that, the first option sounds better - that sounds to me more like a 
> direction forward, not backwards.

Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297491.html.
getenv_utf8 is a static inline in its own header in libavutil.

log.c, bktr.c, fbdev_common.c, oss.c and af_ladspa.c still use plain getenv(),
please check commit message for details.

All other uses of getenv() have been replaced with getenv_utf8():
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297495.html
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297494.html
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297493.html



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".