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 v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
@ 2022-06-17  9:31 Nil Admirari
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Nil Admirari @ 2022-06-17  9:31 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.
---
 configure                  |  1 +
 libavutil/getenv_utf8.h    | 71 ++++++++++++++++++++++++++++++++++++++
 libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)
 create mode 100644 libavutil/getenv_utf8.h

diff --git a/configure b/configure
index 3dca1c4bd3..fa37a74531 100755
--- a/configure
+++ b/configure
@@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
     fcntl
     getaddrinfo
     getauxval
+    getenv
     gethrtime
     getopt
     GetModuleHandle
diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
new file mode 100644
index 0000000000..161e3e6202
--- /dev/null
+++ b/libavutil/getenv_utf8.h
@@ -0,0 +1,71 @@
+/*
+ * 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 HAVE_GETENV
+
+#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
+
+#else
+
+#define getenv_utf8(x) NULL
+
+#endif // HAVE_GETENV
+
+#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] 25+ messages in thread

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

---
 compat/w32dlfcn.h     | 100 ++++++++++++++++++++++++++++++++----------
 libavcodec/mf_utils.h |   1 +
 2 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
index 52a94efafb..fb1aa1b72e 100644
--- a/compat/w32dlfcn.h
+++ b/compat/w32dlfcn.h
@@ -20,11 +20,40 @@
 #define COMPAT_W32DLFCN_H
 
 #ifdef _WIN32
+#include <stdint.h>
+
 #include <windows.h>
+
 #include "config.h"
-#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
+#include "libavutil/macros.h"
 #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 ? FFMIN(2 * path_size, INT16_MAX + 1) : MAX_PATH;
+        new_path = av_realloc_array(path, path_size, sizeof *path);
+        if (!new_path) {
+            av_free(path);
+            return NULL;
+        }
+        path = new_path;
+        // Returns path_size in case of insufficient buffer.
+        // Whether the error is set or not and whether the output
+        // is null-terminated or not depends on the version of Windows.
+        path_len = GetModuleFileNameW(module, path, path_size);
+    } while (path_len && path_size <= INT16_MAX && 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 +63,53 @@
  */
 static inline HMODULE win32_dlopen(const char *name)
 {
+    wchar_t *name_w;
+    HMODULE module = NULL;
+    if (utf8towchar(name, &name_w))
+        name_w = NULL;
 #if _WIN32_WINNT < 0x0602
-    // Need to check if KB2533623 is available
+    // On Win7 and 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;
+        new_path = wcsrchr(path, '\\');
+        if (!new_path)
             goto exit;
-        path[pathlen] = '\\';
+        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,16 +126,19 @@ 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;
+    module = LoadPackagedLibrary(name_w, 0);
 #else
-    return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
+#define LOAD_FLAGS (LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32)
+    /* filename may be be in CP_ACP */
+    if (!name_w)
+        return LoadLibraryExA(name, NULL, LOAD_FLAGS);
+    module = LoadLibraryExW(name_w, NULL, LOAD_FLAGS);
+#undef LOAD_FLAGS
 #endif
+    av_free(name_w);
+    return module;
 }
 #define dlopen(name, flags) win32_dlopen(name)
 #define dlclose FreeLibrary
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] 25+ messages in thread

* [FFmpeg-devel] [PATCH v17 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv()
  2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
@ 2022-06-17  9:31 ` Nil Admirari
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Nil Admirari @ 2022-06-17  9:31 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] 25+ messages in thread

* [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv() Nil Admirari
@ 2022-06-17  9:31 ` Nil Admirari
  2022-06-18 22:24   ` Martin Storsjö
  2022-06-18 22:40   ` Martin Storsjö
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 5/5] libavfilter/vf_frei0r.c: Use " Nil Admirari
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 25+ messages in thread
From: Nil Admirari @ 2022-06-17  9:31 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] 25+ messages in thread

