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

[FFmpeg-devel] [PATCH v9 1/6] 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 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 90f08245..e22ffa8a 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(const unsigned int code_page, const wchar_t *filename_w,
+                            char **filename)
+{
+    const DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
+    const 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_THREAD_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.32.0



_______________________________________________
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 v9 2/6] 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 8ba2bdea..f7bea8c3 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.32.0



_______________________________________________
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 v9 3/6] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW

[Nil Admirari]

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

diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
index 52a94efa..0f41f50b 100644
--- a/compat/w32dlfcn.h
+++ b/compat/w32dlfcn.h
@@ -25,6 +25,30 @@
 #if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
 #include "libavutil/wchar_filename.h"
 #endif
+
+static inline wchar_t *get_module_filename(const 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 +58,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 +121,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.32.0



_______________________________________________
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 v9 4/6] fftools/cmdutils.c: Remove MAX_PATH limit and replace fopen with av_fopen_utf8

[Nil Admirari]

---
 fftools/cmdutils.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 5d7cdc3e..a66dbb22 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -37,6 +37,7 @@
 #include "libswresample/swresample.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
+#include "libavutil/avutil.h"
 #include "libavutil/channel_layout.h"
 #include "libavutil/display.h"
 #include "libavutil/mathematics.h"
@@ -50,6 +51,7 @@
 #include "opt_common.h"
 #ifdef _WIN32
 #include <windows.h>
+#include "compat/w32dlfcn.h"
 #endif
 
 AVDictionary *sws_dict;
@@ -812,28 +814,43 @@ 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, };
 
     if (is_path) {
         av_strlcpy(filename, preset_name, filename_size);
-        f = fopen(filename, "r");
+        f = av_fopen_utf8(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 (wchartoutf8(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;
+                const ptrdiff_t datadir_len = ls - datadir;
+                const 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
@@ -842,17 +859,20 @@ FILE *get_preset_file(char *filename, size_t filename_size,
                 continue;
             snprintf(filename, filename_size, "%s%s/%s.ffpreset", base[i],
                      i != 1 ? "" : "/.ffmpeg", preset_name);
-            f = fopen(filename, "r");
+            f = av_fopen_utf8(filename, "r");
             if (!f && codec_name) {
                 snprintf(filename, filename_size,
                          "%s%s/%s-%s.ffpreset",
                          base[i], i != 1 ? "" : "/.ffmpeg", codec_name,
                          preset_name);
-                f = fopen(filename, "r");
+                f = av_fopen_utf8(filename, "r");
             }
         }
     }
 
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+    av_free(datadir);
+#endif
     return f;
 }
 
-- 
2.32.0



_______________________________________________
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 v9 5/6] fftools: Enable long path support on Windows (fixes #8885)

[Nil Admirari]

---
 fftools/Makefile         |  5 +++++
 fftools/fftools.manifest | 10 ++++++++++
 fftools/manifest.rc      |  3 +++
 3 files changed, 18 insertions(+)
 create mode 100644 fftools/fftools.manifest
 create mode 100644 fftools/manifest.rc

diff --git a/fftools/Makefile b/fftools/Makefile
index 81ad6c4f..105ae5cc 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -15,6 +15,11 @@ OBJS-ffmpeg +=                  \
     fftools/ffmpeg_mux.o        \
     fftools/ffmpeg_opt.o        \
 
+# Windows resource files
+OBJS-ffmpeg-$(HAVE_GNU_WINDRES) += fftools/manifest.o
+OBJS-ffplay-$(HAVE_GNU_WINDRES) += fftools/manifest.o
+OBJS-ffprobe-$(HAVE_GNU_WINDRES) += fftools/manifest.o
+
 define DOFFTOOL
 OBJS-$(1) += fftools/cmdutils.o fftools/opt_common.o fftools/$(1).o $(OBJS-$(1)-yes)
 $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1))
diff --git a/fftools/fftools.manifest b/fftools/fftools.manifest
new file mode 100644
index 00000000..30b7d8fe
--- /dev/null
+++ b/fftools/fftools.manifest
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+  <assemblyIdentity type="win32" name="FFmpeg" version="1.0.0.0"/>
+  <application xmlns="urn:schemas-microsoft-com:asm.v3">
+    <windowsSettings xmlns:ws2016="http://schemas.microsoft.com/SMI/2016/WindowsSettings">
+      <ws2016:longPathAware>true</ws2016:longPathAware>
+    </windowsSettings>
+  </application>
+</assembly>
diff --git a/fftools/manifest.rc b/fftools/manifest.rc
new file mode 100644
index 00000000..e436fa73
--- /dev/null
+++ b/fftools/manifest.rc
@@ -0,0 +1,3 @@
+#include <windows.h>
+
+CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST "fftools.manifest"
-- 
2.32.0



