Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
@ 2022-06-13 16:26 Nil Admirari
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Nil Admirari @ 2022-06-13 16:26 UTC (permalink / raw)
  To: ffmpeg-devel

wchartoutf8() converts strings returned by WinAPI into UTF-8,
which is FFmpeg's preffered encoding.

Some external dependencies, such as AviSynth, are still
not Unicode-enabled. utf8toansi() converts UTF-8 strings
into ANSI in two steps: UTF-8 -> wchar_t -> ANSI.
wchartoansi() is responsible for the second step of the conversion.
Conversion in just one step is not supported by WinAPI.

Since these character converting functions allocate the buffer
of necessary size, they also facilitate the removal of MAX_PATH limit
in places where fixed-size ANSI/WCHAR strings were used
as filename buffers.

getenv_utf8() wraps _wgetenv() converting its input from
and its output to UTF-8. Compared to plain getenv(),
getenv_utf8() requires a cleanup.

Because of that, in places that only test the existence of
an environment variable or compare its value with a string
consisting entirely of ASCII characters, the use of plain getenv()
is still preferred. (libavutil/log.c check_color_terminal()
is an example of such a place.)

Plain getenv() is also preffered in UNIX-only code,
such as bktr.c, fbdev_common.c, oss.c in libavdevice
or af_ladspa.c in libavfilter.
---
 libavutil/getenv_utf8.h    | 63 ++++++++++++++++++++++++++++++++++++++
 libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 libavutil/getenv_utf8.h

diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
new file mode 100644
index 0000000000..2c48a36355
--- /dev/null
+++ b/libavutil/getenv_utf8.h
@@ -0,0 +1,63 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVUTIL_GETENV_UTF8_H
+#define AVUTIL_GETENV_UTF8_H
+
+#include <stdlib.h>
+
+#include "mem.h"
+
+#ifdef _WIN32
+
+#include "libavutil/wchar_filename.h"
+
+static inline char *getenv_utf8(const char *varname)
+{
+    wchar_t *varname_w, *var_w;
+    char *var;
+
+    if (utf8towchar(varname, &varname_w))
+        return NULL;
+    if (!varname_w)
+        return NULL;
+
+    var_w = _wgetenv(varname_w);
+    av_free(varname_w);
+
+    if (!var_w)
+        return NULL;
+    if (wchartoutf8(var_w, &var))
+        return NULL;
+
+    return var;
+
+    // No CP_ACP fallback compared to other *_utf8() functions:
+    // non UTF-8 strings must not be returned.
+}
+
+#else
+
+static inline char *getenv_utf8(const char *varname)
+{
+    return av_strdup(getenv(varname));
+}
+
+#endif // _WIN32
+
+#endif // AVUTIL_GETENV_UTF8_H
diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index f36d9dfea3..a6d71e52e5 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -41,6 +41,57 @@ static inline int utf8towchar(const char *filename_utf8, wchar_t **filename_w)
     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 ? WC_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_malloc_array(num_chars, sizeof *filename);
+    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;
+}
+
 /**
  * Checks for extended path prefixes for which normalization needs to be skipped.
  * see .NET6: PathInternal.IsExtended()
-- 
2.34.1



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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-13 16:26 [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
@ 2022-06-13 16:26 ` Nil Admirari
  2022-06-13 16:39   ` Soft Works
  2022-06-13 20:17   ` Soft Works
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv() Nil Admirari
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 21+ messages in thread
From: Nil Admirari @ 2022-06-13 16:26 UTC (permalink / raw)
  To: ffmpeg-devel

---
 compat/w32dlfcn.h     | 80 ++++++++++++++++++++++++++++++++++---------
 libavcodec/mf_utils.h |  1 +
 2 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
index 52a94efafb..e49d3841aa 100644
--- a/compat/w32dlfcn.h
+++ b/compat/w32dlfcn.h
@@ -22,9 +22,31 @@
 #ifdef _WIN32
 #include <windows.h>
 #include "config.h"
-#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
 #include "libavutil/wchar_filename.h"
-#endif
+
+static inline wchar_t *get_module_filename(HMODULE module)
+{
+    wchar_t *path = NULL, *new_path;
+    DWORD path_size = 0, path_len;
+
+    do {
+        path_size = path_size ? 2 * path_size : MAX_PATH;
+        new_path = av_realloc_array(path, path_size, sizeof *path);
+        if (!new_path) {
+            av_free(path);
+            return NULL;
+        }
+        path = new_path;
+        path_len = GetModuleFileNameW(module, path, path_size);
+    } while (path_len && path_size <= 32768 && path_size <= path_len);
+
+    if (!path_len) {
+        av_free(path);
+        return NULL;
+    }
+    return path;
+}
+
 /**
  * Safe function used to open dynamic libs. This attempts to improve program security
  * by removing the current directory from the dll search path. Only dll's found in the
@@ -34,29 +56,53 @@
  */
 static inline HMODULE win32_dlopen(const char *name)
 {
+    wchar_t *name_w = NULL;
+    if (utf8towchar(name, &name_w))
+        name_w = NULL;
 #if _WIN32_WINNT < 0x0602
     // Need to check if KB2533623 is available
     if (!GetProcAddress(GetModuleHandleW(L"kernel32.dll"), "SetDefaultDllDirectories")) {
         HMODULE module = NULL;
-        wchar_t *path = NULL, *name_w = NULL;
-        DWORD pathlen;
-        if (utf8towchar(name, &name_w))
+        wchar_t *path = NULL, *new_path;
+        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 the 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 the case #2.
+                pathlen = GetSystemDirectoryW(path, pathsize);
+                if (!pathlen)
+                    goto exit;
+            }
+            path[pathlen] = L'\\';
             wcscpy(path + pathlen + 1, name_w);
             module = LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
         }
@@ -73,15 +119,17 @@ exit:
 #   define LOAD_LIBRARY_SEARCH_SYSTEM32        0x00000800
 #endif
 #if HAVE_WINRT
-    wchar_t *name_w = NULL;
     int ret;
-    if (utf8towchar(name, &name_w))
+    if (!name_w)
         return NULL;
     ret = LoadPackagedLibrary(name_w, 0);
     av_free(name_w);
     return ret;
 #else
-    return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
+    /* filename may be be in CP_ACP */
+    if (!name_w)
+        return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
+    return LoadLibraryExW(name_w, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
 #endif
 }
 #define dlopen(name, flags) win32_dlopen(name)
diff --git a/libavcodec/mf_utils.h b/libavcodec/mf_utils.h
index 3b12344f3e..aebfb9ad21 100644
--- a/libavcodec/mf_utils.h
+++ b/libavcodec/mf_utils.h
@@ -29,6 +29,7 @@
 // mf*.h headers below indirectly include strmif.h.)
 #include <icodecapi.h>
 #else
+#define NO_DSHOW_STRSAFE
 #include <dshow.h>
 // Older versions of mingw-w64 need codecapi.h explicitly included, while newer
 // ones include it implicitly from dshow.h (via uuids.h).