* [FFmpeg-devel] [PATCH v17 5/5] libavfilter/vf_frei0r.c: Use UTF-8 version of getenv()
  2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
                   ` (2 preceding siblings ...)
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
@ 2022-06-17  9:31 ` Nil Admirari
  2022-06-17 19:16 ` [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Soft Works
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Nil Admirari @ 2022-06-17  9:31 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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
                   ` (3 preceding siblings ...)
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 5/5] libavfilter/vf_frei0r.c: Use " Nil Admirari
@ 2022-06-17 19:16 ` Soft Works
  2022-06-18 22:21 ` Martin Storsjö
  2022-06-19  4:58 ` Andreas Rheinhardt
  6 siblings, 0 replies; 25+ messages in thread
From: Soft Works @ 2022-06-17 19:16 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: Friday, June 17, 2022 11:32 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(),
> wchartoansi(), utf8toansi() and getenv_utf8()
> 
> 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.
> ---
>  configure                  |  1 +
>  libavutil/getenv_utf8.h    | 71
> ++++++++++++++++++++++++++++++++++++++
>  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+)
>  create mode 100644 libavutil/getenv_utf8.h
> 
> diff --git a/configure b/configure
> index 3dca1c4bd3..fa37a74531 100755
> --- a/configure
> +++ b/configure
> @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
>      fcntl
>      getaddrinfo
>      getauxval
> +    getenv
>      gethrtime
>      getopt
>      GetModuleHandle
> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
> new file mode 100644
> index 0000000000..161e3e6202
> --- /dev/null
> +++ b/libavutil/getenv_utf8.h
> @@ -0,0 +1,71 @@
> +/*
> + * 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 HAVE_GETENV
> +
> +#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
> +
> +#else
> +
> +#define getenv_utf8(x) NULL
> +
> +#endif // HAVE_GETENV
> +
> +#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()
> --


LGTM for the whole patchset. I didn't look at the getenv part, but I think
Martin did (or will do).

Thanks for all your effort (and patience)!

Best wishes,
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
                   ` (4 preceding siblings ...)
  2022-06-17 19:16 ` [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Soft Works
@ 2022-06-18 22:21 ` Martin Storsjö
  2022-06-19 11:49   ` nil-admirari
  2022-06-19  4:58 ` Andreas Rheinhardt
  6 siblings, 1 reply; 25+ messages in thread
From: Martin Storsjö @ 2022-06-18 22:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Fri, 17 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.
> ---
> configure                  |  1 +
> libavutil/getenv_utf8.h    | 71 ++++++++++++++++++++++++++++++++++++++
> libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
> 3 files changed, 123 insertions(+)
> create mode 100644 libavutil/getenv_utf8.h

This looks generally good - as others seem to be ok with this and there 
doesn't seem to be any more objections, I can push this in a while. (I'm 
not familiar with the avisynth bits though, but it seems like there's 
agreement about it.)

> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
> new file mode 100644
> index 0000000000..161e3e6202
> --- /dev/null
> +++ b/libavutil/getenv_utf8.h
> @@ -0,0 +1,71 @@
> +#ifndef AVUTIL_GETENV_UTF8_H
> +#define AVUTIL_GETENV_UTF8_H
> +
> +#include <stdlib.h>
> +
> +#include "mem.h"
> +
> +#ifdef HAVE_GETENV

Note that this should be #if HAVE_GETENV - these constants are always 
defined and evaluate to 0 or 1. No need to resend the patchset just for 
that. (I added an explicit #include "config.h" above here too, just to 
make it clearer.)

// 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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
@ 2022-06-18 22:24   ` Martin Storsjö
  2022-06-18 22:36     ` Martin Storsjö
  2022-06-18 22:48     ` Soft Works
  2022-06-18 22:40   ` Martin Storsjö
  1 sibling, 2 replies; 25+ messages in thread
From: Martin Storsjö @ 2022-06-18 22:24 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Fri, 17 Jun 2022, Nil Admirari wrote:

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

This actually causes some surprise breakage in MSVC builds. Here, 
getenv_utf8.h includes windows.h. If including windows.h and winsock2 
headers in the same file, the winsock2 headers must be included before.

I fixed it locally by moving this new include down, with a comment like 
this:

/* This header can include <windows.h>. That header has to be included after
  * winsock2 headers (included by network.h and os_support.h above). */
#include "libavutil/getenv_utf8.h"

(I had to do the same fixup in http.c and ipfsgateway.c.)

If that seems fine to you and others, I can push this patchset maybe 
tomorrow.

// 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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-18 22:24   ` Martin Storsjö
@ 2022-06-18 22:36     ` Martin Storsjö
  2022-06-18 22:48     ` Soft Works
  1 sibling, 0 replies; 25+ messages in thread
From: Martin Storsjö @ 2022-06-18 22:36 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Sun, 19 Jun 2022, Martin Storsjö wrote:

