[FFmpeg-devel] [PATCH v3 1/5] libavutil/wchar_filename.h: Add wchartoansi and utf8toansi

[FFmpeg-devel] [PATCH v3 1/5] libavutil/wchar_filename.h: Add wchartoansi and utf8toansi

[nihil-admirari]

These functions are going to be used in libavformat/avisynth.c
and fftools/cmdutils.c when replacing MAX_PATH-sized buffers
with dynamically sized ones.
---
 libavutil/wchar_filename.h | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index 90f0824..32260a4 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -40,6 +40,43 @@ 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 wchartoansi(const wchar_t *filename_w, char **filename)
+{
+    const int num_chars = WideCharToMultiByte(CP_THREAD_ACP, 0, filename_w, -1,
+                                              NULL, 0, NULL, NULL);
+    if (num_chars <= 0) {
+        *filename = NULL;
+        return 0;
+    }
+    *filename = (char *)av_calloc(num_chars, sizeof(char));
+    if (!*filename) {
+        errno = ENOMEM;
+        return -1;
+    }
+    WideCharToMultiByte(CP_THREAD_ACP, 0, filename_w, -1,
+                        *filename, num_chars, NULL, NULL);
+    return 0;
+}
+
+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 v3 2/5] libavformat/avisynth.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[nihil-admirari]

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

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 8bc3986..219f307 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -36,6 +36,7 @@
 /* Platform-specific directives. */
 #ifdef _WIN32
   #include "compat/w32dlfcn.h"
+  #include "libavutil/wchar_filename.h"
   #undef EXTERN_C
   #define AVISYNTH_LIB "avisynth"
 #else
@@ -806,8 +807,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))
@@ -815,10 +815,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 v3 3/5] compat/w32dlfcn.h: Replace MAX_PATH-sized buffers with dynamically sized ones

[nihil-admirari]

Also replaces a call to LoadLibraryExA with LoadLibraryExW
since ANSI functions do not support long paths.
---
 compat/w32dlfcn.h | 74 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 61 insertions(+), 13 deletions(-)

diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
index 52a94ef..a8ac780 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;
+    int path_size = 0, path_len = 0;
+
+    do {
+        path_size = path_size ? 1.5 * path_size : MAX_PATH;
+        wchar_t *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,28 +58,50 @@
  */
 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;
+            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;
+                // The buffer might have been not enough for system directory
+                // in the first place.
+                pathlen = GetSystemDirectoryW(path, pathsize);
+                if (!pathlen)
+                    goto exit;
+            }
             path[pathlen] = '\\';
             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.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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[nihil-admirari]

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

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 4b50e15..ea78897 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -62,6 +62,7 @@
 #endif
 #ifdef _WIN32
 #include <windows.h>
+#include "compat/w32dlfcn.h"
 #endif
 
 static int init_report(const char *env);
@@ -2065,6 +2066,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, };
@@ -2074,19 +2078,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;
+                const int datadir_len = ls - datadir;
+                const int 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
@@ -2106,6 +2122,9 @@ FILE *get_preset_file(char *filename, size_t filename_size,
         }
     }
 
+#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 v3 5/5] fftools: Enable long path support on Windows (fixes #8885)

[nihil-admirari]

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

diff --git a/fftools/Makefile b/fftools/Makefile
index da42078..53438b6 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -11,6 +11,11 @@ ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
 
 OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
 
+# Windows resource files
+OBJS-ffmpeg-$(HAVE_GNU_WINDRES) += fftools/long_paths_utf8.o
+OBJS-ffplay-$(HAVE_GNU_WINDRES) += fftools/long_paths_utf8.o
+OBJS-ffprobe-$(HAVE_GNU_WINDRES) += fftools/long_paths_utf8.o
+
 define DOFFTOOL
 OBJS-$(1) += fftools/cmdutils.o fftools/$(1).o $(OBJS-$(1)-yes)
 $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1))
diff --git a/fftools/long_paths_utf8.manifest b/fftools/long_paths_utf8.manifest
new file mode 100644
index 0000000..d1ac1e4
--- /dev/null
+++ b/fftools/long_paths_utf8.manifest
@@ -0,0 +1,12 @@
+<?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"
+                     xmlns:ws2019="http://schemas.microsoft.com/SMI/2019/WindowsSettings">
+      <ws2016:longPathAware>true</ws2016:longPathAware>
+      <ws2019:activeCodePage>UTF-8</ws2019:activeCodePage>
+    </windowsSettings>
+  </application>
+</assembly>
diff --git a/fftools/long_paths_utf8.rc b/fftools/long_paths_utf8.rc
new file mode 100644
index 0000000..f33de76
--- /dev/null
+++ b/fftools/long_paths_utf8.rc
@@ -0,0 +1,3 @@
+#include <windows.h>
+
+CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST "long_paths_utf8.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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[Martin Storsjö]

On Wed, 16 Feb 2022, nihil-admirari wrote:

> ---
> fftools/cmdutils.c | 31 +++++++++++++++++++++++++------
> 1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 4b50e15..ea78897 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -62,6 +62,7 @@
> #endif
> #ifdef _WIN32
> #include <windows.h>
> +#include "compat/w32dlfcn.h"
> #endif
>
> static int init_report(const char *env);
> @@ -2065,6 +2066,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, };
> @@ -2074,19 +2078,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;

Why would you use ansi here? Aren't all internal char based paths supposed 
to be UTF-8, unless passing them to an external API that expects e.g. ACP 
paths?

// 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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[nil-admirari]

Previously there was GetModuleFileNameA. wchartoansi is used to match old behaviour. I can replace it with wchartoutf8 if you wish.

> - if (GetModuleFileNameA(GetModuleHandleA(NULL), datadir, sizeof(datadir) - 1))
> + if (wchartoansi(datadir_w, &datadir))
> + datadir = NULL;

From: Martin Storsjö <martin@martin.st>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones
Date: 16/02/2022 17:08:12 Europe/Moscow

Why would you use ansi here? Aren't all internal char based paths supposed 
to be UTF-8, unless passing them to an external API that expects e.g. ACP 
paths?

// 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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[Martin Storsjö]

> On Feb 16, 2022, at 18:32, nil-admirari@mailo.com wrote:
> 
> Previously there was GetModuleFileNameA. wchartoansi is used to match old behaviour. I can replace it with wchartoutf8 if you wish.

Oh, right. Well yes - if the path later is going to end up in a codepath that expects it to be UTF8 (please do check!), then we should go that way instead, then the existing code had a latent bug wrt that.

(Also, don’t top post!)

// 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".

[FFmpeg-devel] [PATCH v4] fftools: Enable long path support on Windows (fixes #8885)

[nihil-admirari]

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

diff --git a/fftools/Makefile b/fftools/Makefile
index da42078..37bdf38 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -11,6 +11,11 @@ ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
 
 OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
 
+# Windows resource files
+OBJS-ffmpeg-$(HAVE_GNU_WINDRES) += fftools/long_paths.o
+OBJS-ffplay-$(HAVE_GNU_WINDRES) += fftools/long_paths.o
+OBJS-ffprobe-$(HAVE_GNU_WINDRES) += fftools/long_paths.o
+
 define DOFFTOOL
 OBJS-$(1) += fftools/cmdutils.o fftools/$(1).o $(OBJS-$(1)-yes)
 $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1))
diff --git a/fftools/long_paths.manifest b/fftools/long_paths.manifest
new file mode 100644
index 0000000..30b7d8f
--- /dev/null
+++ b/fftools/long_paths.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/long_paths.rc b/fftools/long_paths.rc
new file mode 100644
index 0000000..689f8b2
--- /dev/null
+++ b/fftools/long_paths.rc
@@ -0,0 +1,3 @@
+#include <windows.h>
+
+CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST "long_paths.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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[nil-admirari]

> if the path later is going to end up in a codepath that expects it to be UTF8 (please do check!), then we should go that way instead

I checked. datadir ends up in (cmdutils.c:2104)

    base[2] = datadir;

and base[*] are later used in (cmdutils.c:2112 or 2116)

    snprintf(filename, filename_size, "%s%s/%s.ffpreset", base[i],
             i != 1 ? "" : "/.ffmpeg", preset_name);
    f = fopen(filename, "r");

On Windows fopen expects ANSI encoded strings, so we cannot change the encoding to UTF-8 without rewriting the rest of the function.



I've also overlooked something from your previous comment.

> unless passing them to an external API

Previous version of the manifest (https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293109.html) included

    <ws2019:activeCodePage>UTF-8</ws2019:activeCodePage>

which makes CP_ACP the same as CP_UTF8. Unfortunately external APIs must also opt in for such a change, otherwise they won't be able to decode the strings FFmpeg sent them. A new version of the manifest without code page changes is at https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293168.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 v3 5/5] fftools: Enable long path support on Windows (fixes #8885)

[Marton Balint]

On Wed, 16 Feb 2022, nihil-admirari wrote:

> ---
> fftools/Makefile                 |  5 +++++
> fftools/long_paths_utf8.manifest | 12 ++++++++++++
> fftools/long_paths_utf8.rc       |  3 +++
> 3 files changed, 20 insertions(+)
> create mode 100644 fftools/long_paths_utf8.manifest
> create mode 100644 fftools/long_paths_utf8.rc
>
> diff --git a/fftools/Makefile b/fftools/Makefile
> index da42078..53438b6 100644
> --- a/fftools/Makefile
> +++ b/fftools/Makefile
> @@ -11,6 +11,11 @@ ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
>
> OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
>
> +# Windows resource files
> +OBJS-ffmpeg-$(HAVE_GNU_WINDRES) += fftools/long_paths_utf8.o
> +OBJS-ffplay-$(HAVE_GNU_WINDRES) += fftools/long_paths_utf8.o
> +OBJS-ffprobe-$(HAVE_GNU_WINDRES) += fftools/long_paths_utf8.o
> +
> define DOFFTOOL
> OBJS-$(1) += fftools/cmdutils.o fftools/$(1).o $(OBJS-$(1)-yes)
> $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1))
> diff --git a/fftools/long_paths_utf8.manifest b/fftools/long_paths_utf8.manifest
> new file mode 100644
> index 0000000..d1ac1e4
> --- /dev/null
> +++ b/fftools/long_paths_utf8.manifest
> @@ -0,0 +1,12 @@
> +<?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"
> +                     xmlns:ws2019="http://schemas.microsoft.com/SMI/2019/WindowsSettings">
> +      <ws2016:longPathAware>true</ws2016:longPathAware>
> +      <ws2019:activeCodePage>UTF-8</ws2019:activeCodePage>

Generally UTF-8 codepage should not be needed, because unicode windows 
functions should be used everywhere, right?

I'd perfer if you enable UTF8 codepage in a separate patch. And use simple 
fftools/fftools.manifest and fftools/manifest.rc as filenames, because 
later other things might be put there, not only utf8/longpath support.

Thanks,
Marton

> +    </windowsSettings>
> +  </application>
> +</assembly>
> diff --git a/fftools/long_paths_utf8.rc b/fftools/long_paths_utf8.rc
> new file mode 100644
> index 0000000..f33de76
> --- /dev/null
> +++ b/fftools/long_paths_utf8.rc
> @@ -0,0 +1,3 @@
> +#include <windows.h>
> +
> +CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST "long_paths_utf8.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".
>
_______________________________________________
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 v3 5/5] fftools: Enable long path support on Windows (fixes #8885)

[nil-admirari]

> Generally UTF-8 codepage should not be needed, because unicode windows 
functions should be used everywhere, right?

No. FFmpeg does not seem to use WinAPI ANSI functions explicitly, but it still uses ordinary stdlib functions (fopen etc.) instead of their wide equivalents. See https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293169.html.

> And use simple fftools/fftools.manifest and fftools/manifest.rc as filenames, because 
later other things might be put there, not only utf8/longpath support.

Done in https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293204.html.

> I'd perfer if you enable UTF8 codepage in a separate patch.

Done in https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293207.html.

Actually, UTF-8 codepage was already absent in the fourth version of this patch: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293168.html. Microsoft documentation (https://docs.microsoft.com/en-us/windows/win32/sbscs/application-manifests) says that manifests can be added to DLLs too, and does not say whether longPathAware and activeCodePage apply to the process or to individual DLLs loaded into it. Thus I wasn't sure whether ANSI-encoded strings passed to external APIs will be decoded correctly. Unfortunately, if it were that longPathAware is per DLL, manifests would need to be added to avformat.dll etc., so I decided to test how things actually work. It turned out that longPathAware and activeCodePage are applied per process, which means that all externally loaded libraries will have CP_ACP set to CP_UTF8, which means that there should be no problem in using activeCodePage in FFmpeg.

For some reason, fifth version of patches ended up being sent as separate messages. Only the second part (https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293206.html) is a reply to the first (https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293205.html). It's a strange email glitch. I swear I used git send-email to send these 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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[Martin Storsjö]

On Thu, 17 Feb 2022, nil-admirari@mailo.com wrote:

>> if the path later is going to end up in a codepath that expects it to be UTF8 (please do check!), then we should go that way instead
>
> I checked. datadir ends up in (cmdutils.c:2104)
>
>    base[2] = datadir;
>
> and base[*] are later used in (cmdutils.c:2112 or 2116)
>
>    snprintf(filename, filename_size, "%s%s/%s.ffpreset", base[i],
>             i != 1 ? "" : "/.ffmpeg", preset_name);
>    f = fopen(filename, "r");
>
> On Windows fopen expects ANSI encoded strings, so we cannot change the 
> encoding to UTF-8 without rewriting the rest of the function.

Ok, well maybe we should change that too, to use wchar paths (or 
utf8->whcar routines?). But maybe those are libavutil internals that 
cmdutils shouldn't use?

If we stick with this form, with ansi paths, it would be good to leave a 
comment at the call to wchartoansi(), to explain why that doesn't use 
UTF-8 like everything else.

// 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 v3 4/5] fftools/cmdutils.c: Replace MAX_PATH-sized buffers with dynamically sized ones

[nil-admirari]

> Ok, well maybe we should change that too, to use wchar paths (or utf8->whcar routines?).

Changed to wchartoutf8, and replaced fopen with av_fopen_utf8 throughout the file. See https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/293229.html.

> doesn't use UTF-8 like everything else.

 This is not the only place where plain fopen is used.

    grep -RP '\bfopen\(' | wc -l

gives 66 occurrences.



_______________________________________________
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".