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

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

[nil-admirari]

> 1. Patch 3/6 - Replace LoadLibraryExA with LoadLibraryExW
> What's the point in making changes to library loading? What does it fix?



From the commit https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295571.html:

... the path length limitation is only lifted for file APIs that pass paths as wchar_t...



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

> ...

> Both of these manifest attributes are affecting the runtime behavior of
> an application running on Windows - but only starting from a certain 

> OS version.

> ...

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



The same can be said about e.g. DirectX 11 hardware acceleration, which is available

only starting with Windows 7; and you have to check OS version before providing

parameters that enable it. Does not mean implementing that feature made no sense,

since it made FFmpeg inconsistent with previous versions of Windows. Consistency

with the greatest common denominator does not make much sense.



Considering that people from yt-dlp and StaxRip projects requested long paths feature,

looks like it does make sense to the users.



> 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?



All patches with that line replace MAX_PATH-sized buffers with dynamically-allocated

buffers of potentially arbitrary size. If application continues to use fixed-size buffers,

it's not long path aware, regardless of the manifest.


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

It simply moves the burden of long path support from FFmpeg to FFmpeg clients.



> All this is working in a predictable and reliable way on all common Windows

> versions.



It is neither predictable nor reliable. Please check https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html

for things like “I actually exploited this behaviour to create

arbitrary named pipes from the Chrome sandbox awhile back.”

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



For all intents and purposes, versions of Windows < 1903 have EOLed. Exceptions:

- Windows 8.1 will EOL in January 2023,

- LTSB/LTSC branches of Windows 10 before 21H2.

None of these exceptions are particularly popular with users.



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



Which is exactly what these patches are doing.



> 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



Is it a big deal to change a registry and reboot?


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

> attributes.



Adding anything risks regressions, how manifest is supposed to be different

from any other patch?


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



Every command line argument that supplies a path needs to be updated.

Every filter with a file argument needs to be updated.

Code in https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295568.html

normalises a path to use '/' instead of '\\' for whatever reason, and it cannot use \\?\

since it requires backslashes.



How adding a prefix \\?\ is supposed to be simpler than replacing a couple of buffers?



> This would work on all common Windows versions and would be predictable and

> reliable.



See https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html

for predictability and reliability.


_______________________________________________
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@mailo.com
> Sent: Sunday, April 24, 2022 2:08 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > 1. Patch 3/6 - Replace LoadLibraryExA with LoadLibraryExW
> > What's the point in making changes to library loading? What does it
> fix?
> 
> 
> 
> From the commit https://ffmpeg.org/pipermail/ffmpeg-devel/2022-
> April/295571.html:
> 
> ... the path length limitation is only lifted for file APIs that pass
> paths as wchar_t...

You normally don't load libraries from such longs paths. As I had written 
previously (but stripped off by you):

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

Which dll do you think would need to be loaded from a long path?
----------------------------------------------------------------


> > 2. Patches 5/6 and 6/6 - Add Fusion Manifest
> > ...
> > Both of these manifest attributes are affecting the runtime behavior
> of
> > an application running on Windows - but only starting from a certain
> > OS version.
> > ... 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.
> 
> 
> The same can be said about e.g. DirectX 11 hardware acceleration,
> which is available
> only starting with Windows 7; and you have to check OS version before
> providing
> parameters that enable it. Does not mean implementing that feature
> made no sense,
> since it made FFmpeg inconsistent with previous versions of Windows.
> Consistency
> 
> with the greatest common denominator does not make much sense.

Your example is making a comparison that is not quite valid IMO.
The use of DirectX 11 is a very specific feature and that surely 
involves checking of availability.

But file IO is a fundamental feature where a common and predictable 
behavior is crucial.

----------------------------------------------------------------
 
> > 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?
> 
> All patches with that line replace MAX_PATH-sized buffers with
> dynamically-allocated
> 
> buffers of potentially arbitrary size. If application continues to use
> fixed-size buffers,
> 
> it's not long path aware, regardless of the manifest.

That's true, but the file IO in ffmpeg does not use MAX_PATH buffers,
so it's only about those few specific cases:

Patch 2/6: Removing the fixed-size buffer is surely a good change,
           but can't you just completely remove the charset 
           conversion and use the same file IO patterns that ffmpeg
           is using everywhere else?
Patch 3/6: see 1.
Patch 4/6: Seems to be pointless because you cannot run ffmpeg.exe from
           a long path anyway. Neither with cmd nor with powershell.
           Even if you could, it would be a pretty exotic use case.


> It is neither predictable nor reliable. Please check
> https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-
> on-win32-to-nt.html
> 
> for things like “I actually exploited this behaviour to create
> arbitrary named pipes from the Chrome sandbox awhile back.”

I know about that. But it's something that is possible now already
and would be still possible after applying your patchset.
So, that's not a relevant point in this regard.


> > 1. Windows version needs to be >= 1903
> > (for being able to have both attributes in effect)
> For all intents and purposes, versions of Windows < 1903 have EOLed.
> Exceptions:
> 
> - Windows 8.1 will EOL in January 2023,
> - LTSB/LTSC branches of Windows 10 before 21H2.