-- 
2.34.1



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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [FFmpeg-devel] [PATCH v14 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv()
  2022-06-13 16:26 [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
@ 2022-06-13 16:26 ` Nil Admirari
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Nil Admirari @ 2022-06-13 16:26 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/cmdutils.c   | 53 +++++++++++++++++++++++++++++++++-----------
 fftools/ffmpeg_opt.c |  9 ++++++--
 2 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 5d7cdc3e10..5e7fbbe2ee 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -39,6 +39,7 @@
 #include "libavutil/avstring.h"
 #include "libavutil/channel_layout.h"
 #include "libavutil/display.h"
+#include "libavutil/getenv_utf8.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/libm.h"
@@ -47,9 +48,11 @@
 #include "libavutil/dict.h"
 #include "libavutil/opt.h"
 #include "cmdutils.h"
+#include "fopen_utf8.h"
 #include "opt_common.h"
 #ifdef _WIN32
 #include <windows.h>
+#include "compat/w32dlfcn.h"
 #endif
 
 AVDictionary *sws_dict;
@@ -465,7 +468,7 @@ static void check_options(const OptionDef *po)
 void parse_loglevel(int argc, char **argv, const OptionDef *options)
 {
     int idx = locate_option(argc, argv, options, "loglevel");
-    const char *env;
+    char *env;
 
     check_options(options);
 
@@ -474,7 +477,8 @@ void parse_loglevel(int argc, char **argv, const OptionDef *options)
     if (idx && argv[idx + 1])
         opt_loglevel(NULL, "loglevel", argv[idx + 1]);
     idx = locate_option(argc, argv, options, "report");
-    if ((env = getenv("FFREPORT")) || idx) {
+    env = getenv_utf8("FFREPORT");
+    if (env || idx) {
         FILE *report_file = NULL;
         init_report(env, &report_file);
         if (report_file) {
@@ -487,6 +491,7 @@ void parse_loglevel(int argc, char **argv, const OptionDef *options)
             fflush(report_file);
         }
     }
+    av_free(env);
     idx = locate_option(argc, argv, options, "hide_banner");
     if (idx)
         hide_banner = 1;
@@ -812,28 +817,45 @@ FILE *get_preset_file(char *filename, size_t filename_size,
 {
     FILE *f = NULL;
     int i;
-    const char *base[3] = { getenv("FFMPEG_DATADIR"),
-                            getenv("HOME"),
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+    char *datadir = NULL;
+#endif
+    char *env_home = getenv_utf8("HOME");
+    char *env_ffmpeg_datadir = getenv_utf8("FFMPEG_DATADIR");
+    const char *base[3] = { env_home,
+                            env_ffmpeg_datadir,
                             FFMPEG_DATADIR, };
 
     if (is_path) {
         av_strlcpy(filename, preset_name, filename_size);
-        f = fopen(filename, "r");
+        f = fopen_utf8(filename, "r");
     } else {
 #if HAVE_GETMODULEHANDLE && defined(_WIN32)
-        char datadir[MAX_PATH], *ls;
+        wchar_t *datadir_w = get_module_filename(NULL);
         base[2] = NULL;
 
-        if (GetModuleFileNameA(GetModuleHandleA(NULL), datadir, sizeof(datadir) - 1))
+        if (wchartoutf8(datadir_w, &datadir))
+            datadir = NULL;
+        av_free(datadir_w);
+
+        if (datadir)
         {
-            for (ls = datadir; ls < datadir + strlen(datadir); ls++)
+            char *ls;
+            for (ls = datadir; *ls; ls++)
                 if (*ls == '\\') *ls = '/';
 
             if (ls = strrchr(datadir, '/'))
             {
-                *ls = 0;
-                strncat(datadir, "/ffpresets",  sizeof(datadir) - 1 - strlen(datadir));
-                base[2] = datadir;
+                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
@@ -842,17 +864,22 @@ FILE *get_preset_file(char *filename, size_t filename_size,
                 continue;
             snprintf(filename, filename_size, "%s%s/%s.ffpreset", base[i],
                      i != 1 ? "" : "/.ffmpeg", preset_name);
-            f = fopen(filename, "r");
+            f = fopen_utf8(filename, "r");
             if (!f && codec_name) {
                 snprintf(filename, filename_size,
                          "%s%s/%s-%s.ffpreset",
                          base[i], i != 1 ? "" : "/.ffmpeg", codec_name,
                          preset_name);
-                f = fopen(filename, "r");
+                f = fopen_utf8(filename, "r");
             }
         }
     }
 
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+    av_free(datadir);
+#endif
+    av_free(env_ffmpeg_datadir);
+    av_free(env_home);
     return f;
 }
 
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 398067da96..f49acf6ad0 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -44,6 +44,7 @@
 #include "libavutil/avutil.h"
 #include "libavutil/bprint.h"
 #include "libavutil/channel_layout.h"
+#include "libavutil/getenv_utf8.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/fifo.h"
 #include "libavutil/mathematics.h"
@@ -1402,8 +1403,10 @@ static int get_preset_file_2(const char *preset_name, const char *codec_name, AV
 {
     int i, ret = -1;
     char filename[1000];
-    const char *base[3] = { getenv("AVCONV_DATADIR"),
-                            getenv("HOME"),
+    char *env_avconv_datadir = getenv_utf8("AVCONV_DATADIR");
+    char *env_home = getenv_utf8("HOME");
+    const char *base[3] = { env_avconv_datadir,
+                            env_home,
                             AVCONV_DATADIR,
                             };
 
@@ -1421,6 +1424,8 @@ static int get_preset_file_2(const char *preset_name, const char *codec_name, AV
             ret = avio_open2(s, filename, AVIO_FLAG_READ, &int_cb, NULL);
         }
     }
+    av_free(env_home);
+    av_free(env_avconv_datadir);
     return ret;
 }
 
-- 
2.34.1



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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 16:26 [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv() Nil Admirari
@ 2022-06-13 16:26 ` Nil Admirari
  2022-06-13 17:47   ` Soft Works
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 5/5] libavfilter/vf_frei0r.c: Use " Nil Admirari
  2022-06-13 20:50 ` [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Martin Storsjö
  4 siblings, 1 reply; 21+ messages in thread
From: Nil Admirari @ 2022-06-13 16:26 UTC (permalink / raw)
  To: ffmpeg-devel

1. getenv() is replaced with getenv_utf8() across libavformat.
2. New versions of AviSynth+ are now called with UTF-8 filenames.
3. Old versions of AviSynth are still using ANSI strings,
   but MAX_PATH limit on filename is removed.
---
 libavformat/avisynth.c    | 39 +++++++++++++++++++++++++++------------
 libavformat/http.c        | 20 +++++++++++++-------
 libavformat/ipfsgateway.c | 35 +++++++++++++++++++++++------------
 libavformat/tls.c         | 11 +++++++++--
 4 files changed, 72 insertions(+), 33 deletions(-)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 8ba2bdead2..a97d12b6b6 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
@@ -56,6 +57,7 @@ typedef struct AviSynthLibrary {
 #define AVSC_DECLARE_FUNC(name) name ## _func name
     AVSC_DECLARE_FUNC(avs_bit_blt);
     AVSC_DECLARE_FUNC(avs_clip_get_error);
+    AVSC_DECLARE_FUNC(avs_check_version);
     AVSC_DECLARE_FUNC(avs_create_script_environment);
     AVSC_DECLARE_FUNC(avs_delete_script_environment);
     AVSC_DECLARE_FUNC(avs_get_audio);
@@ -137,6 +139,7 @@ static av_cold int avisynth_load_library(void)
 
     LOAD_AVS_FUNC(avs_bit_blt, 0);
     LOAD_AVS_FUNC(avs_clip_get_error, 0);
+    LOAD_AVS_FUNC(avs_check_version, 0);
     LOAD_AVS_FUNC(avs_create_script_environment, 0);
     LOAD_AVS_FUNC(avs_delete_script_environment, 0);
     LOAD_AVS_FUNC(avs_get_audio, 0);
@@ -807,26 +810,38 @@ static int avisynth_create_stream(AVFormatContext *s)
 static int avisynth_open_file(AVFormatContext *s)
 {
     AviSynthContext *avs = s->priv_data;
-    AVS_Value arg, val;
+    AVS_Value val;
     int ret;
-#ifdef _WIN32
-    char filename_ansi[MAX_PATH * 4];
-    wchar_t filename_wc[MAX_PATH * 4];
-#endif
 
     if (ret = avisynth_context_create(s))
         return ret;
 
+    if (!avs_library.avs_check_version(avs->env, 7)) {
+        AVS_Value args[] = {
+            avs_new_value_string(s->url),
+            avs_new_value_bool(1) // filename is in UTF-8
+        };
+        val = avs_library.avs_invoke(avs->env, "Import",
+                                     avs_new_value_array(args, 2), 0);
+    } else {
+        AVS_Value arg;
 #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);
-    arg = avs_new_value_string(filename_ansi);
+        char *filename_ansi;
+        /* Convert UTF-8 to ANSI code page */
+        if (utf8toansi(s->url, &filename_ansi)) {
+            ret = AVERROR_UNKNOWN;
+            goto fail;
+        }
+        arg = avs_new_value_string(filename_ansi);
 #else
-    arg = avs_new_value_string(s->url);
+        arg = avs_new_value_string(s->url);
 #endif
-    val = avs_library.avs_invoke(avs->env, "Import", arg, 0);
+        val = avs_library.avs_invoke(avs->env, "Import", arg, 0);
+#ifdef _WIN32
+        av_free(filename_ansi);
+#endif
+    }
+
     if (avs_is_error(val)) {
         av_log(s, AV_LOG_ERROR, "%s\n", avs_as_error(val));
         ret = AVERROR_UNKNOWN;
diff --git a/libavformat/http.c b/libavformat/http.c
index c8f3f4b6a3..d90117e422 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -29,6 +29,7 @@
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
 #include "libavutil/bprint.h"
+#include "libavutil/getenv_utf8.h"
 #include "libavutil/opt.h"
 #include "libavutil/time.h"
 #include "libavutil/parseutils.h"
@@ -198,6 +199,7 @@ void ff_http_init_auth_state(URLContext *dest, const URLContext *src)
 static int http_open_cnx_internal(URLContext *h, AVDictionary **options)
 {
     const char *path, *proxy_path, *lower_proto = "tcp", *local_path;
+    char *env_http_proxy, *env_no_proxy;
     char *hashmark;
     char hostname[1024], hoststr[1024], proto[10];
     char auth[1024], proxyauth[1024] = "";
@@ -211,9 +213,13 @@ static int http_open_cnx_internal(URLContext *h, AVDictionary **options)
                  path1, sizeof(path1), s->location);
     ff_url_join(hoststr, sizeof(hoststr), NULL, NULL, hostname, port, NULL);
 
-    proxy_path = s->http_proxy ? s->http_proxy : getenv("http_proxy");
-    use_proxy  = !ff_http_match_no_proxy(getenv("no_proxy"), hostname) &&
+    env_http_proxy = getenv_utf8("http_proxy");
+    proxy_path = s->http_proxy ? s->http_proxy : env_http_proxy;
+
+    env_no_proxy = getenv_utf8("no_proxy");
+    use_proxy  = !ff_http_match_no_proxy(env_no_proxy, hostname) &&
                  proxy_path && av_strstart(proxy_path, "http://", NULL);
+    av_freep(&env_no_proxy);
 
     if (!strcmp(proto, "https")) {
         lower_proto = "tls";
@@ -224,7 +230,7 @@ static int http_open_cnx_internal(URLContext *h, AVDictionary **options)
         if (s->http_proxy) {
             err = av_dict_set(options, "http_proxy", s->http_proxy, 0);
             if (err < 0)
-                return err;
+                goto end;
         }
     }
     if (port < 0)
@@ -259,12 +265,12 @@ static int http_open_cnx_internal(URLContext *h, AVDictionary **options)
         err = ffurl_open_whitelist(&s->hd, buf, AVIO_FLAG_READ_WRITE,
                                    &h->interrupt_callback, options,
                                    h->protocol_whitelist, h->protocol_blacklist, h);
-        if (err < 0)
-            return err;
     }
 
-    return http_connect(h, path, local_path, hoststr,
-                        auth, proxyauth);
+end:
+    av_freep(&env_http_proxy);
+    return err < 0 ? err : http_connect(
+        h, path, local_path, hoststr, auth, proxyauth);
 }
 
 static int http_should_reconnect(HTTPContext *s, int err)
diff --git a/libavformat/ipfsgateway.c b/libavformat/ipfsgateway.c
index 83d52293b4..a8323403f0 100644
--- a/libavformat/ipfsgateway.c
+++ b/libavformat/ipfsgateway.c
@@ -20,6 +20,7 @@
  */
 
 #include "libavutil/avstring.h"
+#include "libavutil/getenv_utf8.h"
 #include "libavutil/opt.h"
 #include <sys/stat.h>
 #include "os_support.h"
@@ -55,12 +56,15 @@ static int populate_ipfs_gateway(URLContext *h)
     int stat_ret = 0;
     int ret = AVERROR(EINVAL);
     FILE *gateway_file = NULL;
+    char *env_ipfs_gateway, *env_ipfs_path;
 
     // Test $IPFS_GATEWAY.
-    if (getenv("IPFS_GATEWAY") != NULL) {
-        if (snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
-                     getenv("IPFS_GATEWAY"))
-            >= sizeof(c->gateway_buffer)) {
+    env_ipfs_gateway = getenv_utf8("IPFS_GATEWAY");
+    if (env_ipfs_gateway != NULL) {
+        int printed = snprintf(c->gateway_buffer, sizeof(c->gateway_buffer),
+                               "%s", env_ipfs_gateway);
+        av_freep(&env_ipfs_gateway);
+        if (printed >= sizeof(c->gateway_buffer)) {
             av_log(h, AV_LOG_WARNING,
                    "The IPFS_GATEWAY environment variable "
                    "exceeds the maximum length. "
@@ -77,20 +81,25 @@ static int populate_ipfs_gateway(URLContext *h)
 
     // We need to know the IPFS folder to - eventually - read the contents of
     // the "gateway" file which would tell us the gateway to use.
-    if (getenv("IPFS_PATH") == NULL) {
+    env_ipfs_path = getenv_utf8("IPFS_PATH");
+    if (env_ipfs_path == NULL) {
+        char *env_home = getenv_utf8("HOME");
+
         av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
 
         // Try via the home folder.
-        if (getenv("HOME") == NULL) {
+        if (env_home == NULL) {
             av_log(h, AV_LOG_WARNING, "$HOME appears to be empty.\n");
             ret = AVERROR(EINVAL);
             goto err;
         }
 
         // Verify the composed path fits.
-        if (snprintf(ipfs_full_data_folder, sizeof(ipfs_full_data_folder),
-                     "%s/.ipfs/", getenv("HOME"))
-            >= sizeof(ipfs_full_data_folder)) {
+        int printed = snprintf(
+            ipfs_full_data_folder, sizeof(ipfs_full_data_folder),
+            "%s/.ipfs/", env_home);
+        av_freep(&env_home);
+        if (printed >= sizeof(ipfs_full_data_folder)) {
             av_log(h, AV_LOG_WARNING,
                    "The IPFS data path exceeds the "
                    "max path length (%zu)\n",
@@ -113,9 +122,11 @@ static int populate_ipfs_gateway(URLContext *h)
             goto err;
         }
     } else {
-        if (snprintf(ipfs_full_data_folder, sizeof(ipfs_full_data_folder), "%s",
-                     getenv("IPFS_PATH"))
-            >= sizeof(ipfs_full_data_folder)) {
+        int printed = snprintf(
+            ipfs_full_data_folder, sizeof(ipfs_full_data_folder),
+            "%s", env_ipfs_path);
+        av_freep(&env_ipfs_path);
+        if (printed >= sizeof(ipfs_full_data_folder)) {
             av_log(h, AV_LOG_WARNING,
                    "The IPFS_PATH environment variable "
                    "exceeds the maximum length. "
diff --git a/libavformat/tls.c b/libavformat/tls.c
index 302c0f8d59..ea68f18d3a 100644
--- a/libavformat/tls.c
+++ b/libavformat/tls.c
@@ -26,6 +26,7 @@
 #include "url.h"
 #include "tls.h"
 #include "libavutil/avstring.h"
+#include "libavutil/getenv_utf8.h"
 #include "libavutil/opt.h"
 #include "libavutil/parseutils.h"
 
@@ -60,6 +61,7 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV
     char buf[200], opts[50] = "";
     struct addrinfo hints = { 0 }, *ai = NULL;
     const char *proxy_path;
+    char *env_http_proxy, *env_no_proxy;
     int use_proxy;
 
     set_options(c, uri);
@@ -89,9 +91,13 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV
     if (!c->host && !(c->host = av_strdup(c->underlying_host)))
         return AVERROR(ENOMEM);
 
-    proxy_path = c->http_proxy ? c->http_proxy : getenv("http_proxy");
-    use_proxy = !ff_http_match_no_proxy(getenv("no_proxy"), c->underlying_host) &&
+    env_http_proxy = getenv_utf8("http_proxy");
+    proxy_path = c->http_proxy ? c->http_proxy : env_http_proxy;
+
+    env_no_proxy = getenv_utf8("no_proxy");
+    use_proxy = !ff_http_match_no_proxy(env_no_proxy, c->underlying_host) &&
                 proxy_path && av_strstart(proxy_path, "http://", NULL);
+    av_freep(&env_no_proxy);
 
     if (use_proxy) {
         char proxy_host[200], proxy_auth[200], dest[200];
@@ -104,6 +110,7 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV
                     proxy_port, "/%s", dest);
     }
 
+    av_freep(&env_http_proxy);
     return ffurl_open_whitelist(&c->tcp, buf, AVIO_FLAG_READ_WRITE,
                                 &parent->interrupt_callback, options,
                                 parent->protocol_whitelist, parent->protocol_blacklist, parent);
-- 
2.34.1



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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [FFmpeg-devel] [PATCH v14 5/5] libavfilter/vf_frei0r.c: Use UTF-8 version of getenv()
  2022-06-13 16:26 [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
                   ` (2 preceding siblings ...)
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
@ 2022-06-13 16:26 ` Nil Admirari
  2022-06-13 20:50 ` [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Martin Storsjö
  4 siblings, 0 replies; 21+ messages in thread
From: Nil Admirari @ 2022-06-13 16:26 UTC (permalink / raw)
  To: ffmpeg-devel

---
 libavfilter/vf_frei0r.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
index f11ae6e55c..727e96561a 100644
--- a/libavfilter/vf_frei0r.c
+++ b/libavfilter/vf_frei0r.c
@@ -31,6 +31,7 @@
 #include "libavutil/avstring.h"
 #include "libavutil/common.h"
 #include "libavutil/eval.h"
+#include "libavutil/getenv_utf8.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/internal.h"
 #include "libavutil/mathematics.h"
@@ -204,7 +205,7 @@ static av_cold int frei0r_init(AVFilterContext *ctx,
     }
 
     /* see: http://frei0r.dyne.org/codedoc/html/group__pluglocations.html */
-    if ((path = av_strdup(getenv("FREI0R_PATH")))) {
+    if ((path = getenv_utf8("FREI0R_PATH"))) {
 #ifdef _WIN32
         const char *separator = ";";
 #else
@@ -231,12 +232,17 @@ static av_cold int frei0r_init(AVFilterContext *ctx,
         if (ret < 0)
             return ret;
     }
-    if (!s->dl_handle && (path = getenv("HOME"))) {
+    if (!s->dl_handle && (path = getenv_utf8("HOME"))) {
         char *prefix = av_asprintf("%s/.frei0r-1/lib/", path);
-        if (!prefix)
-            return AVERROR(ENOMEM);
+        if (!prefix) {
+            ret = AVERROR(ENOMEM);
+            goto home_path_end;
+        }
         ret = load_path(ctx, &s->dl_handle, prefix, dl_name);
         av_free(prefix);
+
+    home_path_end:
+        av_free(path);
         if (ret < 0)
             return ret;
     }
-- 
2.34.1



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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
@ 2022-06-13 16:39   ` Soft Works
  2022-06-13 17:03     ` nil-admirari
  2022-06-13 20:17   ` Soft Works
  1 sibling, 1 reply; 21+ messages in thread
From: Soft Works @ 2022-06-13 16:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> Admirari
> Sent: Monday, June 13, 2022 6:26 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove
> MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
> 
> ---
>  compat/w32dlfcn.h     | 80 ++++++++++++++++++++++++++++++++++-------
> --
>  libavcodec/mf_utils.h |  1 +
>  2 files changed, 65 insertions(+), 16 deletions(-)
> 
> diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
> index 52a94efafb..e49d3841aa 100644
> --- a/compat/w32dlfcn.h
> +++ b/compat/w32dlfcn.h
> @@ -22,9 +22,31 @@
>  #ifdef _WIN32
>  #include <windows.h>
>  #include "config.h"
> -#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
>  #include "libavutil/wchar_filename.h"
> -#endif
> +
> +static inline wchar_t *get_module_filename(HMODULE module)
> +{
> +    wchar_t *path = NULL, *new_path;
> +    DWORD path_size = 0, path_len;
> +
> +    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);

Why do you use a 'do' loop? Can't you use the normal 2-step
approach, i.e. call the winapi function with a NULL buffer,
and then use the returned size to allocate the buffer.
This way you always need a single allocation only.


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-13 16:39   ` Soft Works
@ 2022-06-13 17:03     ` nil-admirari
  2022-06-13 19:02       ` Soft Works
  0 siblings, 1 reply; 21+ messages in thread
From: nil-admirari @ 2022-06-13 17:03 UTC (permalink / raw)
  To: ffmpeg-devel

> Why do you use a 'do' loop? Can't you use the normal 2-step
> approach, i.e. call the winapi function with a NULL buffer,
> and then use the returned size to allocate the buffer.
> This way you always need a single allocation only.

GetModuleFileNameW does not follow this convention:
https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-getmodulefilenamew

> If the length of the path exceeds the size that the nSize parameter specifies,
> the function succeeds and the string is truncated to nSize characters
> including the terminating null character.

MS does the looping too in their WIL library:
https://github.com/microsoft/wil/blob/master/include/wil/win32_helpers.h#L339-L341.



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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
@ 2022-06-13 17:47   ` Soft Works
  2022-06-13 18:55     ` Hendrik Leppkes
  2022-06-13 20:53     ` nil-admirari
  0 siblings, 2 replies; 21+ messages in thread
From: Soft Works @ 2022-06-13 17:47 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> Admirari
> Sent: Monday, June 13, 2022 6:26 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH
> limit and use UTF-8 version of getenv()
> 
> 1. getenv() is replaced with getenv_utf8() across libavformat.
> 2. New versions of AviSynth+ are now called with UTF-8 filenames.
> 3. Old versions of AviSynth are still using ANSI strings,
>    but MAX_PATH limit on filename is removed.
> ---
>  libavformat/avisynth.c    | 39 +++++++++++++++++++++++++++----------
> --
>  libavformat/http.c        | 20 +++++++++++++-------
>  libavformat/ipfsgateway.c | 35 +++++++++++++++++++++++------------
>  libavformat/tls.c         | 11 +++++++++--
>  4 files changed, 72 insertions(+), 33 deletions(-)
> 
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 8ba2bdead2..a97d12b6b6 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
> @@ -56,6 +57,7 @@ typedef struct AviSynthLibrary {
>  #define AVSC_DECLARE_FUNC(name) name ## _func name
>      AVSC_DECLARE_FUNC(avs_bit_blt);
>      AVSC_DECLARE_FUNC(avs_clip_get_error);
> +    AVSC_DECLARE_FUNC(avs_check_version);
>      AVSC_DECLARE_FUNC(avs_create_script_environment);
>      AVSC_DECLARE_FUNC(avs_delete_script_environment);
>      AVSC_DECLARE_FUNC(avs_get_audio);
> @@ -137,6 +139,7 @@ static av_cold int avisynth_load_library(void)
> 
>      LOAD_AVS_FUNC(avs_bit_blt, 0);
>      LOAD_AVS_FUNC(avs_clip_get_error, 0);
> +    LOAD_AVS_FUNC(avs_check_version, 0);
>      LOAD_AVS_FUNC(avs_create_script_environment, 0);
>      LOAD_AVS_FUNC(avs_delete_script_environment, 0);
>      LOAD_AVS_FUNC(avs_get_audio, 0);
> @@ -807,26 +810,38 @@ static int
> avisynth_create_stream(AVFormatContext *s)
>  static int avisynth_open_file(AVFormatContext *s)
>  {
>      AviSynthContext *avs = s->priv_data;
> -    AVS_Value arg, val;
> +    AVS_Value val;
>      int ret;
> -#ifdef _WIN32
> -    char filename_ansi[MAX_PATH * 4];
> -    wchar_t filename_wc[MAX_PATH * 4];
> -#endif
> 
>      if (ret = avisynth_context_create(s))
>          return ret;
> 
> +    if (!avs_library.avs_check_version(avs->env, 7)) {

I like the version check. I don't know about all the derivatives
of AviSynth, but I assume you have checked that it's valid for
the common ones (or at least the original non-Plus variant)?

> +        AVS_Value args[] = {
> +            avs_new_value_string(s->url),
> +            avs_new_value_bool(1) // filename is in UTF-8
> +        };
> +        val = avs_library.avs_invoke(avs->env, "Import",
> +                                     avs_new_value_array(args, 2),
> 0);
> +    } else {
> +        AVS_Value arg;
>  #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);
> -    arg = avs_new_value_string(filename_ansi);
> +        char *filename_ansi;
> +        /* Convert UTF-8 to ANSI code page */
> +        if (utf8toansi(s->url, &filename_ansi)) {

Two ideas came to my mind how this could be done better.
What's actually needed here is not a string conversion, we need
a valid and usable filename, and the function could be more
something like "get_ansi_filename()".

The first thing that this function could do is to convert the
the filename to ANSI and right back to UTF-8, then compare the
UTF-8 result with the original UTF-8 string. When both are equal,
we know that the conversion is safe, otherwise we know that it
won't work.

Then, we can use the win32 API GetShortFileName(). Which returns
file and directory names in 8.3 notation which (IIRC) contains
only letters which are valid in the ANSI code page.

8.3 file names do not always exist (depending on system config), 
but it's always worth trying.

Should both of these procedures fail, we could at least output
a useful message, explaining why it doesn't work.

Let me know what you think.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 17:47   ` Soft Works
@ 2022-06-13 18:55     ` Hendrik Leppkes
  2022-06-13 19:00       ` Soft Works
  2022-06-13 19:07       ` Stephen Hutchinson
  2022-06-13 20:53     ` nil-admirari
  1 sibling, 2 replies; 21+ messages in thread
From: Hendrik Leppkes @ 2022-06-13 18:55 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Mon, Jun 13, 2022 at 7:47 PM Soft Works <softworkz@hotmail.com> wrote:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> > Admirari
> > Sent: Monday, June 13, 2022 6:26 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH
> > limit and use UTF-8 version of getenv()
> >
> > 1. getenv() is replaced with getenv_utf8() across libavformat.
> > 2. New versions of AviSynth+ are now called with UTF-8 filenames.
> > 3. Old versions of AviSynth are still using ANSI strings,
> >    but MAX_PATH limit on filename is removed.
> > ---
> >  libavformat/avisynth.c    | 39 +++++++++++++++++++++++++++----------
> > --
> >  libavformat/http.c        | 20 +++++++++++++-------
> >  libavformat/ipfsgateway.c | 35 +++++++++++++++++++++++------------
> >  libavformat/tls.c         | 11 +++++++++--
> >  4 files changed, 72 insertions(+), 33 deletions(-)
> >
> > diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> > index 8ba2bdead2..a97d12b6b6 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
> > @@ -56,6 +57,7 @@ typedef struct AviSynthLibrary {
> >  #define AVSC_DECLARE_FUNC(name) name ## _func name
> >      AVSC_DECLARE_FUNC(avs_bit_blt);
> >      AVSC_DECLARE_FUNC(avs_clip_get_error);
> > +    AVSC_DECLARE_FUNC(avs_check_version);
> >      AVSC_DECLARE_FUNC(avs_create_script_environment);
> >      AVSC_DECLARE_FUNC(avs_delete_script_environment);
> >      AVSC_DECLARE_FUNC(avs_get_audio);
> > @@ -137,6 +139,7 @@ static av_cold int avisynth_load_library(void)
> >
> >      LOAD_AVS_FUNC(avs_bit_blt, 0);
> >      LOAD_AVS_FUNC(avs_clip_get_error, 0);
> > +    LOAD_AVS_FUNC(avs_check_version, 0);
> >      LOAD_AVS_FUNC(avs_create_script_environment, 0);
> >      LOAD_AVS_FUNC(avs_delete_script_environment, 0);
> >      LOAD_AVS_FUNC(avs_get_audio, 0);
> > @@ -807,26 +810,38 @@ static int
> > avisynth_create_stream(AVFormatContext *s)
> >  static int avisynth_open_file(AVFormatContext *s)
> >  {
> >      AviSynthContext *avs = s->priv_data;
> > -    AVS_Value arg, val;
> > +    AVS_Value val;
> >      int ret;
> > -#ifdef _WIN32
> > -    char filename_ansi[MAX_PATH * 4];
> > -    wchar_t filename_wc[MAX_PATH * 4];
> > -#endif
> >
> >      if (ret = avisynth_context_create(s))
> >          return ret;
> >
> > +    if (!avs_library.avs_check_version(avs->env, 7)) {
>
> I like the version check. I don't know about all the derivatives
> of AviSynth, but I assume you have checked that it's valid for
> the common ones (or at least the original non-Plus variant)?
>
> > +        AVS_Value args[] = {
> > +            avs_new_value_string(s->url),
> > +            avs_new_value_bool(1) // filename is in UTF-8
> > +        };
> > +        val = avs_library.avs_invoke(avs->env, "Import",
> > +                                     avs_new_value_array(args, 2),
> > 0);
> > +    } else {
> > +        AVS_Value arg;
> >  #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);
> > -    arg = avs_new_value_string(filename_ansi);
> > +        char *filename_ansi;
> > +        /* Convert UTF-8 to ANSI code page */
> > +        if (utf8toansi(s->url, &filename_ansi)) {
>
> Two ideas came to my mind how this could be done better.
> What's actually needed here is not a string conversion, we need
> a valid and usable filename, and the function could be more
> something like "get_ansi_filename()".
>
> The first thing that this function could do is to convert the
> the filename to ANSI and right back to UTF-8, then compare the
> UTF-8 result with the original UTF-8 string. When both are equal,
> we know that the conversion is safe, otherwise we know that it
> won't work.
>
> Then, we can use the win32 API GetShortFileName(). Which returns
> file and directory names in 8.3 notation which (IIRC) contains
> only letters which are valid in the ANSI code page.
>

This seems unrelated to this patch, which is about removing the
MAX_PATH limit. The code previously converted UTF-8 to ANSI, and still
does so now, just without the MAX_PATH limit.
Further improvements tangential to this topic can, and should, be
applied independently, and not hold up this patch in discussion-hell
for longer than necessary.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 18:55     ` Hendrik Leppkes
@ 2022-06-13 19:00       ` Soft Works
  2022-06-13 19:07       ` Stephen Hutchinson
  1 sibling, 0 replies; 21+ messages in thread
From: Soft Works @ 2022-06-13 19:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Hendrik Leppkes
> Sent: Monday, June 13, 2022 8:55 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove
> MAX_PATH limit and use UTF-8 version of getenv()
> 
> On Mon, Jun 13, 2022 at 7:47 PM Soft Works <softworkz@hotmail.com>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nil
> > > Admirari
> > > Sent: Monday, June 13, 2022 6:26 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove
> MAX_PATH
> > > limit and use UTF-8 version of getenv()
> > >
> > > 1. getenv() is replaced with getenv_utf8() across libavformat.
> > > 2. New versions of AviSynth+ are now called with UTF-8 filenames.
> > > 3. Old versions of AviSynth are still using ANSI strings,
> > >    but MAX_PATH limit on filename is removed.
> > > ---
> > >  libavformat/avisynth.c    | 39 +++++++++++++++++++++++++++------
> ----
> > > --
> > >  libavformat/http.c        | 20 +++++++++++++-------
> > >  libavformat/ipfsgateway.c | 35 +++++++++++++++++++++++----------
> --
> > >  libavformat/tls.c         | 11 +++++++++--
> > >  4 files changed, 72 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> > > index 8ba2bdead2..a97d12b6b6 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
> > > @@ -56,6 +57,7 @@ typedef struct AviSynthLibrary {
> > >  #define AVSC_DECLARE_FUNC(name) name ## _func name
> > >      AVSC_DECLARE_FUNC(avs_bit_blt);
> > >      AVSC_DECLARE_FUNC(avs_clip_get_error);
> > > +    AVSC_DECLARE_FUNC(avs_check_version);
> > >      AVSC_DECLARE_FUNC(avs_create_script_environment);
> > >      AVSC_DECLARE_FUNC(avs_delete_script_environment);
> > >      AVSC_DECLARE_FUNC(avs_get_audio);
> > > @@ -137,6 +139,7 @@ static av_cold int
> avisynth_load_library(void)
> > >
> > >      LOAD_AVS_FUNC(avs_bit_blt, 0);
> > >      LOAD_AVS_FUNC(avs_clip_get_error, 0);
> > > +    LOAD_AVS_FUNC(avs_check_version, 0);
> > >      LOAD_AVS_FUNC(avs_create_script_environment, 0);
> > >      LOAD_AVS_FUNC(avs_delete_script_environment, 0);
> > >      LOAD_AVS_FUNC(avs_get_audio, 0);
> > > @@ -807,26 +810,38 @@ static int
> > > avisynth_create_stream(AVFormatContext *s)
> > >  static int avisynth_open_file(AVFormatContext *s)
> > >  {
> > >      AviSynthContext *avs = s->priv_data;
> > > -    AVS_Value arg, val;
> > > +    AVS_Value val;
> > >      int ret;
> > > -#ifdef _WIN32
> > > -    char filename_ansi[MAX_PATH * 4];
> > > -    wchar_t filename_wc[MAX_PATH * 4];
> > > -#endif
> > >
> > >      if (ret = avisynth_context_create(s))
> > >          return ret;
> > >
> > > +    if (!avs_library.avs_check_version(avs->env, 7)) {
> >
> > I like the version check. I don't know about all the derivatives
> > of AviSynth, but I assume you have checked that it's valid for
> > the common ones (or at least the original non-Plus variant)?
> >
> > > +        AVS_Value args[] = {
> > > +            avs_new_value_string(s->url),
> > > +            avs_new_value_bool(1) // filename is in UTF-8
> > > +        };
> > > +        val = avs_library.avs_invoke(avs->env, "Import",
> > > +                                     avs_new_value_array(args,
> 2),
> > > 0);
> > > +    } else {
> > > +        AVS_Value arg;
> > >  #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);
> > > -    arg = avs_new_value_string(filename_ansi);
> > > +        char *filename_ansi;
> > > +        /* Convert UTF-8 to ANSI code page */
> > > +        if (utf8toansi(s->url, &filename_ansi)) {
> >
> > Two ideas came to my mind how this could be done better.
> > What's actually needed here is not a string conversion, we need
> > a valid and usable filename, and the function could be more
> > something like "get_ansi_filename()".
> >
> > The first thing that this function could do is to convert the
> > the filename to ANSI and right back to UTF-8, then compare the
> > UTF-8 result with the original UTF-8 string. When both are equal,
> > we know that the conversion is safe, otherwise we know that it
> > won't work.
> >
> > Then, we can use the win32 API GetShortFileName(). Which returns
> > file and directory names in 8.3 notation which (IIRC) contains
> > only letters which are valid in the ANSI code page.
> >
> 
> This seems unrelated to this patch, which is about removing the
> MAX_PATH limit. The code previously converted UTF-8 to ANSI, and
> still
> does so now, just without the MAX_PATH limit.
> Further improvements tangential to this topic can, and should, be
> applied independently, and not hold up this patch in discussion-hell
> for longer than necessary.

It was meant as a suggestion not as an objection. I'm fine with this
patch, just to be clear.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-13 17:03     ` nil-admirari
@ 2022-06-13 19:02       ` Soft Works
  0 siblings, 0 replies; 21+ messages in thread
From: Soft Works @ 2022-06-13 19:02 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> nil-admirari@mailo.com
> Sent: Monday, June 13, 2022 7:03 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove
> MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
> 
> > Why do you use a 'do' loop? Can't you use the normal 2-step
> > approach, i.e. call the winapi function with a NULL buffer,
> > and then use the returned size to allocate the buffer.
> > This way you always need a single allocation only.
> 
> GetModuleFileNameW does not follow this convention:
> https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-
> libloaderapi-getmodulefilenamew
> 
> > If the length of the path exceeds the size that the nSize parameter
> specifies,
> > the function succeeds and the string is truncated to nSize
> characters
> > including the terminating null character.
> 
> MS does the looping too in their WIL library:
> https://github.com/microsoft/wil/blob/master/include/wil/win32_helper
> s.h#L339-L341.

Yes, you're right in this case; hadn't looked it up.

Thanks for the pointers.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 18:55     ` Hendrik Leppkes
  2022-06-13 19:00       ` Soft Works
@ 2022-06-13 19:07       ` Stephen Hutchinson
  1 sibling, 0 replies; 21+ messages in thread
From: Stephen Hutchinson @ 2022-06-13 19:07 UTC (permalink / raw)
  To: ffmpeg-devel

On 6/13/22 2:55 PM, Hendrik Leppkes wrote:
> This seems unrelated to this patch, which is about removing the
> MAX_PATH limit. The code previously converted UTF-8 to ANSI, and still
> does so now, just without the MAX_PATH limit.
> Further improvements tangential to this topic can, and should, be
> applied independently, and not hold up this patch in discussion-hell
> for longer than necessary.
> 

Agreed.  As is stands, if a user finds that they need to open files that 
use non-ANSI characters in their filenames, they can always go into 
their Language settings and turn on UTF-8 for worldwide language 
support, which was just as true before the MAX_PATH-related patches. 
Honestly, Microsoft just needs to stop delaying it and make UTF-8 the 
default, and then we won't have this problem anymore.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
  2022-06-13 16:39   ` Soft Works
@ 2022-06-13 20:17   ` Soft Works
  2022-06-15 20:00     ` nil-admirari
  1 sibling, 1 reply; 21+ messages in thread
From: Soft Works @ 2022-06-13 20:17 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nil
> Admirari
> Sent: Monday, June 13, 2022 6:26 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove
> MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
> 
> ---
>  compat/w32dlfcn.h     | 80 ++++++++++++++++++++++++++++++++++-------
> --
>  libavcodec/mf_utils.h |  1 +
>  2 files changed, 65 insertions(+), 16 deletions(-)
> 
> diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
> index 52a94efafb..e49d3841aa 100644
> --- a/compat/w32dlfcn.h
> +++ b/compat/w32dlfcn.h
> @@ -22,9 +22,31 @@
>  #ifdef _WIN32
>  #include <windows.h>
>  #include "config.h"
> -#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
>  #include "libavutil/wchar_filename.h"
> -#endif
> +
> +static inline wchar_t *get_module_filename(HMODULE module)
> +{
> +    wchar_t *path = NULL, *new_path;
> +    DWORD path_size = 0, path_len;
> +
> +    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);

 path_size <= INT16_MAX

(the edge case is already covered by the equals)

> +
> +    if (!path_len) {
> +        av_free(path);
> +        return NULL;
> +    }
> +    return path;
> +}
> +
>  /**
>   * Safe function used to open dynamic libs. This attempts to improve
> program security
>   * by removing the current directory from the dll search path. Only
> dll's found in the
> @@ -34,29 +56,53 @@
>   */
>  static inline HMODULE win32_dlopen(const char *name)
>  {
> +    wchar_t *name_w = NULL;
> +    if (utf8towchar(name, &name_w))
> +        name_w = NULL;
>  #if _WIN32_WINNT < 0x0602
>      // Need to check if KB2533623 is available

I know this line existed before your path, but it would be nice
to clarify check and condition, like:

// On Win7 an earlier we 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;
> +        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 the 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 the case #2.
> +                pathlen = GetSystemDirectoryW(path, pathsize);
> +                if (!pathlen)
> +                    goto exit;
> +            }
> +            path[pathlen] = L'\\';
>              wcscpy(path + pathlen + 1, name_w);
>              module = LoadLibraryExW(path, NULL,
> LOAD_WITH_ALTERED_SEARCH_PATH);
>          }
> @@ -73,15 +119,17 @@ exit:
>  #   define LOAD_LIBRARY_SEARCH_SYSTEM32        0x00000800
>  #endif
>  #if HAVE_WINRT
> -    wchar_t *name_w = NULL;
>      int ret;
> -    if (utf8towchar(name, &name_w))
> +    if (!name_w)
>          return NULL;
>      ret = LoadPackagedLibrary(name_w, 0);
>      av_free(name_w);
>      return ret;
>  #else
> -    return LoadLibraryExA(name, NULL,
> LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
> +    /* filename may be be in CP_ACP */
> +    if (!name_w)
> +        return LoadLibraryExA(name, NULL,
> LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
> +    return LoadLibraryExW(name_w, NULL,
> LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);

name_w leaks here

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-13 16:26 [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
                   ` (3 preceding siblings ...)
  2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 5/5] libavfilter/vf_frei0r.c: Use " Nil Admirari
@ 2022-06-13 20:50 ` Martin Storsjö
  2022-06-15 20:06   ` nil-admirari
  4 siblings, 1 reply; 21+ messages in thread
From: Martin Storsjö @ 2022-06-13 20:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Mon, 13 Jun 2022, Nil Admirari wrote:

> wchartoutf8() converts strings returned by WinAPI into UTF-8,
> which is FFmpeg's preffered encoding.
>
> Some external dependencies, such as AviSynth, are still
> not Unicode-enabled. utf8toansi() converts UTF-8 strings
> into ANSI in two steps: UTF-8 -> wchar_t -> ANSI.
> wchartoansi() is responsible for the second step of the conversion.
> Conversion in just one step is not supported by WinAPI.
>
> Since these character converting functions allocate the buffer
> of necessary size, they also facilitate the removal of MAX_PATH limit
> in places where fixed-size ANSI/WCHAR strings were used
> as filename buffers.
>
> getenv_utf8() wraps _wgetenv() converting its input from
> and its output to UTF-8. Compared to plain getenv(),
> getenv_utf8() requires a cleanup.
>
> Because of that, in places that only test the existence of
> an environment variable or compare its value with a string
> consisting entirely of ASCII characters, the use of plain getenv()
> is still preferred. (libavutil/log.c check_color_terminal()
> is an example of such a place.)
>
> Plain getenv() is also preffered in UNIX-only code,
> such as bktr.c, fbdev_common.c, oss.c in libavdevice
> or af_ladspa.c in libavfilter.
> ---
> libavutil/getenv_utf8.h    | 63 ++++++++++++++++++++++++++++++++++++++
> libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++
> 2 files changed, 114 insertions(+)
> create mode 100644 libavutil/getenv_utf8.h
>
> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
> new file mode 100644
> index 0000000000..2c48a36355
> --- /dev/null
> +++ b/libavutil/getenv_utf8.h
> @@ -0,0 +1,63 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVUTIL_GETENV_UTF8_H
> +#define AVUTIL_GETENV_UTF8_H
> +
> +#include <stdlib.h>
> +
> +#include "mem.h"
> +
> +#ifdef _WIN32
> +
> +#include "libavutil/wchar_filename.h"
> +
> +static inline char *getenv_utf8(const char *varname)
> +{
> +    wchar_t *varname_w, *var_w;
> +    char *var;
> +
> +    if (utf8towchar(varname, &varname_w))
> +        return NULL;
> +    if (!varname_w)
> +        return NULL;
> +
> +    var_w = _wgetenv(varname_w);
> +    av_free(varname_w);
> +
> +    if (!var_w)
> +        return NULL;
> +    if (wchartoutf8(var_w, &var))
> +        return NULL;
> +
> +    return var;
> +
> +    // No CP_ACP fallback compared to other *_utf8() functions:
> +    // non UTF-8 strings must not be returned.
> +}

Note, there are Windows configurations that entirely lack getenv() (and 
presumably _wgetenv() too), namely the Windows Store App / Windows Phone 
App API subsets for Windows 8.x. I think those functions were allowed 
again in Windows 10 though...

In configure, we have this:

check_func_headers stdlib.h getenv

enabled getenv || echo "#define getenv(x) NULL" >> $TMPH

I guess we'd might have to add getenv to e.g. the SYSTEM_FUNCS list, so 
we'd get a HAVE_GETENV in config.h - then we could make getenv_utf8 a 
no-op if HAVE_GETENV is 0.

(I'm not in a situation where I can test and investigate properly right 
now, I can maybe look closer into it later.)

// Martin


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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 17:47   ` Soft Works
  2022-06-13 18:55     ` Hendrik Leppkes
@ 2022-06-13 20:53     ` nil-admirari
  2022-06-13 21:52       ` Soft Works
  1 sibling, 1 reply; 21+ messages in thread
From: nil-admirari @ 2022-06-13 20:53 UTC (permalink / raw)
  To: ffmpeg-devel

> I like the version check. I don't know about all the derivatives
> of AviSynth, but I assume you have checked that it's valid for
> the common ones (or at least the original non-Plus variant)?

Interface version was changed to 7 in 2020:
https://github.com/AviSynth/AviSynthPlus/commit/40900dc1c54c14ea9f188c7242b88d464d067a44
three years after utf8 was implemented. If I'm not mistaken, there is no way to check
for a particular revision.

> Two ideas came to my mind how this could be done better.
> What's actually needed here is not a string conversion, we need
> a valid and usable filename, and the function could be more
> something like "get_ansi_filename()".
>
> The first thing that this function could do is to convert the
> the filename to ANSI and right back to UTF-8, then compare the
> UTF-8 result with the original UTF-8 string. When both are equal,
> we know that the conversion is safe, otherwise we know that it
> won't work.
>
> Then, we can use the win32 API GetShortFileName(). Which returns
> file and directory names in 8.3 notation which (IIRC) contains
> only letters which are valid in the ANSI code page.
>
> 8.3 file names do not always exist (depending on system config), 
> but it's always worth trying.
>
> Should both of these procedures fail, we could at least output
> a useful message, explaining why it doesn't work.
>
> Let me know what you think.

Too much work for something that was fixed on AviSynth+ side two years ago.



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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 20:53     ` nil-admirari
@ 2022-06-13 21:52       ` Soft Works
  2022-06-13 22:01         ` Soft Works
  0 siblings, 1 reply; 21+ messages in thread
From: Soft Works @ 2022-06-13 21:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> nil-admirari@mailo.com
> Sent: Monday, June 13, 2022 10:53 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove
> MAX_PATH limit and use UTF-8 version of getenv()
> 
> > I like the version check. I don't know about all the derivatives
> > of AviSynth, but I assume you have checked that it's valid for
> > the common ones (or at least the original non-Plus variant)?
> 
> Interface version was changed to 7 in 2020:
> https://github.com/AviSynth/AviSynthPlus/commit/40900dc1c54c14ea9f188
> c7242b88d464d067a44
> three years after utf8 was implemented. If I'm not mistaken, there is
> no way to check
> for a particular revision.
> 
> > Two ideas came to my mind how this could be done better.
> > What's actually needed here is not a string conversion, we need
> > a valid and usable filename, and the function could be more
> > something like "get_ansi_filename()".
> >
> > The first thing that this function could do is to convert the
> > the filename to ANSI and right back to UTF-8, then compare the
> > UTF-8 result with the original UTF-8 string. When both are equal,
> > we know that the conversion is safe, otherwise we know that it
> > won't work.
> >
> > Then, we can use the win32 API GetShortFileName(). Which returns
> > file and directory names in 8.3 notation which (IIRC) contains
> > only letters which are valid in the ANSI code page.
> >
> > 8.3 file names do not always exist (depending on system config),
> > but it's always worth trying.
> >
> > Should both of these procedures fail, we could at least output
> > a useful message, explaining why it doesn't work.
> >
> > Let me know what you think.
> 
> Too much work for something that was fixed on AviSynth+ side two
> years ago.

I wasn't sure how important AviSynth is for you. Until I had given
the hint at the UTF8 option in AviSynth, it seemed to be a high
priority to enable the use of long paths with AviSynth, and what
I was thinking is that this might be of interest because paths 
with non-ANSI chars are a way more frequent case than long paths
(on Windows).

Anyway, as long as people are using a non-ancient version of 
AviSynthPlus, it's all covered by your patch, and I'm totally
fine with this part!

Thanks,
sw




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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-13 21:52       ` Soft Works
@ 2022-06-13 22:01         ` Soft Works
  0 siblings, 0 replies; 21+ messages in thread
From: Soft Works @ 2022-06-13 22:01 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Monday, June 13, 2022 11:53 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove
> MAX_PATH limit and use UTF-8 version of getenv()
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > nil-admirari@mailo.com
> > Sent: Monday, June 13, 2022 10:53 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove
> > MAX_PATH limit and use UTF-8 version of getenv()
> >
> > > I like the version check. I don't know about all the derivatives
> > > of AviSynth, but I assume you have checked that it's valid for
> > > the common ones (or at least the original non-Plus variant)?
> >
> > Interface version was changed to 7 in 2020:
> >
> https://github.com/AviSynth/AviSynthPlus/commit/40900dc1c54c14ea9f188
> > c7242b88d464d067a44
> > three years after utf8 was implemented. If I'm not mistaken, there
> is
> > no way to check
> > for a particular revision.
> >
> > > Two ideas came to my mind how this could be done better.
> > > What's actually needed here is not a string conversion, we need
> > > a valid and usable filename, and the function could be more
> > > something like "get_ansi_filename()".
> > >
> > > The first thing that this function could do is to convert the
> > > the filename to ANSI and right back to UTF-8, then compare the
> > > UTF-8 result with the original UTF-8 string. When both are equal,
> > > we know that the conversion is safe, otherwise we know that it
> > > won't work.
> > >
> > > Then, we can use the win32 API GetShortFileName(). Which returns
> > > file and directory names in 8.3 notation which (IIRC) contains
> > > only letters which are valid in the ANSI code page.
> > >
> > > 8.3 file names do not always exist (depending on system config),
> > > but it's always worth trying.
> > >
> > > Should both of these procedures fail, we could at least output
> > > a useful message, explaining why it doesn't work.
> > >
> > > Let me know what you think.
> >
> > Too much work for something that was fixed on AviSynth+ side two
> > years ago.
> 
> I wasn't sure how important AviSynth is for you. Until I had given
> the hint at the UTF8 option in AviSynth, it seemed to be a high
> priority to enable the use of long paths with AviSynth, and what
> I was thinking is that this might be of interest because paths
> with non-ANSI chars are a way more frequent case than long paths
> (on Windows).

Before somebody will ask how I come to make such claims, here's 
why: 

As of Win 11, the Explorer still warns or even prevents you from 
creating long paths, but you are free to use any kind of non-ANSI
characters for file and folders names. This does not only apply to 
the file management application, but includes the file open and save
dialogs of all applications that are using the standard dialogs.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-13 20:17   ` Soft Works
@ 2022-06-15 20:00     ` nil-admirari
  2022-06-16  0:00       ` Soft Works
  0 siblings, 1 reply; 21+ messages in thread
From: nil-admirari @ 2022-06-15 20:00 UTC (permalink / raw)
  To: ffmpeg-devel

> path_size <= INT16_MAX
>
> (the edge case is already covered by the equals)

Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297590.html.
Don't quite understand what edge case you've meant:
INT16_MAX is 32767, which is the maximal path length allowed,
+ 1 is needed for the terminating null.

> I know this line existed before your path, but it would be nice
> to clarify check and condition, like:
>
> // On Win7 an earlier we check if KB2533623 is available

Changed the comment.

> name_w leaks here

Fixed.



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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-13 20:50 ` [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Martin Storsjö
@ 2022-06-15 20:06   ` nil-admirari
  0 siblings, 0 replies; 21+ messages in thread
From: nil-admirari @ 2022-06-15 20:06 UTC (permalink / raw)
  To: ffmpeg-devel

> I guess we'd might have to add getenv to e.g. the SYSTEM_FUNCS list, so 
> we'd get a HAVE_GETENV in config.h - then we could make getenv_utf8 a 
> no-op if HAVE_GETENV is 0.

Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297596.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".

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-15 20:00     ` nil-admirari
@ 2022-06-16  0:00       ` Soft Works
  2022-06-17  9:33         ` nil-admirari
  0 siblings, 1 reply; 21+ messages in thread
From: Soft Works @ 2022-06-16  0:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> nil-admirari@mailo.com
> Sent: Wednesday, June 15, 2022 10:00 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove
> MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
> 
> > path_size <= INT16_MAX
> >
> > (the edge case is already covered by the equals)
> 
> Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-
> June/297590.html.
> Don't quite understand what edge case you've meant:
> INT16_MAX is 32767, which is the maximal path length allowed,
> + 1 is needed for the terminating null.

With the +1 your while condition term is effectively

     path_size <= 32768

But when the path_size is 32768, you do not need to
go for another loop with an increased buffer because this is
already as large as it can get. There won't be any 32769
or 32770 (...) cases, I think.

> > I know this line existed before your path, but it would be nice
> > to clarify check and condition, like:
> >
> > // On Win7 an earlier we check if KB2533623 is available
> 
> Changed the comment.

Cool. Thanks.

softworkz

> 
> > name_w leaks here
> 
> Fixed.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
  2022-06-16  0:00       ` Soft Works
@ 2022-06-17  9:33         ` nil-admirari
  0 siblings, 0 replies; 21+ messages in thread
From: nil-admirari @ 2022-06-17  9:33 UTC (permalink / raw)
  To: ffmpeg-devel

> With the +1 your while condition term is effectively
>
> path_size <= 32768
>
> But when the path_size is 32768, you do not need to
> go for another loop with an increased buffer because this is
> already as large as it can get. There won't be any 32769
> or 32770 (...) cases, I think.

Removed +1: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297688.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".

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-06-17  9:34 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 16:26 [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
2022-06-13 16:39   ` Soft Works
2022-06-13 17:03     ` nil-admirari
2022-06-13 19:02       ` Soft Works
2022-06-13 20:17   ` Soft Works
2022-06-15 20:00     ` nil-admirari
2022-06-16  0:00       ` Soft Works
2022-06-17  9:33         ` nil-admirari
2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv() Nil Admirari
2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
2022-06-13 17:47   ` Soft Works
2022-06-13 18:55     ` Hendrik Leppkes
2022-06-13 19:00       ` Soft Works
2022-06-13 19:07       ` Stephen Hutchinson
2022-06-13 20:53     ` nil-admirari
2022-06-13 21:52       ` Soft Works
2022-06-13 22:01         ` Soft Works
2022-06-13 16:26 ` [FFmpeg-devel] [PATCH v14 5/5] libavfilter/vf_frei0r.c: Use " Nil Admirari
2022-06-13 20:50 ` [FFmpeg-devel] [PATCH v14 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Martin Storsjö
2022-06-15 20:06   ` nil-admirari

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git