_______________________________________________
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 v9 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Martin Storsjö]

On Fri, 15 Apr 2022, Nil Admirari wrote:

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

I looked through this patchset now, and I think it overall looks 
acceptable - with a couple minor things I'd like touched up and clarified.

However while reviewing this, I noticed a preexisting issue regarding the 
av_fopen_utf8 function. This patchset extends the use of that function 
into fftools, which isn't great given the issue... The use is fairly 
marginal (the preset files) so I guess it can be tolerated, as it's a 
preexisting orthogonal issue.

The other question is whether it's tolerable to use more non-installed 
headers (like libavutil/wchar_filename.h) in fftools. (I'd like to have 
this point confirmed with Anton before landing the patchset.)

A couple comments on the patch below:

> diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
> index 90f08245..e22ffa8a 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(const unsigned int code_page, const wchar_t *filename_w,
> +                            char **filename)
> +{
> +    const DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
> +    const int num_chars = WideCharToMultiByte(code_page, flags, filename_w, -1,
> +                                              NULL, 0, NULL, NULL);

We don't generally use 'const' like this in ffmpeg; we use 'const' where 
it makes a functional difference (i.e. on the type that pointers point 
at), but not for plain scalar values (neither parameters nor local 
variables).


> +    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_THREAD_ACP, filename_w, filename);
> +}

I think this actually would be more correct to use CP_ACP, not 
CP_THREAD_ACP. A bit of googling led me to 
https://github.com/ocaml/ocaml/issues/7854 which indicates that CP_ACP is 
used for ansi file functions, regardless of the current thread specific 
codepage. (But I see that this already is used in libavformat/avisynth.c, 
so it's in one sense a preexisting issue, but I think it'd be more correct 
to use CP_ACP here.)

// 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 v9 2/6] libavformat/avisynth.c: Remove MAX_PATH limit

[Martin Storsjö]

On Fri, 15 Apr 2022, Nil Admirari wrote:

> ---
> libavformat/avisynth.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 8ba2bdea..f7bea8c3 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