> On Fri, 17 Jun 2022, Nil Admirari wrote:
>
>> 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/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"
>
> This actually causes some surprise breakage in MSVC builds. Here, 
> getenv_utf8.h includes windows.h. If including windows.h and winsock2 headers 
> in the same file, the winsock2 headers must be included before.
>
> I fixed it locally by moving this new include down, with a comment like this:
>
> /* This header can include <windows.h>. That header has to be included after
> * winsock2 headers (included by network.h and os_support.h above). */
> #include "libavutil/getenv_utf8.h"
>
> (I had to do the same fixup in http.c and ipfsgateway.c.)

Alternatively, as these env variables mostly are used for host names to a 
proxy or similar, there's probably not much need for handling anything 
outside of ASCII anyway, so we could also consider just leaving these as 
plain getenv().

// 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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
  2022-06-18 22:24   ` Martin Storsjö
@ 2022-06-18 22:40   ` Martin Storsjö
  2022-06-19 11:47     ` nil-admirari
  1 sibling, 1 reply; 25+ messages in thread
From: Martin Storsjö @ 2022-06-18 22:40 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Fri, 17 Jun 2022, Nil Admirari wrote:

> 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(-)
>
> @@ -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);
> }

FWIW - I wasn't entirely sure we can conclude that we always pass through 
a case that initializes the err variable here, so just to be sure, I 
locally amended this patch to initialize the err variable to 0 too.

// Martin

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

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

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

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-18 22:24   ` Martin Storsjö
  2022-06-18 22:36     ` Martin Storsjö
@ 2022-06-18 22:48     ` Soft Works
  2022-06-19  7:49       ` Martin Storsjö
  1 sibling, 1 reply; 25+ messages in thread
From: Soft Works @ 2022-06-18 22:48 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Martin Storsjö
> Sent: Sunday, June 19, 2022 12:24 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove
> MAX_PATH limit and use UTF-8 version of getenv()
> 
> On Fri, 17 Jun 2022, Nil Admirari wrote:
> 
> > 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/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"
> 
> This actually causes some surprise breakage in MSVC builds. Here,
> getenv_utf8.h includes windows.h. If including windows.h and winsock2
> headers in the same file, the winsock2 headers must be included
> before.
> 
> I fixed it locally by moving this new include down, with a comment
> like
> this:
> 
> /* This header can include <windows.h>. That header has to be
> included after
>   * winsock2 headers (included by network.h and os_support.h above).
> */

This is the recommended way:


#define WIN32_LEAN_AND_MEAN

#include <windows.h>
#include <winsock2.h>


<Quote>
The Winsock2.h header file internally includes core elements from the Windows.h header file, so there is not usually an #include line for the Windows.h header file in Winsock applications. If an #include line is needed for the Windows.h header file, this should be preceded with the #define WIN32_LEAN_AND_MEAN macro. For historical reasons, the Windows.h header defaults to including the Winsock.h header file for Windows Sockets 1.1. The declarations in the Winsock.h header file will conflict with the declarations in the Winsock2.h header file required by Windows Sockets 2. The WIN32_LEAN_AND_MEAN macro prevents the Winsock.h from being included by the Windows.h header.
</Quote>

References:

https://docs.microsoft.com/en-us/windows/win32/winsock/include-files-2
https://docs.microsoft.com/en-us/windows/win32/winsock/creating-a-basic-winsock-application

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

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

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
                   ` (5 preceding siblings ...)
  2022-06-18 22:21 ` Martin Storsjö
@ 2022-06-19  4:58 ` Andreas Rheinhardt
  2022-06-19  5:56   ` Soft Works
  2022-06-19 11:56   ` nil-admirari
  6 siblings, 2 replies; 25+ messages in thread
From: Andreas Rheinhardt @ 2022-06-19  4:58 UTC (permalink / raw)
  To: ffmpeg-devel

