On Fri, 15 Apr 2022, Nil Admirari wrote:
These functions are going to be used in libavformat/avisynth.c
and fftools/cmdutils.c remove MAX_PATH limit.
---
libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
I looked through this patchset now, and I think it overall looks
acceptable - with a couple minor things I'd like touched up and clarified.
However while reviewing this, I noticed a preexisting issue regarding the
av_fopen_utf8 function. This patchset extends the use of that function
into fftools, which isn't great given the issue... The use is fairly
marginal (the preset files) so I guess it can be tolerated, as it's a
preexisting orthogonal issue.
The other question is whether it's tolerable to use more non-installed
headers (like libavutil/wchar_filename.h) in fftools. (I'd like to have
this point confirmed with Anton before landing the patchset.)
A couple comments on the patch below:
diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index 90f08245..e22ffa8a 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -40,6 +40,57 @@ static inline int utf8towchar(const char *filename_utf8,
wchar_t **filename_w)
MultiByteToWideChar(CP_UTF8, 0, filename_utf8, -1, *filename_w, num_chars);
return 0;
}
+
+av_warn_unused_result
+static inline int wchartocp(const unsigned int code_page, const wchar_t
*filename_w,
+ char **filename)
+{
+ const DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
+ const int num_chars = WideCharToMultiByte(code_page, flags, filename_w, -1,
+ NULL, 0, NULL, NULL);
We don't generally use 'const' like this in ffmpeg; we use 'const' where
it makes a functional difference (i.e. on the type that pointers point
at), but not for plain scalar values (neither parameters nor local
variables).
+ if (num_chars <= 0) {
+ *filename = NULL;
+ return 0;
+ }
+ *filename = av_calloc(num_chars, sizeof(char));
+ if (!*filename) {
+ errno = ENOMEM;
+ return -1;
+ }
+ WideCharToMultiByte(code_page, flags, filename_w, -1,
+ *filename, num_chars, NULL, NULL);
+ return 0;
+}
+
+av_warn_unused_result
+static inline int wchartoutf8(const wchar_t *filename_w, char **filename)
+{
+ return wchartocp(CP_UTF8, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int wchartoansi(const wchar_t *filename_w, char **filename)
+{
+ return wchartocp(CP_THREAD_ACP, filename_w, filename);
+}
I think this actually would be more correct to use CP_ACP, not
CP_THREAD_ACP. A bit of googling led me to
https://github.com/ocaml/ocaml/issues/7854 which indicates that CP_ACP is
used for ansi file functions, regardless of the current thread specific
codepage. (But I see that this already is used in libavformat/avisynth.c,
so it's in one sense a preexisting issue, but I think it'd be more correct
to use CP_ACP here.)
// 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-requ...@ffmpeg.org with subject "unsubscribe".
