From: "Martin Storsjö" <martin@martin.st> To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows Date: Wed, 20 Apr 2022 15:47:59 +0300 (EEST) Message-ID: <3174427a-8eb7-b2c-eca4-242d8c9d32c@martin.st> (raw) Hi, I just became aware of the av_fopen_utf8 function - which was introduced to fix path name translations on Windows - actually has a notable design flaw. Background: On Windows, a process can contain more than one C runtime (CRT); the system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC builds, each DLL/EXE can have one statically linked in instead of linking against a shared library CRT (and that's actually the default configuration when building with MSVC). This means that CRT objects (file descriptors from open(), FILE* opened with fopen/fdopen) mustn't be shared across DLLs; such an object must be opened, accessed and closed within the same DLL. The issue: This was fixed for the avpriv_open() function in e743e7ae6ee7e535c4394bec6fe6650d2b0dbf65 by duplicating the file_open.c source file in all libav* libraries (in MSVC builds, and renaming avpriv_open to ff_open), so that each library gets their own copy of ff_open (which isn't exported), so that all calls to open/read/write/close are done within the same DLL. When av_fopen_utf8 was added afterwards, in 85cabf1ca98fcc502fcf5b8d6bfb6d8061c2caef, this issue wasn't taken into account - although the issue is somewhat eased by lucky coincidence. As av_fopen_utf8 is implemented in the same source file, libavutil/file_open.c, which gets duplicated in all libraries that use it, all uses of the function in other libraries (such as libavformat) actually end up using their own copy of it. (This also means that all the libav* DLLs export this function.) But for any users of the function outside of the ffmpeg libraries, this function (in libavutil, or whichever library it ends up imported from) returns a FILE* allocated by libavutil's CRT, which then can't be used with the fread/fwrite/fclose/whatever functions in the DLL/EXE that called it. One concrete example of this is that the function is used for the twopass log file in fftools/ffmpeg_opt.c. To see the issue in action, build ffmpeg with MSVC with this config: ../ffmpeg/configure --enable-shared --toolchain=msvc --prefix=<dest> Then try to do a twopass encoding with it: ffmpeg -i <input> -an -c:v ffv1 -pass 1 -f null - ffmpeg -i <input> -an -c:v ffv1 -pass 2 -y test-2pass.mkv The same issue would appear anywhere this function is used from libavutil built as a DLL, from a caller built with a different CRT choice (e.g. often if mixing mingw/MSVC builds, which otherwise is supported just fine). (If built with a shared CRT, i.e. configured with --extra-cflags=-MD, it does work though.) As this is a public function, we can't really do many tricks like we do within the libraries. (On the other hand, while it is a public function, it doesn't seem to be used much outside of ffmpeg, other than in ffmpeg API bindings for other languages.) I guess the only really robust solution would be to turn av_fopen_utf8 into a static inline function within the headers, essentially inlineing a copy of wchar_filename.h there, so that it expands to a call to the callers' _wfopen or similar. But that would end up polluting users' code by implicitly including windows.h everywhere, which really isn't nice to do either. Or should we just document this issue, discourage further external use of the function, and fold this function into a ffmpeg-internal inline helper like libavutil/whcar_filename.h? // 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".
next reply other threads:[~2022-04-20 12:48 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-20 12:47 Martin Storsjö [this message] 2022-05-07 4:22 ` Soft Works 2022-05-07 4:32 ` Andreas Rheinhardt 2022-05-07 5:02 ` Soft Works 2022-05-08 20:11 ` Martin Storsjö 2022-05-09 0:30 ` Soft Works 2022-05-09 9:41 ` Martin Storsjö 2022-05-09 10:38 ` Soft Works 2022-05-09 10:47 ` Martin Storsjö 2022-05-09 10:53 ` Soft Works 2022-05-08 20:01 ` Martin Storsjö 2022-05-09 0:28 ` Soft Works 2022-05-09 9:36 ` Martin Storsjö 2022-05-09 10:59 ` Soft Works
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=3174427a-8eb7-b2c-eca4-242d8c9d32c@martin.st \ --to=martin@martin.st \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
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