Nil Admirari:
> 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.
> ---
>  configure                  |  1 +
>  libavutil/getenv_utf8.h    | 71 ++++++++++++++++++++++++++++++++++++++
>  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+)
>  create mode 100644 libavutil/getenv_utf8.h
> 
> diff --git a/configure b/configure
> index 3dca1c4bd3..fa37a74531 100755
> --- a/configure
> +++ b/configure
> @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
>      fcntl
>      getaddrinfo
>      getauxval
> +    getenv
>      gethrtime
>      getopt
>      GetModuleHandle
> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
> new file mode 100644
> index 0000000000..161e3e6202
> --- /dev/null
> +++ b/libavutil/getenv_utf8.h
> @@ -0,0 +1,71 @@
> +/*
> + * 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 HAVE_GETENV
> +
> +#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));

This forces allocations and frees in scenarios where this is wholly
unnecessary. This can be avoided by adding a custom deallocator for
strings returned via getenv_utf8: Namely a define/wrapper around av_free
in the _WIN32 and a no-op else.

> +}
> +
> +#endif // _WIN32
> +
> +#else
> +
> +#define getenv_utf8(x) NULL
> +
> +#endif // HAVE_GETENV
> +
> +#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()

_______________________________________________
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19  4:58 ` Andreas Rheinhardt
@ 2022-06-19  5:56   ` Soft Works
  2022-06-19  6:27     ` Andreas Rheinhardt
  2022-06-19  6:33     ` Martin Storsjö
  2022-06-19 11:56   ` nil-admirari
  1 sibling, 2 replies; 25+ messages in thread
From: Soft Works @ 2022-06-19  5:56 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Sunday, June 19, 2022 6:59 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
> wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
> 
> Nil Admirari:
> > 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.
> > ---
> >  configure                  |  1 +
> >  libavutil/getenv_utf8.h    | 71
> ++++++++++++++++++++++++++++++++++++++
> >  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
> >  3 files changed, 123 insertions(+)
> >  create mode 100644 libavutil/getenv_utf8.h
> >
> > diff --git a/configure b/configure
> > index 3dca1c4bd3..fa37a74531 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
> >      fcntl
> >      getaddrinfo
> >      getauxval
> > +    getenv
> >      gethrtime
> >      getopt
> >      GetModuleHandle
> > diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
> > new file mode 100644
> > index 0000000000..161e3e6202
> > --- /dev/null
> > +++ b/libavutil/getenv_utf8.h
> > @@ -0,0 +1,71 @@
> > +/*
> > + * 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 HAVE_GETENV
> > +
> > +#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));
> 
> This forces allocations and frees in scenarios where this is wholly
> unnecessary. 

Why do you think this is unnecessary? At least on Windows, there is
no guarantee regarding the lifetime of strings returned from
getenv(). In case when some other code would call _putenv to set the
env variable, this can cause the previously returned string to become 
invalid without the caller being able to know.


> This can be avoided by adding a custom deallocator for
> strings returned via getenv_utf8: Namely a define/wrapper around
> av_free in the _WIN32 and a no-op else.

I don't think I really understand what you mean, by the above 😉


Best regards,
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19  5:56   ` Soft Works
@ 2022-06-19  6:27     ` Andreas Rheinhardt
  2022-06-19  7:24       ` Soft Works
  2022-06-19  6:33     ` Martin Storsjö
  1 sibling, 1 reply; 25+ messages in thread
From: Andreas Rheinhardt @ 2022-06-19  6:27 UTC (permalink / raw)
  To: ffmpeg-devel

Soft Works:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Andreas Rheinhardt
>> Sent: Sunday, June 19, 2022 6:59 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
>> wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
>>
>> Nil Admirari:
>>> 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.
>>> ---
>>>  configure                  |  1 +
>>>  libavutil/getenv_utf8.h    | 71
>> ++++++++++++++++++++++++++++++++++++++
>>>  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
>>>  3 files changed, 123 insertions(+)
>>>  create mode 100644 libavutil/getenv_utf8.h
>>>
>>> diff --git a/configure b/configure
>>> index 3dca1c4bd3..fa37a74531 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
>>>      fcntl
>>>      getaddrinfo
>>>      getauxval
>>> +    getenv
>>>      gethrtime
>>>      getopt
>>>      GetModuleHandle
>>> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
>>> new file mode 100644
>>> index 0000000000..161e3e6202
>>> --- /dev/null
>>> +++ b/libavutil/getenv_utf8.h
>>> @@ -0,0 +1,71 @@
>>> +/*
>>> + * 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 HAVE_GETENV
>>> +
>>> +#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));
>>
>> This forces allocations and frees in scenarios where this is wholly
>> unnecessary. 
> 
> Why do you think this is unnecessary? At least on Windows, there is
> no guarantee regarding the lifetime of strings returned from
> getenv(). In case when some other code would call _putenv to set the
> env variable, this can cause the previously returned string to become 
> invalid without the caller being able to know.
> 
> 
>> This can be avoided by adding a custom deallocator for
>> strings returned via getenv_utf8: Namely a define/wrapper around
>> av_free in the _WIN32 and a no-op else.
> 
> I don't think I really understand what you mean, by the above 😉
> 

The "scenarios where this is wholly unnecessary" are the scenarios in
which one is in the above #else branch, i.e. not on Windows (I thought
this was clear from the placement of my comment). In this case, the
above av_strdup() can be avoided by adding a custom deallocator that
boils down to av_free on Windows and a no-op in all other cases.

- Andreas
_______________________________________________
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19  5:56   ` Soft Works
  2022-06-19  6:27     ` Andreas Rheinhardt
@ 2022-06-19  6:33     ` Martin Storsjö
  2022-06-19  6:43       ` Andreas Rheinhardt
  1 sibling, 1 reply; 25+ messages in thread
From: Martin Storsjö @ 2022-06-19  6:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Sun, 19 Jun 2022, Soft Works wrote:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Andreas Rheinhardt
>> Sent: Sunday, June 19, 2022 6:59 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
>> wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
>>
>> Nil Admirari:
>>> 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.
>>> ---
>>>  configure                  |  1 +
>>>  libavutil/getenv_utf8.h    | 71
>> ++++++++++++++++++++++++++++++++++++++
>>>  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
>>>  3 files changed, 123 insertions(+)
>>>  create mode 100644 libavutil/getenv_utf8.h
>>>
>>> diff --git a/configure b/configure
>>> index 3dca1c4bd3..fa37a74531 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
>>>      fcntl
>>>      getaddrinfo
>>>      getauxval
>>> +    getenv
>>>      gethrtime
>>>      getopt
>>>      GetModuleHandle
>>> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
>>> new file mode 100644
>>> index 0000000000..161e3e6202
>>> --- /dev/null
>>> +++ b/libavutil/getenv_utf8.h
>>> @@ -0,0 +1,71 @@
>>> +/*
>>> + * 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 HAVE_GETENV
>>> +
>>> +#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));
>>
>> This forces allocations and frees in scenarios where this is wholly
>> unnecessary.
>
> Why do you think this is unnecessary? At least on Windows, there is
> no guarantee regarding the lifetime of strings returned from
> getenv(). In case when some other code would call _putenv to set the
> env variable, this can cause the previously returned string to become
> invalid without the caller being able to know.

Yes, if you would keep the return value from getenv for too long, while 
something else changes the environment in the same process, you'd have 
such an issue. But that hasn't been a concern so far - right? And isn't 
what we try to fix here.

>> This can be avoided by adding a custom deallocator for
>> strings returned via getenv_utf8: Namely a define/wrapper around
>> av_free in the _WIN32 and a no-op else.
>
> I don't think I really understand what you mean, by the above 😉

He means that we could add a getenv_utf8_free(), which would be a noop on 
unix, and keeping getenv_utf8() just returning plain getenv() on unix. 
Then on Windows we'd do the extra alloc/free.

That sounds doable to me. There's a bigger risk of forgetting to free the 
getenv_utf8 output (when tools like valgrind and asan wouldn't notice it 
in unix), but that risk is probably acceptable.

// 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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19  6:33     ` Martin Storsjö
@ 2022-06-19  6:43       ` Andreas Rheinhardt
  0 siblings, 0 replies; 25+ messages in thread
From: Andreas Rheinhardt @ 2022-06-19  6:43 UTC (permalink / raw)
  To: ffmpeg-devel

Martin Storsjö:
> On Sun, 19 Jun 2022, Soft Works wrote:
> 
>>
>>
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>>> Andreas Rheinhardt
>>> Sent: Sunday, June 19, 2022 6:59 AM
>>> To: ffmpeg-devel@ffmpeg.org
>>> Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
>>> wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
>>>
>>> Nil Admirari:
>>>> 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.
>>>> ---
>>>>  configure                  |  1 +
>>>>  libavutil/getenv_utf8.h    | 71
>>> ++++++++++++++++++++++++++++++++++++++
>>>>  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
>>>>  3 files changed, 123 insertions(+)
>>>>  create mode 100644 libavutil/getenv_utf8.h
>>>>
>>>> diff --git a/configure b/configure
>>>> index 3dca1c4bd3..fa37a74531 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
>>>>      fcntl
>>>>      getaddrinfo
>>>>      getauxval
>>>> +    getenv
>>>>      gethrtime
>>>>      getopt
>>>>      GetModuleHandle
>>>> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
>>>> new file mode 100644
>>>> index 0000000000..161e3e6202
>>>> --- /dev/null
>>>> +++ b/libavutil/getenv_utf8.h
>>>> @@ -0,0 +1,71 @@
>>>> +/*
>>>> + * 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 HAVE_GETENV
>>>> +
>>>> +#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));
>>>
>>> This forces allocations and frees in scenarios where this is wholly
>>> unnecessary.
>>
>> Why do you think this is unnecessary? At least on Windows, there is
>> no guarantee regarding the lifetime of strings returned from
>> getenv(). In case when some other code would call _putenv to set the
>> env variable, this can cause the previously returned string to become
>> invalid without the caller being able to know.
> 
> Yes, if you would keep the return value from getenv for too long, while
> something else changes the environment in the same process, you'd have
> such an issue. But that hasn't been a concern so far - right? And isn't
> what we try to fix here.
> 

And if this were an issue, then I don't see what would preclude it from
it already happening in getenv_utf8, in e.g. wchartoutf8 (or in
av_strdup() in the current non-Windows implementation).

- Andreas
_______________________________________________
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19  6:27     ` Andreas Rheinhardt
@ 2022-06-19  7:24       ` Soft Works
  0 siblings, 0 replies; 25+ messages in thread
From: Soft Works @ 2022-06-19  7:24 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Sunday, June 19, 2022 8:28 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
> wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
> 
> Soft Works:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Andreas Rheinhardt
> >> Sent: Sunday, June 19, 2022 6:59 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
> >> wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
> >>
> >> Nil Admirari:
> >>> 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.
> >>> ---
> >>>  configure                  |  1 +
> >>>  libavutil/getenv_utf8.h    | 71
> >> ++++++++++++++++++++++++++++++++++++++
> >>>  libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
> >>>  3 files changed, 123 insertions(+)
> >>>  create mode 100644 libavutil/getenv_utf8.h
> >>>
> >>> diff --git a/configure b/configure
> >>> index 3dca1c4bd3..fa37a74531 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
> >>>      fcntl
> >>>      getaddrinfo
> >>>      getauxval
> >>> +    getenv
> >>>      gethrtime
> >>>      getopt
> >>>      GetModuleHandle
> >>> diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
> >>> new file mode 100644
> >>> index 0000000000..161e3e6202
> >>> --- /dev/null
> >>> +++ b/libavutil/getenv_utf8.h
> >>> @@ -0,0 +1,71 @@
> >>> +/*
> >>> + * 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 HAVE_GETENV
> >>> +
> >>> +#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));
> >>
> >> This forces allocations and frees in scenarios where this is
> wholly
> >> unnecessary.
> >
> > Why do you think this is unnecessary? At least on Windows, there is
> > no guarantee regarding the lifetime of strings returned from
> > getenv(). In case when some other code would call _putenv to set
> the
> > env variable, this can cause the previously returned string to
> become
> > invalid without the caller being able to know.
> >
> >
> >> This can be avoided by adding a custom deallocator for
> >> strings returned via getenv_utf8: Namely a define/wrapper around
> >> av_free in the _WIN32 and a no-op else.
> >
> > I don't think I really understand what you mean, by the above 😉
> >
> 
> The "scenarios where this is wholly unnecessary" are the scenarios in
> which one is in the above #else branch, i.e. not on Windows (I
> thought
> this was clear from the placement of my comment). In this case, the
> above av_strdup() can be avoided by adding a custom deallocator that
> boils down to av_free on Windows and a no-op in all other cases.

Right. I saw the placement was on non-Windows and talked about Windows.
I had read some time ago that the getenv() return value on Linux would
be subject to the same non-granted lifetime "policy" like on Windows, 
but I was too lazy to reconfirm.

> Yes, if you would keep the return value from getenv for too long,
> while
> something else changes the environment in the same process, you'd
> have
> such an issue. But that hasn't been a concern so far - right?

Has this ever been a valid argument on this ML? :-)

> And it isn't what we try to fix here.

Two points regarding that line of argumentation:

First of all: getenv() is a horrible API, it's neither guaranteed to be
thread-safe nor is it safe to make any assumption about the lifetime of
the returned value.

1. For getenv(), a caller needs to know about it's awful specifics, but
   getenv_utf8() is our own API, and I'm not sure whether the fact that
   it's replacing getenv() justifies that we introduce such behavior
   into "our" API as well.

2. For Windows, the code change already has the effect that the caller
   does not need to be afraid about the lifetime/validity of the result.
   So why not expose the same behavior on non-Win platforms as well?
   (via strdup)


In case that one might argue that even with the use of strdup, it won't be
100% safe, which is true, but when comparing the time ranges of the 
possible points of failure,  then, the use of strdup would still catch 
the vast majority of race conditions. 

I think that Nil's version is a better choice. Anyway I don't understand 
the motivation: ffmpeg is allocating huge amounts of memory for video 
frames and here we want to save what - the string return value of an
environment variable?

Maybe I'm missing something and I just can't see it.. :-)

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

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

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-18 22:48     ` Soft Works
@ 2022-06-19  7:49       ` Martin Storsjö
  2022-06-19  8:00         ` Soft Works
  2022-06-19 11:44         ` nil-admirari
  0 siblings, 2 replies; 25+ messages in thread
From: Martin Storsjö @ 2022-06-19  7:49 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Sat, 18 Jun 2022, Soft Works wrote:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Martin Storsjö
>> Sent: Sunday, June 19, 2022 12:24 AM
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove
>> MAX_PATH limit and use UTF-8 version of getenv()
>>
>> On Fri, 17 Jun 2022, Nil Admirari wrote:
>>
>>> 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/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"
>>
>> This actually causes some surprise breakage in MSVC builds. Here,
>> getenv_utf8.h includes windows.h. If including windows.h and winsock2
>> headers in the same file, the winsock2 headers must be included
>> before.
>>
>> I fixed it locally by moving this new include down, with a comment
>> like
>> this:
>>
>> /* This header can include <windows.h>. That header has to be
>> included after
>>   * winsock2 headers (included by network.h and os_support.h above).
>> */
>
> This is the recommended way:
>
>
> #define WIN32_LEAN_AND_MEAN
>
> #include <windows.h>
> #include <winsock2.h>

