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

[FFmpeg-devel] [PATCH v11 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 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 90f08245..c0e5d47e 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.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 v11 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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> Admirari
> Sent: Saturday, April 23, 2022 10:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v11 2/6] libavformat/avisynth.c: Remove
> MAX_PATH limit
> 
> ---
>  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
> 

LGTM. I had assumed it would depend on 6/6
_______________________________________________
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 v11 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..2284ac7a 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(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".

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

[nil-admirari]

#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
#include "libavutil/wchar_filename.h"
#endif

caused build error due to utf8towchar being undefined.

Made wchar_filename.h include unconditional.
Also removed manifest changes since it was decided to adopt \\?\ prefixes instead.

New version is at https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297198.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".

[FFmpeg-devel] [PATCH v11 4/6] 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 5d7cdc3e..d42bb04e 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.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 v11 5/6] fftools: Enable long path support on Windows (fixes #8885)

[Nil Admirari]

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.

Application must be prepared to handle filenames greater than MAX_PATH.
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 where:
1. filename length was restricted to MAX_PATH
2. files were opened using ANSI functions.

On older versions of Windows, the newly added manifest has no effect.
---
 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 v11 5/6] fftools: Enable long path support on Windows (fixes #8885)

[Tobias Rapp]

On 23/04/2022 22:56, Nil Admirari wrote:
> 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.
> 
> Application must be prepared to handle filenames greater than MAX_PATH.
> 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 where:
> 1. filename length was restricted to MAX_PATH
> 2. files were opened using ANSI functions.
> 
> On older versions of Windows, the newly added manifest has no effect.
> ---
>   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"/>

What is the effect of the version attribute here? Would it be meaningful 
to replace the static dummy value with something more realistic like 
"5.1.n" or similar?

Just asking as I'm not very familiar with manifest resources.

> +  <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"

Regards,
Tobias

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

[Hendrik Leppkes]

On Tue, Apr 26, 2022 at 9:08 AM Tobias Rapp <t.rapp@noa-archive.com> wrote:
>
> On 23/04/2022 22:56, Nil Admirari wrote:
> > 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.
> >
> > Application must be prepared to handle filenames greater than MAX_PATH.
> > 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 where:
> > 1. filename length was restricted to MAX_PATH
> > 2. files were opened using ANSI functions.
> >
> > On older versions of Windows, the newly added manifest has no effect.
> > ---
> >   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"/>
>
> What is the effect of the version attribute here? Would it be meaningful
> to replace the static dummy value with something more realistic like
> "5.1.n" or similar?
>
> Just asking as I'm not very familiar with manifest resources.
>

The assembly version does not have any important meaning, not for
assemblies used in this manner. It would only matter if you were to
reference other assemblies across files, which is not done for these
application settings - and even then the only real importance would be
to increment it when its changed in an incompatible manner, not
necessarily linked to the application version, which is stored in a
resource file instead of the assembly.

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

[Tobias Rapp]

On 26/04/2022 09:29, Hendrik Leppkes wrote:
> On Tue, Apr 26, 2022 at 9:08 AM Tobias Rapp <t.rapp@noa-archive.com> wrote:
>>
>> On 23/04/2022 22:56, Nil Admirari wrote:
>>> 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.
>>>
>>> Application must be prepared to handle filenames greater than MAX_PATH.
>>> 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 where:
>>> 1. filename length was restricted to MAX_PATH
>>> 2. files were opened using ANSI functions.
>>>
>>> On older versions of Windows, the newly added manifest has no effect.
>>> ---
>>>    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"/>
>>
>> What is the effect of the version attribute here? Would it be meaningful
>> to replace the static dummy value with something more realistic like
>> "5.1.n" or similar?
>>
>> Just asking as I'm not very familiar with manifest resources.
>>
> 
> The assembly version does not have any important meaning, not for
> assemblies used in this manner. It would only matter if you were to
> reference other assemblies across files, which is not done for these
> application settings - and even then the only real importance would be
> to increment it when its changed in an incompatible manner, not
> necessarily linked to the application version, which is stored in a
> resource file instead of the assembly.

Alright then, thanks for the clarification.

Regards, Tobias

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

[nil-admirari]

> What is the effect of the version attribute here? Would it be meaningful 
> to replace the static dummy value with something more realistic like 
> "5.1.n" or similar?

Version is a required attribute, see https://docs.microsoft.com/en-us/windows/win32/sbscs/application-manifests. It does not have any meaning, but some value has to be provided, and "1.0.0.0" is as good a value as any (version must be in major.minor.build.revision format: https://docs.microsoft.com/en-us/windows/win32/sbscs/assembly-versions).

"5.1.n.m" must be either manually updated or regenerated with something like "ffmpeg/tools/gen-rc". Both approaches are more laborious than just "1.0.0.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 v11 6/6] fftools: Set active code page to UTF-8 on Windows

[Nil Admirari]

Starting with Windows 1903, applications can set active code page to UTF-8
in the application manifest. It improves path name compatibility
with dependencies that use char* pathnames internally, but whose code page
has already been changed to UTF-8 with a manifest (e.g. AviSynth).

On older versions of Windows, changes in the manifest have no effect.
---
 fftools/fftools.manifest | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fftools/fftools.manifest b/fftools/fftools.manifest
index 30b7d8fe..d1ac1e4e 100644
--- a/fftools/fftools.manifest
+++ b/fftools/fftools.manifest
@@ -3,8 +3,10 @@
 <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">
+    <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>
-- 
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 v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> Admirari
> Sent: Saturday, April 23, 2022 10:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h:
> Add whcartoutf8, wchartoansi and utf8toansi
> 
> These functions are going to be used in libavformat/avisynth.c
> and fftools/cmdutils.c to remove MAX_PATH limit.
> ---

Hi,

thanks for submitting this patchset.

I'm afraid that I hadn't found time to look into this at an earlier stage,
so please apologize if one of my questions has been covered before.



1. Patch 3/6 - Replace LoadLibraryExA with LoadLibraryExW

What's the point in making changes to library loading? What does it fix?
Which dll cannot be loaded with the current implementation and what 
would be the location of the dll and the location of the exe in that case?

Could you please give an example of a situation that this is supposed 
to fix?



2. Patches 5/6 and 6/6 - Add Fusion Manifest

The manifest that you want to add includes two settings:

 - longPathAware
   effective on Windows 10, version 1607 or later

 - activeCodePage
   effective on Windows 10, version 1903 or later

Both of these manifest attributes are affecting the runtime behavior of
an application running on Windows - but only starting from a certain 
OS version. That means - effectively you would end up having an ffmpeg
executable with three different kinds of behavior:

1. Win < 1607
2. Win >= 1607 && < 1903
3. Win >= 1903

An ffmpeg executable, where you can't rely on the exposed behavior being
consistent and where you would need to check the operating system version 
before using to make sure that you are providing parameters in the "right"
way - I'm not sure whether that would make much sense.



3. All Patches x/6 - Remove MAX_PATH limit

The punch line sounds compelling. But how is the current situation and 
what exactly would be the benefit?

What's important to understand is that the basic Windows file APIs are
supporting long (> MAX_PATH length) for a long time already.
MS has just been a bit hesitant regarding the transition and wanted to
avoid breaking changes to the API. The outcome was that long paths are
only supported when using a lower-level path syntax, which is as simple 
as prepending "\\?\" to the actual path, no matter whether drive or UNC
network path.

As an example, the following command runs without issue on Windows 
with a normal current ffmpeg build:

.\ffmpeg.exe -i "\\?\U:\TestMedia\This is a very long path - This is a very long path - This is a very long path - This is a very long path - This is a very long path -\This is a very long path - This is a very long path - This is a very long path - This is a very lon\aaaaaaaaaaaaaassssssssssssssdddddddddd\TestMediaThis is a very long path - This is a very long path - This is a very long path - This is a very long path - This is a very long path -This is a very long path - This is a very long path - Thi - 不要抬头.ts"

It's not only a path with len > MAX_PATH, the name also includes 
Chinese (non-ANSI) characters.
All this is working in a predictable and reliable way on all common Windows
versions.


Recently, MS have taken additional effort in order to ease compatibility 
and improve platform portability, by allowing applications to use ANSI
file APIs with long paths and UTF-8 charset. 
BUT: there are several prerequisites for this to work:

1. Windows version needs to be >= 1903
   (for being able to have both attributes in effect)

2. Application needs to be compiled with and manifest file as resource

3. A registry key or group policy needs to be set on Windows to enable this
´  (in both cases, administrative permission/UAC is required to set it)

4. Even when registry key or group policy is set, it might still be pending
   a reboot


SUMMARY

From my understanding, the benefit of this patchset would be:

When 1. and 2. and 3. and 4. are fulfilled (and only then) - it would be 
possible to use long path names in ffmpeg _without_ prefixing it with '\\?\'

On the other side, there's a risk of regressions by adding those manifest
attributes.

I wonder whether it wouldn’t be a better idea, to simply auto-add this prefix
in the ffmpeg file handling code if:

- OS == Windows
- len > MAX_PATH

This would work on all common Windows versions and would be predictable and
reliable.

Best regards,
softworkz

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> Admirari
> Sent: Saturday, April 23, 2022 10:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h:
> Add whcartoutf8, wchartoansi and utf8toansi
> 
> 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 90f08245..c0e5d47e 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.32.0
> 

LGTM now as it seems to be of use in several places.  
_______________________________________________
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".