This looks ok to me, but as mentioned in the other patch, I think CP_ACP 
actually would be more correct than the current CP_THREAD_ACP 
(https://github.com/ocaml/ocaml/issues/7854), so it could be worthwhile to 
change that at the same time (either just as part of this refactoring, or 
as a totally separate patch for clarity).

// 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 v9 3/6] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW

[Martin Storsjö]

On Fri, 15 Apr 2022, Nil Admirari wrote:

> ---
> compat/w32dlfcn.h | 78 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 64 insertions(+), 14 deletions(-)
>
> diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
> index 52a94efa..0f41f50b 100644
> --- a/compat/w32dlfcn.h
> +++ b/compat/w32dlfcn.h
> @@ -25,6 +25,30 @@
> #if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
> #include "libavutil/wchar_filename.h"
> #endif
> +
> +static inline wchar_t *get_module_filename(const 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 +58,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 +121,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.32.0

This looks ok to me.

// 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 v9 4/6] fftools/cmdutils.c: Remove MAX_PATH limit and replace fopen with av_fopen_utf8

[Martin Storsjö]

On Fri, 15 Apr 2022, Nil Admirari wrote:

> ---
> fftools/cmdutils.c | 38 +++++++++++++++++++++++++++++---------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 5d7cdc3e..a66dbb22 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -37,6 +37,7 @@
> #include "libswresample/swresample.h"
> #include "libavutil/avassert.h"
> #include "libavutil/avstring.h"
> +#include "libavutil/avutil.h"
> #include "libavutil/channel_layout.h"
> #include "libavutil/display.h"
> #include "libavutil/mathematics.h"
> @@ -50,6 +51,7 @@
> #include "opt_common.h"
> #ifdef _WIN32
> #include <windows.h>
> +#include "compat/w32dlfcn.h"

This adds a dependency on nonpublic headers - which I think can be 
tolerated as it's only a build-time issue, and fftools are currently built 
as part of the rest of the ffmpeg build anyway.

> #endif
>
> AVDictionary *sws_dict;
> @@ -812,28 +814,43 @@ 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"),

Hmm, I guess neither of these are commonly set on Windows - otherwise this 
would suddenly change to interpret generic environment variables as UTF8.

>                             FFMPEG_DATADIR, };
>
>     if (is_path) {
>         av_strlcpy(filename, preset_name, filename_size);
> -        f = fopen(filename, "r");
> +        f = av_fopen_utf8(filename, "r");
>     } else {

As mentioned elsewhere, I realized that av_fopen_utf8 is problematic, but 
that's an orthogonal issue, and the issue is already preexisting, and it's 
used for a fairly marginal feature here, so I guess that can be tolerated 
too (and if the root cause is fixed, this gets taken care of at the same 
time too).

// 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 v9 5/6] fftools: Enable long path support on Windows (fixes #8885)

[Martin Storsjö]

On Fri, 15 Apr 2022, Nil Admirari wrote:

> ---
> fftools/Makefile         |  5 +++++
> fftools/fftools.manifest | 10 ++++++++++
> fftools/manifest.rc      |  3 +++
> 3 files changed, 18 insertions(+)
> create mode 100644 fftools/fftools.manifest
> create mode 100644 fftools/manifest.rc

I think the change here is fine, but the commit message absolutely needs 
to explain the full situation about what this does, how, etc.

As far as I've understood Windows long path support, it'd be something 
like this:

---8<---

Newer versions of Windows (Windows 10 1607 and newer) can support path 
names longer than MAX_PATH (260 characters). To take advantage of that, an 
application needs to opt in, by including a small manifest as a resource. 
Additionally, the path length limitation is only lifted for file APIs that 
pass paths as wchar_t. Therefore, the preceding patches have refactored a 
few remaining cases to internally open files with wchar_t path name 
functions.

On older versions of Windows, the newly added manifest has no effect.

---8<---

Does that sound like the correct explanation of the situation?

// 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 v9 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[nil-admirari]

> However while reviewing this, I noticed a preexisting issue regarding the 
> av_fopen_utf8 function. This patchset extends the use of that function 
> into fftools, which isn't great given the issue...

Reverted back to fopen().

> The other question is whether it's tolerable to use more non-installed 
> headers (like libavutil/wchar_filename.h) in fftools. (I'd like to have 
> this point confirmed with Anton before landing the patchset.)

Currently the header consist entirely of static inline functions. If it's not OK to use it here,
please suggest a better place for these functions.

> We don't generally use 'const' like this in ffmpeg; we use 'const' where 
> it makes a functional difference (i.e. on the type that pointers point 
> at), but not for plain scalar values (neither parameters nor local 
> variables).

Removed const from everywhere except pointer arguments.

> I think this actually would be more correct to use CP_ACP, not 
> CP_THREAD_ACP.

Changed to CP_ACP.

New version of the patch: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295569.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".

Re: [FFmpeg-devel] [PATCH v9 2/6] libavformat/avisynth.c: Remove MAX_PATH limit

[nil-admirari]

> This looks ok to me, but as mentioned in the other patch, I think CP_ACP 
> actually would be more correct than the current CP_THREAD_ACP 

utf8toansi was changed to use CP_ACP in https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295569.html, so avisynth.c now uses CP_ACP as well.



_______________________________________________
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 v9 4/6] fftools/cmdutils.c: Remove MAX_PATH limit and replace fopen with av_fopen_utf8

[nil-admirari]

> > +#include "compat/w32dlfcn.h"

> This adds a dependency on nonpublic headers - which I think can be 
> tolerated as it's only a build-time issue, and fftools are currently built 
> as part of the rest of the ffmpeg build anyway.

Currently the header consist entirely of static inline functions and macros.
If it's not OK to use it here, please suggest a better place for get_module_filename().

> > const char *base[3] = { getenv("FFMPEG_DATADIR"),
> > getenv("HOME"),
>
> Hmm, I guess neither of these are commonly set on Windows - otherwise this 
> would suddenly change to interpret generic environment variables as UTF8.
>
> ...
>
> As mentioned elsewhere, I realized that av_fopen_utf8 is problematic, but 
> that's an orthogonal issue, and the issue is already preexisting, and it's 
> used for a fairly marginal feature here, so I guess that can be tolerated 
> too (and if the root cause is fixed, this gets taken care of at the same 
> time too).

Reverted back to fopen().



_______________________________________________
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 v9 5/6] fftools: Enable long path support on Windows (fixes #8885)

[nil-admirari]

> Does that sound like the correct explanation of the situation?

Yes, thanks. I altered the suggested message a bit.

New version of the patch: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295571.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".