Thanks, adding #define WIN32_LEAN_AND_MEAN in wchar_filename.h fixes the 
issue.

// 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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-19  7:49       ` Martin Storsjö
@ 2022-06-19  8:00         ` Soft Works
  2022-06-19 11:44         ` nil-admirari
  1 sibling, 0 replies; 25+ messages in thread
From: Soft Works @ 2022-06-19  8:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Martin Storsjö
> Sent: Sunday, June 19, 2022 9:49 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove
> MAX_PATH limit and use UTF-8 version of getenv()
> 
> On Sat, 18 Jun 2022, Soft Works wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Martin Storsjö
> >> Sent: Sunday, June 19, 2022 12:24 AM
> >> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove
> >> MAX_PATH limit and use UTF-8 version of getenv()
> >>
> >> On Fri, 17 Jun 2022, Nil Admirari wrote:
> >>
> >>> 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/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"
> >>
> >> This actually causes some surprise breakage in MSVC builds. Here,
> >> getenv_utf8.h includes windows.h. If including windows.h and
> winsock2
> >> headers in the same file, the winsock2 headers must be included
> >> before.
> >>
> >> I fixed it locally by moving this new include down, with a comment
> >> like
> >> this:
> >>
> >> /* This header can include <windows.h>. That header has to be
> >> included after
> >>   * winsock2 headers (included by network.h and os_support.h
> above).
> >> */
> >
> > This is the recommended way:
> >
> >
> > #define WIN32_LEAN_AND_MEAN
> >
> > #include <windows.h>
> > #include <winsock2.h>
> 
> Thanks, adding #define WIN32_LEAN_AND_MEAN in wchar_filename.h fixes
> the
> issue.