And the servers? And all those which are still using older 
versions?`

> > 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
> 
> Is it a big deal to change a registry and reboot?

Yes it is. Especially when you don't know that it's needed or whether
it's needed or when it is needed.
Also it requires administrative permissions which not everybody has.
Further it's a serious change to OS behavior and you cannot expect that 
all users are educated enough for being able to make this decision 
and be confident that it won't have any unexpected side effects.

> > On the other side, there's a risk of regressions by adding those
> manifest
> > attributes.
> 
> Adding anything risks regressions, how manifest is supposed to be
> different from any other patch?

That's what I tried to explain. "Any other patch" makes a change
after which you know that the change has been made.

But adding those attributes will impose changes that will be effective
under certain conditions only and about which neither the user nor the
application "knows" whether they are effective or not.

> > I wonder whether it wouldn’t be a better idea, to simply auto-add
> this prefix
> > in the ffmpeg file handling code if:
> 
> Every command line argument that supplies a path needs to be updated.
> Every filter with a file argument needs to be updated.

No. There's are common file APIs where such change could be made.

Best regards,
softworkz

PS: Your line breaks were all doubled. Please check before replying.


_______________________________________________
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

[nil-admirari]

> You normally don't load libraries from such longs paths.

And? 80% of FFmpeg functionality is normally not used.

> But file IO is a fundamental feature where a common and predictable 
> behavior is crucial.

You're talking as if the manifest change somehow broke or fundamentally altered
file IO, which it clearly did not.

> That's true, but the file IO in ffmpeg does not use MAX_PATH buffers,
> so it's only about those few specific cases:

I'm glad it doesn't use MAX_PATH everywhere. Takes only 6 pathes to fix,
not a thousand.

> Patch 2/6: Removing the fixed-size buffer is surely a good change,
> but can't you just completely remove the charset 
> conversion and use the same file IO patterns that ffmpeg
> is using everywhere else?

If you were to look at the code, you would've seen that charset conversion
was already there. AviSynth is not Unicode-aware, it expects ANSI strings.
Inline charset conversion was replaced by a library call, which, again,
was already there, only slightly extended.

> Patch 4/6: Seems to be pointless because you cannot run ffmpeg.exe from
> a long path anyway. Neither with cmd nor with powershell.
> Even if you could, it would be a pretty exotic use case.

This current limitation may disappear in a future version of Windows,
the same way path limit disappeared.

> And the servers?

Servers have the same lifecycle as client versions do.

> And all those which are still using older versions?

For them manifest change is a no-op.

> Yes it is. Especially when you don't know that it's needed or whether
> it's needed or when it is needed.
> Also it requires administrative permissions which not everybody has.
> Further it's a serious change to OS behavior and you cannot expect that 
> all users are educated enough for being able to make this decision 
> and be confident that it won't have any unexpected side effects.

What does all of that have to do with these patches?

> That's what I tried to explain. "Any other patch" makes a change
> after which you know that the change has been made.
>
> But adding those attributes will impose changes that will be effective
> under certain conditions only and about which neither the user nor the
> application "knows" whether they are effective or not.

Before these patches FFmpeg cannot handle long paths. After these patches, it can.
They are certainly not invisible.

> No. There's are common file APIs where such change could be made.

During the review of these patches it was found that parts FFmpeg use av_fopen_utf8,
while others use plain fopen. There is no common file API.

> PS: Your line breaks were all doubled. Please check before replying.

Were normal in email client. I don't know how they got garbled. My previous messages
didn't have such a problem.



_______________________________________________
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@mailo.com
> Sent: Monday, April 25, 2022 11:04 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > You normally don't load libraries from such longs paths.
> 
> And? 80% of FFmpeg functionality is normally not used.

Again, you have deleted my asking for an example scenario
and which library would need to be loaded from a long path.


> > But file IO is a fundamental feature where a common and predictable
> > behavior is crucial.
> 
> You're talking as if the manifest change somehow broke or
> fundamentally altered
> file IO, which it clearly did not.

For the longpaths attribute, you could surely argue that it's "just"
that you don't know whether it will work or not.
But forcing a different code page for a process _IS_ a fundamental
alteration. 


I'm restoring the line of your own question which you have deleted:

> > > > Is it a big deal to change a registry and reboot?

My response to that was:

> > Yes it is. Especially when you don't know that it's needed or
> whether
> > it's needed or when it is needed.
> > Also it requires administrative permissions which not everybody has.
> > Further it's a serious change to OS behavior and you cannot expect
> that
> > all users are educated enough for being able to make this decision
> > and be confident that it won't have any unexpected side effects.
> 
> What does all of that have to do with these patches?

Really? I answer your question and then you delete your question 
and ask what my answer would have to do with the patchset?

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

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

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

[nil-admirari]

> Again, you have deleted my asking for an example scenario
> and which library would need to be loaded from a long path.

Because I don't think that an example would be relevant. Generic library
function must be able to load a library, no matter the location.
You're talking as if MAX_PATH limited library loader is self-evidently
superior, and it is the loader that has no such limitation that has to justify
its existence. As far as I'm concerned it just the other way around.

> For the longpaths attribute, you could surely argue that it's "just"
> that you don't know whether it will work or not.
> But forcing a different code page for a process _IS_ a fundamental
> alteration. 

Which is necessary for compatibility with AviSynth, and commit message
says exactly that: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295572.html.

> I'm restoring the line of your own question which you have deleted:
> > > > > Is it a big deal to change a registry and reboot?
> My response to that was:
> ...
> Really? I answer your question and then you delete your question 
> and ask what my answer would have to do with the patchset?

Sorry, I should've directly asked what:

> 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

has to do with this patchset, instead of asking a rhetorical question of whether
it's a big deal or not.



_______________________________________________
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@mailo.com
> Sent: Monday, April 25, 2022 11:52 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > Again, you have deleted my asking for an example scenario
> > and which library would need to be loaded from a long path.
> 
> Because I don't think that an example would be relevant.

A code change for which no use case exists and does not provide
any benefit is not relevant. That's my point.


> > For the longpaths attribute, you could surely argue that it's "just"
> > that you don't know whether it will work or not.
> > But forcing a different code page for a process _IS_ a fundamental
> > alteration.
> 
> Which is necessary for compatibility with AviSynth, and commit message
> says exactly that: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-
> April/295572.html.

Yes, I understand. You want to make a fundamental change to ffmpeg 
because of AviSynth.


> > I'm restoring the line of your own question which you have deleted:
> > > > > > Is it a big deal to change a registry and reboot?
> > My response to that was:
> > ...
> > Really? I answer your question and then you delete your question
> > and ask what my answer would have to do with the patchset?
> 
> Sorry, I should've directly asked what:
> 
> > 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
> 
> has to do with this patchset, instead of asking a rhetorical question
> of whether
> it's a big deal or not.

Imagine, you are creating a software (no matter whether you're big or small,
open or closed source, targeting business or home users, using a custom or 
public built ffmpeg) and you bundle ffmpeg.exe with your software like many 
are doing. Now, re-read my comments, maybe it will make more sense to you.

If not - I'm in no way authoritative for ffmpeg, others may have different 
opinions. 

From my point of view:
ffmpeg is already working pretty well in handling long file paths (also with 
Unicode characters) when pre-fixing paths with \\?\, and this is working 
on all Windows versions without all the caveats, requirements and conditions
that I mentioned.

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

[Hendrik Leppkes]

On Mon, Apr 25, 2022 at 1:12 PM Soft Works <softworkz@hotmail.com> wrote:
>
> From my point of view:
> ffmpeg is already working pretty well in handling long file paths (also with
> Unicode characters) when pre-fixing paths with \\?\, and this is working
> on all Windows versions without all the caveats, requirements and conditions
> that I mentioned.

"We have already worked around this problem in our deployment,
therefore there is no need to try to improve it" is surely not a very
strong argument.

Will your work-around continue to work? Yes.
Will the changes actually impact anyone negatively? No known case is
documented, here or otherwise.
Will this change objectively improve the operation of ffmpeg on
Windows? Maybe not for everyone (yet), but certainly it'll allow it to
do so in controlled environments.

I'm not seeing a good argument here to generally block the patch on,
as this entire thread boils down to .. what? Fear of change?
Unless you can demonstrate an actual problem resulting from applying
this patch, this line of arguments seems not very productive.

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

[Martin Storsjö]

On Mon, 25 Apr 2022, Hendrik Leppkes wrote:

> On Mon, Apr 25, 2022 at 1:12 PM Soft Works <softworkz@hotmail.com> wrote:
>>
>> From my point of view:
>> ffmpeg is already working pretty well in handling long file paths (also with
>> Unicode characters) when pre-fixing paths with \\?\, and this is working
>> on all Windows versions without all the caveats, requirements and conditions
>> that I mentioned.
>
> "We have already worked around this problem in our deployment,
> therefore there is no need to try to improve it" is surely not a very
> strong argument.
>
> Will your work-around continue to work? Yes.
> Will the changes actually impact anyone negatively? No known case is
> documented, here or otherwise.
> Will this change objectively improve the operation of ffmpeg on
> Windows? Maybe not for everyone (yet), but certainly it'll allow it to
> do so in controlled environments.
>
> I'm not seeing a good argument here to generally block the patch on,
> as this entire thread boils down to .. what? Fear of change?
> Unless you can demonstrate an actual problem resulting from applying
> this patch, this line of arguments seems not very productive.

+1, I agree here.

Asking users to manually use \\?\ paths isn't something we should 
recommend as solution.

Internally prepending \\?\ when necessary could work (and would also be a 
possible fix, which at least would fix everything that goes through avio), 
but that's clearly much more risky than just adding a mostly-noop 
manifest. (And that only works for absolute paths; it requires some path 
munging logic to be able to do that.) I wouldn't mind going that way too, 
but the current patchset seems risk free to me.

FWIW, LLVM does something like that [1] - before opening files, it checks 
if a path seems to be too long, and if it is, it converts it into an 
absolute path and adds such a prefix. But that's clearly more risky and 
more nontrivial than this patchset.

[1] https://github.com/llvm/llvm-project/blob/llvmorg-14.0.0/llvm/lib/Support/Windows/Path.inc#L98-L124

// 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 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
> Martin Storsjö
> Sent: Monday, April 25, 2022 3:02 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> On Mon, 25 Apr 2022, Hendrik Leppkes wrote:
> 
> > On Mon, Apr 25, 2022 at 1:12 PM Soft Works <softworkz@hotmail.com>
> wrote:
> >>
> >> From my point of view:
> >> ffmpeg is already working pretty well in handling long file paths
> (also with
> >> Unicode characters) when pre-fixing paths with \\?\, and this is
> working
> >> on all Windows versions without all the caveats, requirements and
> conditions
> >> that I mentioned.
> >
> > "We have already worked around this problem in our deployment,
> > therefore there is no need to try to improve it" is surely not a
> very
> > strong argument.
> >
> > Will your work-around continue to work? Yes.
> > Will the changes actually impact anyone negatively? No known case is
> > documented, here or otherwise.
> > Will this change objectively improve the operation of ffmpeg on
> > Windows? Maybe not for everyone (yet), but certainly it'll allow it
> to
> > do so in controlled environments.
> >
> > I'm not seeing a good argument here to generally block the patch on,
> > as this entire thread boils down to .. what? Fear of change?
> > Unless you can demonstrate an actual problem resulting from applying
> > this patch, this line of arguments seems not very productive.
> 
> +1, I agree here.
> 
> Asking users to manually use \\?\ paths isn't something we should
> recommend as solution.
> 
> Internally prepending \\?\ when necessary could work (and would also
> be a
> possible fix, which at least would fix everything that goes through
> avio),
> but that's clearly much more risky than just adding a mostly-noop
> manifest. (And that only works for absolute paths; it requires some
> path
> munging logic to be able to do that.) I wouldn't mind going that way
> too,
> but the current patchset seems risk free to me.
> 
> FWIW, LLVM does something like that [1] - before opening files, it
> checks
> if a path seems to be too long, and if it is, it converts it into an
> absolute path and adds such a prefix. But that's clearly more risky
> and
> more nontrivial than this patchset.

I see it the other way round.

This patchset does not provide reliable behavior. This way, you won't 
be able to use long paths with ffmpeg within the next 5-8 years at minimum,
because even in the latest versions of Windows 11, this is not enabled by 
default in the operating system.

What is the value of adding a capability where it will be a lottery
game whether it will work or not on each system?

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

[nil-admirari]

> This patchset does not provide reliable behavior.

Actually it does.

> This way, you won't be able to use long paths with ffmpeg within the next 5-8 years at minimum,

Long paths can be used since August, 2 2016. Some ~6 years have already passed.

> because even in the latest versions of Windows 11, this is not enabled by 
> default in the operating system.

FFmpeg isn't bundled by default either. And not everyone has rights to download and run
arbitrary software on their machines.

> What is the value of adding a capability where it will be a lottery
> game whether it will work or not on each system?

Windows 10 > 1607 + registry key = it works. Otherwise it doesn't. There is no lottery.



_______________________________________________
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
> Hendrik Leppkes
> Sent: Monday, April 25, 2022 2:52 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> On Mon, Apr 25, 2022 at 1:12 PM Soft Works <softworkz@hotmail.com>
> wrote:
> >
> > From my point of view:
> > ffmpeg is already working pretty well in handling long file paths
> (also with
> > Unicode characters) when pre-fixing paths with \\?\, and this is
> working
> > on all Windows versions without all the caveats, requirements and
> conditions
> > that I mentioned.
> 
> "We have already worked around this problem in our deployment,
> therefore there is no need to try to improve it" is surely not a very
> strong argument.
> 
> Will your work-around continue to work? Yes.
> Will the changes actually impact anyone negatively? No known case is
> documented, here or otherwise.

You may want to read this:

https://docs.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

> Will this change objectively improve the operation of ffmpeg on
> Windows? Maybe not for everyone (yet), but certainly it'll allow it to
> do so in controlled environments.
> 
> I'm not seeing a good argument here to generally block the patch on,
> as this entire thread boils down to .. what? Fear of change?

Pardon?

Seems you missed my point (probably you haven't followed the full conversation). 
What I'm saying is that prepending the long path prefix is the better way 
for supporting long paths and I mentioned our experience with it only to 
confirm that it's working well.

The .NET/corefx runtime uses the prefix method internally, rclone is using it,
the Java runtime is using it, just to name a few examples.

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

[nil-admirari]

> What I'm saying is that prepending the long path prefix is the better way 
> for supporting long paths and I mentioned our experience with it only to 
> confirm that it's working well.

Maybe you'll even provide a patchset for such a wonderful approach?

> The .NET/corefx runtime uses the prefix method internally, rclone is using it,
> the Java runtime is using it, just to name a few examples.

They all have uniform IO, FFmpeg does not. av_fopen_utf8 vs plain fopen,
and forward slash conversion have already been mentioned in this very conversation,
but probably you haven't followed.



_______________________________________________
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

[nil-admirari]

> A code change for which no use case exists and does not provide
> any benefit is not relevant. That's my point.

You've deleted me saying

>> You're talking as if MAX_PATH limited library loader is self-evidently
>> superior, and it is the loader that has no such limitation that has to justify
>> its existence. As far as I'm concerned it just the other way around.

and now claim that code change is irrelevant.

> Imagine, you are creating a software (no matter whether you're big or small,
> open or closed source, targeting business or home users, using a custom or 
> public built ffmpeg) and you bundle ffmpeg.exe with your software like many 
> are doing. Now, re-read my comments, maybe it will make more sense to you.

They do not. Customer ask for long path support and gets two responses:

1. Enable long path support via registry once, and never care about it again.
2. Convert path to absolute and prefix it with \\?\ every time you use our software.

I don't see how second workaround can be any good at all. With the first option,
customers can at least pressure Microsoft into making it a default, if registry tweak
is too much a hassle for them; with the second they're stuck with workarounds—forever.

> ffmpeg is already working pretty well in handling long file paths (also with 
> Unicode characters) when pre-fixing paths with \\?\

It handles them most of the time, but not always. I already mentioned that the code from
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295568.html
explicitly converts all backslashes into forward slashes for unknown reasons,
and that code will not work with \\?\ paths because //?/ is not a valid prefix.
There are probably other places like that. Examples you've given simply
do not exercise such code paths, but it does mean they do not exists.



_______________________________________________
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@mailo.com
> Sent: Friday, April 29, 2022 8:52 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > A code change for which no use case exists and does not provide
> > any benefit is not relevant. That's my point.
> 
> You've deleted me saying
> 
> >> You're talking as if MAX_PATH limited library loader is self-
> evidently
> >> superior, and it is the loader that has no such limitation that has
> to justify
> >> its existence. As far as I'm concerned it just the other way
> around.
> 
> and now claim that code change is irrelevant.

Yes, because there is no realistic case in which ffmpeg will need
to load any dll from a long path.
I've asked you multiple times to give an example.

> > Imagine, you are creating a software (no matter whether you're big
> or small,
> > open or closed source, targeting business or home users, using a
> custom or
> > public built ffmpeg) and you bundle ffmpeg.exe with your software
> like many
> > are doing. Now, re-read my comments, maybe it will make more sense
> to you.
> 
> They do not. Customer ask for long path support and gets two
> responses:
> 
> 1. Enable long path support via registry once, and never care about it
> again.
> 2. Convert path to absolute and prefix it with \\?\ every time you use
> our software.
> 
> I don't see how second workaround can be any good at all. With the
> first option,
> customers can at least pressure Microsoft into making it a default

So, ffmpeg should implement something that doesn't work without a
registry change in order to "pressure Microsoft" to change it at 
some point in the future?

> , if
> registry tweak
> is too much a hassle for them; with the second they're stuck with
> workarounds—forever.

I apologize when I have created the impression that ffmpeg should
be left as is and users should continue adding the long path prefix
themselves. I'm not opposed to adding support for long paths, I'm
just opposed to the method.

Implementing long path support by adding the prefix is not 
a "workaround". Those kinds of paths are part of OS functionality
since Windows NT and will still work in Windows 20 or whatever
may come. 

What IS a workaround is having to add the prefix manually to 
paths when using ffmpeg.

You wrote

> with the second they're stuck with
> workarounds—forever.

But it's just the other way round: 
Because with the first, you cannot rely that long paths are
working, you would be stuck needing to continue prefixing paths 
manually "forever".
(to also cover cases where it's not working)


> > ffmpeg is already working pretty well in handling long file paths
> (also with
> > Unicode characters) when pre-fixing paths with \\?\
> 
> It handles them most of the time, but not always. I already mentioned
> that the code from
> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295568.html
> explicitly converts all backslashes into forward slashes for unknown
> reasons,
> and that code will not work with \\?\ paths because //?/ is not a
> valid prefix.

IIRC this code is handling code paths relative to the executable path,
and the executable doesn't run from a long path anyway.

Nonetheless it might make sense to adjust that slash replacement for 
Windows. Same like you, I don't know why it has been added.

> > This patchset does not provide reliable behavior.
> 
> Actually it does.
> 
> > This way, you won't be able to use long paths with ffmpeg within the
> next 5-8 years at minimum,
> 
> Long paths can be used since August, 2 2016. Some ~6 years have
> already passed.

You know what I meant: it's not activated by default.

> > because even in the latest versions of Windows 11, this is not
> enabled by
> > default in the operating system.
> 
> FFmpeg isn't bundled by default either. And not everyone has rights to
> download and run arbitrary software on their machines.

That's right, but that's not a question that matters because that's
not in the hands of ffmpeg.

What is in the hands of ffmpeg though, is requiring a change which
needs administrative privileges or not.

> > What is the value of adding a capability where it will be a lottery
> > game whether it will work or not on each system?
> 
> Windows 10 > 1607 + registry key = it works. Otherwise it doesn't.
> There is no lottery.

I don't want to argue about the term lottery.

What I'm saying is that a solution is preferable which doesn't have
an "Otherwise it doesn't" part.

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

[nil-admirari]

> Yes, because there is no realistic case in which ffmpeg will need
> to load any dll from a long path.
> I've asked you multiple times to give an example.

Managed to find time to actually test: https://postimg.cc/F1C64nq1.
Windows 10 21H2 LTSC, long paths are enabled and 8.3 paths are disabled
via group policy.

CMD is capable of starting applications from long paths: test app prints get_module_filename().
As can be seen from the output, module path is an actual long path, not 8.3 short name.
Windows is clearly capable of running apps from long paths. Always was
via the first argument to CreateProcess, prefixed with your favourite \\?\.
This facility is no longer esoteric and is available to end users via CMD.

FFmpeg in its current form doesn't work when started from a long path,
even with these patches. It prints nothing and just returns a non-zero exit code.
That's a problem with FFmpeg. Something in FFmpeg startup breaks down
when running from a long path, though it's not clear exactly what.

You're talking about realistic use cases as if someone made a careful analysis
of use cases before putting the limit in place. No one did, the limit came from the OS.
OS no longer has this limit, so neither should FFmpeg.

> So, ffmpeg should implement something that doesn't work without a
> registry change in order to "pressure Microsoft" to change it at
> some point in the future?

Things are implemented in order to provide features, not to pressure someone.
You still have to abide by API's contract, however. Long paths are enabled
by a registry tweak, and that's how things are.

> I apologize when I have created the impression that ffmpeg should
> be left as is and users should continue adding the long path prefix
> themselves. I'm not opposed to adding support for long paths, I'm
> just opposed to the method.

As a matter of fact, you are. Your alternative method implies ploughing
through hundreds of C files normalising path handling across the entire application,
and then fixing numerous regressions introduced along the way. No one is doing that,
just too much work. Manifest is the only practically implementable solution,
and if it's not accepted, FFmpeg stays as it is and users are forced to continue
to manually prefix all paths.

Of course, if you think so strongly that \\?\ is the right way to go,
then how about writing the necessary patches yourself?

And even then I have some bad news for you. MAX_PATH-sized buffers simply do not work
with long paths, even the ones that start with \\?\. You will still have to replace
them with dynamically allocated buffers. And that's what the majority of this patch-set
is about, not about the manifest. Manifest is just a single patch.

> Implementing long path support by adding the prefix is not
> a "workaround". Those kinds of paths are part of OS functionality
> since Windows NT and will still work in Windows 20 or whatever
> may come.

It is and always was. \\?\ support none of the usual path handling
such as relative paths, slash-collapsing, removal of . from paths etc.

> Because with the first, you cannot rely that long paths are
> working, you would be stuck needing to continue prefixing paths
> manually "forever".
> (to also cover cases where it's not working)

No one is prefixing the paths. Neither end users, nor application writers.
If it doesn't work without a prefix, it doesn't work at all, and you end up
without any long path support whatsoever.

> IIRC this code is handling code paths relative to the executable path,
> and the executable doesn't run from a long path anyway.

Because it's broken in FFmpeg, not because you cannot run an app from a long path.
See above.

> > This patchset does not provide reliable behavior.
> ...
> You know what I meant: it's not activated by default.

Not active by default is not the same as not reliable.

> What is in the hands of ffmpeg though, is requiring a change which
> needs administrative privileges or not.

It doesn't require administrative rights. It requires a registry tweak.
If you don't have admin rights but still need the feature, you can ask
your system administrator.

> I don't want to argue about the term lottery.

You're perfectly willing to use it.

> What I'm saying is that a solution is preferable which doesn't have
> an "Otherwise it doesn't" part.

As Thomas Sowell has put it: "There are no solutions. There are only trade-offs."
Do not pretend that \\?\ is even practical, considering the scope of changes required
to implement it.



_______________________________________________
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@mailo.com
> Sent: Thursday, May 5, 2022 10:20 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi


[..]

> As a matter of fact, you are. Your alternative method implies
> ploughing
> through hundreds of C files normalising path handling across the
> entire application,

Almost everybody here knows that this isn't true.


> And even then I have some bad news for you. MAX_PATH-sized buffers
> simply do not work
> with long paths, even the ones that start with \\?\. You will still
> have to replace
> them with dynamically allocated buffers. And that's what the majority
> of this patch-set

Why should that be bad news for me?

Those are three very specific cases and we had already covered this.
I had also said that it surely makes sense to eliminate fixed size buffers.

But that's all just distraction from the main subject, which is 
"Long Path Support in ffmpeg on Windows".

My point is very simple: It should be a solution that will always 
work and not just sometimes.

I stripped the remaining points because this is just going in circles.
My opinion should be sufficiently explained by now, and I'd like to 
leave room for other opinions.

Kind 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

[nil-admirari]

> > As a matter of fact, you are. Your alternative method implies
> > ploughing
> > through hundreds of C files normalising path handling across the
> > entire application,

> Almost everybody here knows that this isn't true.

I do not. During the review of just this particular patchset it was found that FFmpeg
sometimes uses av_fopen_utf8 and sometimes just plain fopen. Plain fopens
were already replaced with av_fopen_utf8 and then reverted back.
because suddenly it turned out that av_fopen_utf8 is problematic:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295488.html.
At least in one place backward slashes are being replaced with forward slashes,
which is not compatible with \\?\.

But you can easily prove me wrong by showing me your patches. As a starting point:
it was already shown how LLVM prefixes paths
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295677.html.
If that functionality is to be ported into FFmpeg, where exactly should the code go?
Is there a Path struct, analogous to LLVM class, that all of FFmpeg is using?
Or FFmpeg isn't using any special structs and paths are indistinguishable
from ordinary strings?

> Why should that be bad news for me?
> Those are three very specific cases and we had already covered this.

Because none of this is covered. They are covered by my patches,
and you're against merging them.

> My point is very simple: It should be a solution that will always 
> work and not just sometimes.

Manifest works whether you like it or not. People without the registry setting
do not have it for the simple reason of never having to work with long paths.
And they most likely will enable it the first time such a need arises.



_______________________________________________
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@mailo.com
> Sent: Friday, May 6, 2022 6:08 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > > As a matter of fact, you are. Your alternative method implies
> > > ploughing
> > > through hundreds of C files normalising path handling across the
> > > entire application,
> 
> > Almost everybody here knows that this isn't true.
> 
> I do not. During the review of just this particular patchset it was
> found that FFmpeg
> sometimes uses av_fopen_utf8 and sometimes just plain fopen. Plain
> fopens
> were already replaced with av_fopen_utf8 and then reverted back.
> because suddenly it turned out that av_fopen_utf8 is problematic:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295488.html.

Read again. As each lib gets its own copy of file_open.c there's
no problem using av_fopen_utf8. The concern in that message was
about it being a public API that could be used by external callers.

> At least in one place backward slashes are being replaced with forward
> slashes,
> which is not compatible with \\?\.

That's the pending issue with your 4/6 which is probably ok otherwise.


> > Why should that be bad news for me?
> > Those are three very specific cases and we had already covered this.
> 
> Because none of this is covered. They are covered by my patches,
> and you're against merging them.

5/6 + 6/6 are the manifest changes
4/6 see above (forward slash replacement problem)
3/6 is pointless without 5/6
2/6 is pointless without 6/6
1/6 remaining bits can be inlined in 4/6

Kind 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

[nil-admirari]

You have completely ignored my question, haven't you? Here it is again:

>> Is there a Path struct, analogous to LLVM class, that all of FFmpeg is using?
>> Or FFmpeg isn't using any special structs and paths are indistinguishable
>> from ordinary strings?

> Read again. As each lib gets its own copy of file_open.c there's
> no problem using av_fopen_utf8. The concern in that message was
> about it being a public API that could be used by external callers.

av_fopen_utf8 wasn't mentioned because it has a particular problem.
It was mentioned to say that
>> FFmpeg sometimes uses av_fopen_utf8 and sometimes just plain fopen
i.e. there is no standardised path handling.

> That's the pending issue with your 4/6 which is probably ok otherwise.

It's not an issue with my patch. It's something that was already there,
and is retained not to break something.

> 3/6 is pointless without 5/6

It is pointless in one and only one case: no long path support arrives ever.
Please reread:

> MAX_PATH-sized buffers simply do not work with long paths, even the ones
> that start with \\?\. You will still have to replace them with dynamically
> allocated buffers. And that's what the majority of this patch-set
> is about, not about the manifest.

> 2/6 is pointless without 6/6

Wrong. 6/6 enables process-wide UTF-8. utf8toansi allocates buffers of necessary size
instead of using MAX_PATH. utf8toansi doesn't care whether ansi is UTF-8 or a legacy encoding.

> 1/6 remaining bits can be inlined in 4/6

Why should they? Are two calls to WideCharToMultiByte + allocation more readable than
a single call to wchartoansi?



_______________________________________________
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@mailo.com
> Sent: Saturday, May 7, 2022 7:33 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> You have completely ignored my question, haven't you? Here it is
> again:
> 
> >> Is there a Path struct, analogous to LLVM class, that all of FFmpeg
> is using?
> >> Or FFmpeg isn't using any special structs and paths are
> indistinguishable
> >> from ordinary strings?

Paths are strings.

> > Read again. As each lib gets its own copy of file_open.c there's
> > no problem using av_fopen_utf8. The concern in that message was
> > about it being a public API that could be used by external callers.
> 
> av_fopen_utf8 wasn't mentioned because it has a particular problem.
> It was mentioned to say that
> >> FFmpeg sometimes uses av_fopen_utf8 and sometimes just plain fopen
> i.e. there is no standardised path handling.
> 
> > That's the pending issue with your 4/6 which is probably ok
> otherwise.
> 
> It's not an issue with my patch. It's something that was already
> there,
> and is retained not to break something.

It needs to be found out why it was added to decide in which way it
can be adjusted.
 

> > 2/6 is pointless without 6/6
> 
> Wrong. 6/6 enables process-wide UTF-8. utf8toansi allocates buffers of
> necessary size
> instead of using MAX_PATH. utf8toansi doesn't care whether ansi is
> UTF-8 or a legacy encoding.

I am sorry about that one. I was under the impression that it would
make sense only when forcing UTF8 code page for the whole application
via manifest.
I have no objections about it then, of course.

Kind 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

[nil-admirari]

> Paths are strings.

In other languages, paths are represented with classes, e.g. std::filesystem::path in C++
or pathlib.Path in Python. In C classes have to be emulated with structs and functions.
Even plain strings would've been OK to represent paths, if there was a set of functions
everyone uses for path operations, but there is none. Which brings us to your second question.

> It needs to be found out why it was added to decide in which way it
> can be adjusted.

Backward to forward slash adjustment is there from the very beginning:
https://github.com/FFmpeg/FFmpeg/commit/1b30e4f5865260323da5232174fc68d6cc283f45.

Apparently it's needed for path concatenation, which is implemented with snprintf,
whose pattern uses / as a separator even on Windows:
https://github.com/FFmpeg/FFmpeg/blame/master/fftools/cmdutils.c#L843.

Now back to classes. If there was a Path class, you could've changed the constructor
and the path concat operation to normalise the path and prepend it with \\?\
when it exceeds MAX_PATH, which is what you want me to do. Two places only
(maybe some more), and the entire application becomes long-path aware.

But there is no Path class. Instead there are 587 occurrences of snprintf, each of which
potentially does path concat. snprintf is not alone, strcat and the likes can also be used
for path concat.

These hundreds of places have to be examined and potentially changed to prepend \\?\.
And a couple dozen of places will surely be missed in the process.

And all that is touted as being somehow better than changing a single manifest.



_______________________________________________
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@mailo.com
> Sent: Tuesday, May 10, 2022 11:23 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > Paths are strings.
> 
> In other languages, paths are represented with classes, e.g.
> std::filesystem::path in C++
> or pathlib.Path in Python. In C classes have to be emulated with
> structs and functions.
> Even plain strings would've been OK to represent paths, if there was a
> set of functions
> everyone uses for path operations, but there is none. Which brings us
> to your second question.
> 
> > It needs to be found out why it was added to decide in which way it
> > can be adjusted.
> 
> Backward to forward slash adjustment is there from the very beginning:
> https://github.com/FFmpeg/FFmpeg/commit/1b30e4f5865260323da5232174fc68
> d6cc283f45.
> 
> Apparently it's needed for path concatenation, which is implemented
> with snprintf,
> whose pattern uses / as a separator even on Windows:
> https://github.com/FFmpeg/FFmpeg/blame/master/fftools/cmdutils.c#L843.

I think that can be changed easily. A path using forward slashes can
still be prefixed with '\\?\', so it will just require to go through 
the few lines of code and fix cases where this might cause an issue.


> Now back to classes. If there was a Path class, you could've changed
> the constructor
> and the path concat operation to normalise the path and prepend it
> with \\?\
> when it exceeds MAX_PATH, which is what you want me to do. Two places
> only
> (maybe some more), and the entire application becomes long-path aware.
> 
> But there is no Path class. Instead there are 587 occurrences of
> snprintf, each of which
> potentially does path concat. snprintf is not alone, strcat and the
> likes can also be used
> for path concat.
> 
> These hundreds of places have to be examined and potentially changed
> to prepend \\?\.
> And a couple dozen of places will surely be missed in the process.

I'm afraid, it's not clear to me what you are talking about here.

The prefixing can be implemented as a function and then be used 
in file_open.c.

Other file system related functions like mkdir, rename, rmdir, etc.
are already intercepted in os_support.h, and the prefixing can be 
applied there.

Maybe I missed some cases, I have not fully analyzed the situation,
but surely there are just a small number of places that need to 
be changed and not 587.


For the procedure of prefixing I would take a look at how it's done
in .NET. This is probably the most mature code for handling this
and might save us from dealing with issues and regressions until we 
got it right.


Kind 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 Soft
> Works
> Sent: Wednesday, May 11, 2022 12:59 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> nil-
> > admirari@mailo.com
> > Sent: Tuesday, May 10, 2022 11:23 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> > utf8toansi
> >
> > > Paths are strings.
> >
> > In other languages, paths are represented with classes, e.g.
> > std::filesystem::path in C++
> > or pathlib.Path in Python. In C classes have to be emulated with
> > structs and functions.
> > Even plain strings would've been OK to represent paths, if there was
> a
> > set of functions
> > everyone uses for path operations, but there is none. Which brings
> us
> > to your second question.
> >
> > > It needs to be found out why it was added to decide in which way
> it
> > > can be adjusted.
> >
> > Backward to forward slash adjustment is there from the very
> beginning:
> >
> https://github.com/FFmpeg/FFmpeg/commit/1b30e4f5865260323da5232174fc68
> > d6cc283f45.
> >
> > Apparently it's needed for path concatenation, which is implemented
> > with snprintf,
> > whose pattern uses / as a separator even on Windows:
> >
> https://github.com/FFmpeg/FFmpeg/blame/master/fftools/cmdutils.c#L843.
> 
> I think that can be changed easily. A path using forward slashes can
> still be prefixed with '\\?\', so it will just require to go through
> the few lines of code and fix cases where this might cause an issue.
> 
> 
> > Now back to classes. If there was a Path class, you could've changed
> > the constructor
> > and the path concat operation to normalise the path and prepend it
> > with \\?\
> > when it exceeds MAX_PATH, which is what you want me to do. Two
> places
> > only
> > (maybe some more), and the entire application becomes long-path
> aware.
> >
> > But there is no Path class. Instead there are 587 occurrences of
> > snprintf, each of which
> > potentially does path concat. snprintf is not alone, strcat and the
> > likes can also be used
> > for path concat.
> >
> > These hundreds of places have to be examined and potentially changed
> > to prepend \\?\.
> > And a couple dozen of places will surely be missed in the process.
> 
> I'm afraid, it's not clear to me what you are talking about here.
> 
> The prefixing can be implemented as a function and then be used
> in file_open.c.
> 
> Other file system related functions like mkdir, rename, rmdir, etc.
> are already intercepted in os_support.h, and the prefixing can be
> applied there.
> 
> Maybe I missed some cases, I have not fully analyzed the situation,
> but surely there are just a small number of places that need to
> be changed and not 587.
> 
> 
> For the procedure of prefixing I would take a look at how it's done
> in .NET. This is probably the most mature code for handling this
> and might save us from dealing with issues and regressions until we
> got it right.

The logic they use is here:

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs

Probably it can be simplified a bit.

Kind 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

[Tobias Rapp]

On 11/05/2022 01:32, Soft Works wrote:
>>
>> [...]
>>
>> The prefixing can be implemented as a function and then be used
>> in file_open.c.
>>
>> Other file system related functions like mkdir, rename, rmdir, etc.
>> are already intercepted in os_support.h, and the prefixing can be
>> applied there.
>>
>> Maybe I missed some cases, I have not fully analyzed the situation,
>> but surely there are just a small number of places that need to
>> be changed and not 587.
>>
>>
>> For the procedure of prefixing I would take a look at how it's done
>> in .NET. This is probably the most mature code for handling this
>> and might save us from dealing with issues and regressions until we
>> got it right.
> 
> The logic they use is here:
> 
> https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs
> 
> Probably it can be simplified a bit.

Out of curiosity I searched for some automatic path prefixing code and 
the author of this file claims that it should be handling most corner cases:

https://github.com/JFLarvoire/SysToolsLib/blob/master/C/MsvcLibX/src/mb2wpath.c

That amount of logic inside CorrectWidePath() does not look appealing to 
me. And even if we simplify that now I guess it will grow once the 
corner cases drop in via bug reports.

So I'm in favor of removing the MAX_PATH limit, converting needed Win32 
function calls from ANSI to WideChar, and adding the longPathAware 
manifest flag. I think the activeCodePage=UTF-8 patch could break 
Windows applications that expect the system-wide ANSI codepage to be 
used for FFmpeg text output, though.

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 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
> Tobias Rapp
> Sent: Wednesday, May 11, 2022 9:46 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> On 11/05/2022 01:32, Soft Works wrote:
> >>
> >> [...]
> >>
> >> The prefixing can be implemented as a function and then be used
> >> in file_open.c.
> >>
> >> Other file system related functions like mkdir, rename, rmdir, etc.
> >> are already intercepted in os_support.h, and the prefixing can be
> >> applied there.
> >>
> >> Maybe I missed some cases, I have not fully analyzed the situation,
> >> but surely there are just a small number of places that need to
> >> be changed and not 587.
> >>
> >>
> >> For the procedure of prefixing I would take a look at how it's done
> >> in .NET. This is probably the most mature code for handling this
> >> and might save us from dealing with issues and regressions until we
> >> got it right.
> >
> > The logic they use is here:
> >
> >
> https://github.com/dotnet/runtime/blob/main/src/libraries/System.Priva
> te.CoreLib/src/System/IO/PathHelper.Windows.cs
> >
> > Probably it can be simplified a bit.
> 
> Out of curiosity I searched for some automatic path prefixing code and
> the author of this file claims that it should be handling most corner
> cases:
> 
> https://github.com/JFLarvoire/SysToolsLib/blob/master/C/MsvcLibX/src/m
> b2wpath.c
> 
> That amount of logic inside CorrectWidePath() does not look appealing
> to
> me. And even if we simplify that now I guess it will grow once the
> corner cases drop in via bug reports.

I would follow the MS code, that will be the safest way.

> So I'm in favor of removing the MAX_PATH limit, converting needed
> Win32
> function calls from ANSI to WideChar, and adding the longPathAware
> manifest flag.

I'm not sure how much you had followed, so please excuse in case you
had already read it: the manifest approach does not work on a default
Windows installation.
To activate long path support, the users needs to opt-in to a behavior
that is probably deactivated by default for some reason. Also it
requires administrative privileges to make this change.
For me - and probably for others as well - that approach is useless,
as it would be the same as if there would be no long path support.
(when you cannot rely on that feature being always working, you
cannot use it)

Kind 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

[Hendrik Leppkes]

On Wed, May 11, 2022 at 9:58 AM Soft Works <softworkz@hotmail.com> wrote:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Tobias Rapp
> > Sent: Wednesday, May 11, 2022 9:46 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> > utf8toansi
> >
> > On 11/05/2022 01:32, Soft Works wrote:
> > >>
> > >> [...]
> > >>
> > >> The prefixing can be implemented as a function and then be used
> > >> in file_open.c.
> > >>
> > >> Other file system related functions like mkdir, rename, rmdir, etc.
> > >> are already intercepted in os_support.h, and the prefixing can be
> > >> applied there.
> > >>
> > >> Maybe I missed some cases, I have not fully analyzed the situation,
> > >> but surely there are just a small number of places that need to
> > >> be changed and not 587.
> > >>
> > >>
> > >> For the procedure of prefixing I would take a look at how it's done
> > >> in .NET. This is probably the most mature code for handling this
> > >> and might save us from dealing with issues and regressions until we
> > >> got it right.
> > >
> > > The logic they use is here:
> > >
> > >
> > https://github.com/dotnet/runtime/blob/main/src/libraries/System.Priva
> > te.CoreLib/src/System/IO/PathHelper.Windows.cs
> > >
> > > Probably it can be simplified a bit.
> >
> > Out of curiosity I searched for some automatic path prefixing code and
> > the author of this file claims that it should be handling most corner
> > cases:
> >
> > https://github.com/JFLarvoire/SysToolsLib/blob/master/C/MsvcLibX/src/m
> > b2wpath.c
> >
> > That amount of logic inside CorrectWidePath() does not look appealing
> > to
> > me. And even if we simplify that now I guess it will grow once the
> > corner cases drop in via bug reports.
>
> I would follow the MS code, that will be the safest way.
>
> > So I'm in favor of removing the MAX_PATH limit, converting needed
> > Win32
> > function calls from ANSI to WideChar, and adding the longPathAware
> > manifest flag.
>
> I'm not sure how much you had followed, so please excuse in case you
> had already read it: the manifest approach does not work on a default
> Windows installation.
> To activate long path support, the users needs to opt-in to a behavior
> that is probably deactivated by default for some reason. Also it
> requires administrative privileges to make this change.
> For me - and probably for others as well - that approach is useless,
> as it would be the same as if there would be no long path support.
> (when you cannot rely on that feature being always working, you
> cannot use it)
>

We've been over this - not every feature needs to be useful to
everyone. The addition of the manifest does not make existing behavior
worse, nor does it prevent working on an alternate solution in the
future.
These are not reasons to block it. If you want to contribute an
alternate solution, you are welcome to do so, this does not block that
or impede that in any way - if anything, it lays the groundwork to do
that, because it already removes the MAX_PATH limitations from the
code in various places.

If you can provide an argument why this patch should not be applied
that isn't "its not a 100% solution for everything", then you are
welcome to do so, otherwise you are beating a dead horse at this
point.
If you want a better solution - provide one.

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

[nil-admirari]

> I'm not sure how much you had followed, so please excuse in case you
> had already read it: the manifest approach does not work on a default
> Windows installation.
> To activate long path support, the users needs to opt-in to a behavior
> that is probably deactivated by default for some reason. Also it
> requires administrative privileges to make this change.
> For me - and probably for others as well - that approach is useless,
> as it would be the same as if there would be no long path support.
> (when you cannot rely on that feature being always working, you
> cannot use it)

For yt-dlp and StaxRip users manifest change is not the same as
the complete absence of a feature. If it's that way for you, well it doesn't take
much effort to pretend that the manifest is just not there.



_______________________________________________
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

[Tobias Rapp]

On 11/05/2022 09:57, Soft Works wrote:
> 
> [...]
> 
> I'm not sure how much you had followed, so please excuse in case you
> had already read it: the manifest approach does not work on a default
> Windows installation.
> To activate long path support, the users needs to opt-in to a behavior
> that is probably deactivated by default for some reason. Also it
> requires administrative privileges to make this change.
> For me - and probably for others as well - that approach is useless,
> as it would be the same as if there would be no long path support.
> (when you cannot rely on that feature being always working, you
> cannot use it)

For me an analogous case is the usage of the "--large-address-aware" 
linker flag. It enables FFmpeg to use more than 2GiB of memory on Win32. 
That feature is useless to users having <= 2GiB of total system memory 
available, and it makes no guarantee that an OOM error is avoided. Still 
it allows to exceed a limit for those that match the requirements -- 
knowingly or by pure coincidence.

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 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
> Tobias Rapp
> Sent: Wednesday, May 11, 2022 3:33 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> On 11/05/2022 09:57, Soft Works wrote:
> >
> > [...]
> >
> > I'm not sure how much you had followed, so please excuse in case you
> > had already read it: the manifest approach does not work on a
> default
> > Windows installation.
> > To activate long path support, the users needs to opt-in to a
> behavior
> > that is probably deactivated by default for some reason. Also it
> > requires administrative privileges to make this change.
> > For me - and probably for others as well - that approach is useless,
> > as it would be the same as if there would be no long path support.
> > (when you cannot rely on that feature being always working, you
> > cannot use it)
> 
> For me an analogous case is the usage of the "--large-address-aware"
> linker flag. It enables FFmpeg to use more than 2GiB of memory on
> Win32.
> That feature is useless to users having <= 2GiB of total system memory
> available, and it makes no guarantee that an OOM error is avoided.
> Still
> it allows to exceed a limit for those that match the requirements --
> knowingly or by pure coincidence.

That's similar except for the fact that there wasn't an alternative
method that would always work.

I respect others opinions, but I don't agree to the "better than 
nothing approach". I'm following this mailing list for a long time
and suggesting a full working solution over a half-baked or half-
working solution has always been a valid argument, so I don't see
why it shouldn't be in this case.

My interest is to have a good solution, not to block anything, so
I'll try to prepare a patch implementing path prefixing shortly.

Thanks,
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

[nil-admirari]

> I think that can be changed easily.

How about writing the necessary patches yourself?

> A path using forward slashes can still be prefixed with '\\?\'

It cannot. Only backslashes are valid in \\?\ paths.

> The prefixing can be implemented as a function and then be used 
> in file_open.c.
> Other file system related functions like mkdir, rename, rmdir, etc.
> are already intercepted in os_support.h, and the prefixing can be 
> applied there.

I haven't found path concat in these headers. What I did find, however,
is the use of snprintf with forward slash separators right inside the #ifdef _WIN32:

#if defined(_WIN32) || defined (__ANDROID__) || defined(__DJGPP__)
    if (fd < 0) {
        snprintf(*filename, len, "./%sXXXXXX", prefix);
        fd = mkstemp(*filename);
    }
#endif

> I have not fully analyzed the situation,
> but surely there are just a small number of places that need to 
> be changed and not 587.

587 was obtained with fgrep snprintf **/*.c | wc -l (became 588 after a recent pull).
At least two of these uses of snprintf correspond to path concatenation,
and have been already presented here. Not all of them do, but I have no interest
in examining the other 586 cases. But if you think that it's easy, well go ahead.



_______________________________________________
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@mailo.com
> Sent: Wednesday, May 11, 2022 10:57 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > I think that can be changed easily.
> 
> How about writing the necessary patches yourself?

I did and submitted today.


> > The prefixing can be implemented as a function and then be used
> > in file_open.c.
> > Other file system related functions like mkdir, rename, rmdir, etc.
> > are already intercepted in os_support.h, and the prefixing can be
> > applied there.
> 
> I haven't found path concat in these headers. What I did find,
> however,
> is the use of snprintf with forward slash separators right inside the
> #ifdef _WIN32:
> 
> #if defined(_WIN32) || defined (__ANDROID__) || defined(__DJGPP__)
>     if (fd < 0) {
>         snprintf(*filename, len, "./%sXXXXXX", prefix);
>         fd = mkstemp(*filename);
>     }
> #endif

All these paths end up either in win32_open (in file_open.c) or in
one of the functions mapped inside os_support.h where they are now 
(with my submitted patchset) handled by the get_extended_win32_path()
function, which handles all cases (e.g. forward slashes, relative paths,
prefixed, non-prefixed, UNC, drive letters, long, short, etc.) in
the same way as .NET does.


> > I have not fully analyzed the situation,
> > but surely there are just a small number of places that need to
> > be changed and not 587.
> 
> 587 was obtained with fgrep snprintf **/*.c | wc -l (became 588 after
> a recent pull).
> At least two of these uses of snprintf correspond to path
> concatenation,
> and have been already presented here. Not all of them do, but I have
> no interest
> in examining the other 586 cases. But if you think that it's easy,
> well go ahead.

I did that now, and you can see that it's as easy as I said.

IMO, submitting "competing" patches is not a nice behavior in general,
I only did that because you asked me to.
Hence, I didn't include replacements for those of your commits that I 
think are still valid; they should be merged as well, even though 
they are just covering niche cases and are not that much relevant 
to "long path support" in general, which is from my understanding 
primarily about being able to access input and output files on long 
paths rather than running ffmpeg.exe (et al.) residing at a long 
path location.

Kind 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

[nil-admirari]

> All these paths end up either in win32_open (in file_open.c) or in
> one of the functions mapped inside os_support.h where they are now 
> (with my submitted patchset) handled by the get_extended_win32_path()
> function, which handles all cases (e.g. forward slashes, relative paths,
> prefixed, non-prefixed, UNC, drive letters, long, short, etc.) in
> the same way as .NET does.

You are saved by the fact that nobody tries to get a path name from
a file descriptor or a FILE* (which is indeed difficult, but possible),
and then appending additional path components to such obtained path name;
thus turning a normalised path name into denormalised one.

.NET by the way, is not fully \\?\-aware. Its Path.Combine
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs
would blindly append "..\relative" to "\\?\C:\extended" turning it
into invalid "\\?\C:\extended\..\relative"

> I did that now, and you can see that it's as easy as I said.

If it's so easy, then why I'm still finding problems:
- absence of \\.\ handling: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296447.html
- stat and is_dos_path: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296448.html
- av_fopen_utf8 remnants: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296452.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 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@mailo.com
> Sent: Sunday, May 15, 2022 9:54 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
> utf8toansi
> 
> > All these paths end up either in win32_open (in file_open.c) or in
> > one of the functions mapped inside os_support.h where they are now
> > (with my submitted patchset) handled by the
> get_extended_win32_path()
> > function, which handles all cases (e.g. forward slashes, relative
> paths,
> > prefixed, non-prefixed, UNC, drive letters, long, short, etc.) in
> > the same way as .NET does.
> 
> You are saved by the fact that nobody tries to get a path name from
> a file descriptor or a FILE* (which is indeed difficult, but
> possible),
> and then appending additional path components to such obtained path
> name;
> thus turning a normalised path name into denormalised one.
> 
> .NET by the way, is not fully \\?\-aware. Its Path.Combine
> https://github.com/dotnet/runtime/blob/main/src/libraries/System.Priva
> te.CoreLib/src/System/IO/Path.cs
> would blindly append "..\relative" to "\\?\C:\extended" turning it
> into invalid "\\?\C:\extended\..\relative"

And what's the point about this?


> > I did that now, and you can see that it's as easy as I said.
> 
> If it's so easy, then why I'm still finding problems:
> - absence of \\.\ handling: https://ffmpeg.org/pipermail/ffmpeg-
> devel/2022-May/296447.html

Not an issue as explained.

> - stat and is_dos_path: https://ffmpeg.org/pipermail/ffmpeg-
> devel/2022-May/296448.html

Will look into it.

> - av_fopen_utf8 remnants: https://ffmpeg.org/pipermail/ffmpeg-
> devel/2022-May/296452.html

Not required as explained.

Thanks again,
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

[nil-admirari]

> And what's the point about this?

Point is obvious: extended paths are difficult to handle correctly.
get_extended_win32_path cannot be used on its own, only as a final step
before getting FILE* or a file descriptor.



_______________________________________________
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

[Martin Storsjö]

On Sat, 7 May 2022, Soft Works wrote:

>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of nil-
>> admirari@mailo.com
>> Sent: Friday, May 6, 2022 6:08 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v11 1/6]
>> libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and
>> utf8toansi
>>
>>>> As a matter of fact, you are. Your alternative method implies
>>>> ploughing
>>>> through hundreds of C files normalising path handling across the
>>>> entire application,
>>
>>> Almost everybody here knows that this isn't true.
>>
>> I do not. During the review of just this particular patchset it was
>> found that FFmpeg
>> sometimes uses av_fopen_utf8 and sometimes just plain fopen. Plain
>> fopens
>> were already replaced with av_fopen_utf8 and then reverted back.
>> because suddenly it turned out that av_fopen_utf8 is problematic:
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295488.html.
>
> Read again. As each lib gets its own copy of file_open.c there's
> no problem using av_fopen_utf8. The concern in that message was
> about it being a public API that could be used by external callers.

No, it's not quite as simple.

Yes, each library gets its own copy of av_fopen_utf8, but fftools doesn't 
get its own copy. That's why it's a real problem.

(Secondly, one part of the trick of giving each library its own copy of is 
that avpriv_open is renamed ff_open, so that it is not an exported 
function. But in the case of av_fopen_utf8, it's an entirely public API, 
so we can't hide it.)

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

[Stephen Hutchinson]

Thus far I've avoided jumping into this because I genuinely do not care
about Windows' code page shenanigans or what-all, but because this seems
to be zeroing in on one thing in particular...

On 4/25/22 5:03 AM, nil-admirari@mailo.com wrote:
> If you were to look at the code, you would've seen that charset conversion
> was already there. AviSynth is not Unicode-aware, it expects ANSI strings.
> Inline charset conversion was replaced by a library call, which, again,
> was already there, only slightly extended.
> 

 > Which is necessary for compatibility with AviSynth,

It has nothing to do with compatibility with AviSynth, because
AviSynth just uses the code page the system is set on.
What 'it's not Unicode-aware' means is that it doesn't
jump into convoluted conversions to UTF-16, because that's
what Windows meant by 'Unicode' for 20+ years,
and nobody from either classic AviSynth or Plus wanted to
do that, because it's an absolute mess and now that Plus
is cross-platform, utterly pointless on every other OS,
because they all use UTF-8 and have for at least 15 years,
if not [much] longer.

Some third-party applications that utilize AviSynth have
decided they want to bend over backwards to make sure their users
don't have to freak themselves out by telling Windows to use UTF-8 as 
the system code page, so they'll do an end-run around it and use
these sorts of manifests to force the program's locale to UTF-8,
AppLocale style. But that's not a compatibility issue
with AviSynth.  As soon as Microsoft actually makes UTF-8
the default code page going forward, that issue will poof
out of existence, as if by magic. It already does if you
toggle it on in the system settings.

Nothing special was required regarding locale support when
Linux, macOS, BSD, and Haiku support landed.  All of those use
UTF-8 and AviSynth+ works with it transparently.  And it works
with UTF-8 on Windows transparently if you've gone into the Region
Settings and actually enabled it.  Even AviSynth 2.6 is cool
with it, as this screenshot from *two years ago* illustrates:

https://i.imgur.com/1p5osrE.jpg
_______________________________________________
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

[nil-admirari]

> As soon as Microsoft actually makes UTF-8
> the default code page going forward, that issue will poof
> out of existence, as if by magic. It already does if you
> toggle it on in the system settings.

True. System-wide UTF-8 can cause problems with legacy software, but starting
with Windows 11 such software can specify a non-UTF8 activeCodePage for itself:
https://docs.microsoft.com/en-us/windows/win32/sbscs/application-manifests#activeCodePage.

"Set active code page to UTF-8 on Windows" is a separate patch, independent from all others:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295572.html.
If it's deemed to be too problematic, the rest of the patchset can simply be merged without it.



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