Besides that, it also sounds good  :-)
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-19  7:49       ` Martin Storsjö
  2022-06-19  8:00         ` Soft Works
@ 2022-06-19 11:44         ` nil-admirari
  1 sibling, 0 replies; 25+ messages in thread
From: nil-admirari @ 2022-06-19 11:44 UTC (permalink / raw)
  To: ffmpeg-devel

> Thanks, adding #define WIN32_LEAN_AND_MEAN in wchar_filename.h fixes the 
> issue.

Added WIN32_LEAN_AND_MEAN:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297804.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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv()
  2022-06-18 22:40   ` Martin Storsjö
@ 2022-06-19 11:47     ` nil-admirari
  0 siblings, 0 replies; 25+ messages in thread
From: nil-admirari @ 2022-06-19 11:47 UTC (permalink / raw)
  To: ffmpeg-devel

> FWIW - I wasn't entirely sure we can conclude that we always pass through 
> a case that initializes the err variable here, so just to be sure, I 
> locally amended this patch to initialize the err variable to 0 too.

Fixed: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297806.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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-18 22:21 ` Martin Storsjö
@ 2022-06-19 11:49   ` nil-admirari
  0 siblings, 0 replies; 25+ messages in thread
From: nil-admirari @ 2022-06-19 11:49 UTC (permalink / raw)
  To: ffmpeg-devel

> Note that this should be #if HAVE_GETENV - these constants are always 
> defined and evaluate to 0 or 1. No need to resend the patchset just for 
> that. (I added an explicit #include "config.h" above here too, just to 
> make it clearer.)

Fixed: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297804.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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19  4:58 ` Andreas Rheinhardt
  2022-06-19  5:56   ` Soft Works
@ 2022-06-19 11:56   ` nil-admirari
  2022-06-20  0:54     ` Andreas Rheinhardt
  1 sibling, 1 reply; 25+ messages in thread
From: nil-admirari @ 2022-06-19 11:56 UTC (permalink / raw)
  To: ffmpeg-devel

> This forces allocations and frees in scenarios where this is wholly
> unnecessary. This can be avoided by adding a custom deallocator for
> strings returned via getenv_utf8: Namely a define/wrapper around av_free
> in the _WIN32 and a no-op else.

Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297804.html

Note, however, that the introduction of freeenv_utf8()
doubles allocations and deallocations in vf_frei0r.c on Windows:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297802.html

These additional memory operations can be avoided only with a whole bunch
of new #ifdef _WIN32 and #if HAVE_GETENV, which haven't been done.



_______________________________________________
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-19 11:56   ` nil-admirari
@ 2022-06-20  0:54     ` Andreas Rheinhardt
  2022-06-20 10:36       ` nil-admirari
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Rheinhardt @ 2022-06-20  0:54 UTC (permalink / raw)
  To: ffmpeg-devel

nil-admirari@mailo.com:
>> This forces allocations and frees in scenarios where this is wholly
>> unnecessary. This can be avoided by adding a custom deallocator for
>> strings returned via getenv_utf8: Namely a define/wrapper around av_free
>> in the _WIN32 and a no-op else.
> 
> Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297804.html
> 

Thanks for this.

> Note, however, that the introduction of freeenv_utf8()
> doubles allocations and deallocations in vf_frei0r.c on Windows:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297802.html
> 
> These additional memory operations can be avoided only with a whole bunch
> of new #ifdef _WIN32 and #if HAVE_GETENV, which haven't been done.
> 

Or any reuses the #ifs from getenv_utf8.h.
https://github.com/mkver/FFmpeg/commits/getenv contains a version that
does this.

- Andreas
_______________________________________________
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] 25+ messages in thread

* Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
  2022-06-20  0:54     ` Andreas Rheinhardt
@ 2022-06-20 10:36       ` nil-admirari
  0 siblings, 0 replies; 25+ messages in thread
From: nil-admirari @ 2022-06-20 10:36 UTC (permalink / raw)
  To: ffmpeg-devel

> Or any reuses the #ifs from getenv_utf8.h.
> https://github.com/mkver/FFmpeg/commits/getenv contains a version that
> does this.

Introduced getenv_dup() and simplified #ifs a little:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297841.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] 25+ messages in thread

end of thread, other threads:[~2022-06-20 10:36 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17  9:31 [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Nil Admirari
2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 2/5] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 3/5] fftools: Remove MAX_PATH limit and switch to UTF-8 versions of fopen() and getenv() Nil Admirari
2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 4/5] libavformat: Remove MAX_PATH limit and use UTF-8 version of getenv() Nil Admirari
2022-06-18 22:24   ` Martin Storsjö
2022-06-18 22:36     ` Martin Storsjö
2022-06-18 22:48     ` Soft Works
2022-06-19  7:49       ` Martin Storsjö
2022-06-19  8:00         ` Soft Works
2022-06-19 11:44         ` nil-admirari
2022-06-18 22:40   ` Martin Storsjö
2022-06-19 11:47     ` nil-admirari
2022-06-17  9:31 ` [FFmpeg-devel] [PATCH v17 5/5] libavfilter/vf_frei0r.c: Use " Nil Admirari
2022-06-17 19:16 ` [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8() Soft Works
2022-06-18 22:21 ` Martin Storsjö
2022-06-19 11:49   ` nil-admirari
2022-06-19  4:58 ` Andreas Rheinhardt
2022-06-19  5:56   ` Soft Works
2022-06-19  6:27     ` Andreas Rheinhardt
2022-06-19  7:24       ` Soft Works
2022-06-19  6:33     ` Martin Storsjö
2022-06-19  6:43       ` Andreas Rheinhardt
2022-06-19 11:56   ` nil-admirari
2022-06-20  0:54     ` Andreas Rheinhardt
2022-06-20 10:36       ` 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