| Message ID | pull.28.ffstaging.FFmpeg.1652435595.ffmpegagent@gmail.com |
|---|---|
| Headers | show |
| Series | Support long file names on Windows | expand |
> -----Original Message----- > From: ffmpegagent <ffmpegagent@gmail.com> > Sent: Friday, May 13, 2022 11:53 AM > To: ffmpeg-devel@ffmpeg.org > Cc: softworkz <softworkz@hotmail.com> > Subject: [PATCH 0/2] Support long file names on Windows > > This patchset adds support for long file and directory paths on > Windows. The > implementation follows the same logic that .NET is using internally, > with > the only exception that it doesn't expand short path components in 8.3 > format. .NET does this as the same function is also used for other > purposes, > but in our case, that's not required. Short (8.3) paths are working as > well > with the extended path prefix, even when longer than 260. > > Successfully tested: > > * Regular paths wth drive letter > * Regular UNC paths > * Long paths wth drive letter > * Long paths wth drive letter and forward slashes > * Long UNC paths > * Prefixed paths wth drive letter > * Prefixed UNC paths > > I have kept the individual functions separate on purpose, to make it > easy to > compare with the .NET impl. (compilers should inlinie those anyway) Forgot to mention that I tested this with builds from both, MSVC and MinGW/MSYS2 (gcc 10.3). sw
> I have kept the individual functions separate on purpose, to make it easy to > compare with the .NET impl. (compilers should inlinie those anyway) Calling add_extended_prefix without pre-validation results in error, since it does check for \\?\, \\.\, or \??\; only it's wrapper get_extended_win32_path does. And it's not private, it's in the header. path_normalize is a do nothing function. Keeping the comments about where the code originated from may be useful. Copying the structure of .NET results in problems. These patches are very difficult to review. E.g. stat is not covered: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296448.html. Parts of FFmpeg still use fopen instead of av_fopen_utf8. Some of these uses are in examples or tests or inside #ifdef DEBUG blocks. Others aren't: dvdsubdec.c:620 uses fopen and it is not covered by your previous patchset https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296189.html. So please check that all uses of CRT functions that do I/O: https://docs.microsoft.com/en-us/cpp/c-runtime-library/stream-i-o?view=msvc-170 https://docs.microsoft.com/en-us/cpp/c-runtime-library/low-level-i-o?view=msvc-170 https://docs.microsoft.com/en-us/cpp/c-runtime-library/directory-control?view=msvc-170 https://docs.microsoft.com/en-us/cpp/c-runtime-library/file-handling?view=msvc-170 are actually covered (plus WinAPI functions as well).
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of nil- > admirari@mailo.com > Sent: Sunday, May 15, 2022 9:36 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 0/2] Support long file names on > Windows > > > I have kept the individual functions separate on purpose, to make it > easy to > > compare with the .NET impl. (compilers should inlinie those anyway) > > Calling add_extended_prefix without pre-validation results in error, > since it does check for \\?\, \\.\, or \??\; only it's wrapper > get_extended_win32_path does. > And it's not private, it's in the header. > path_normalize is a do nothing function. > > Keeping the comments about where the code originated from may be > useful. > Copying the structure of .NET results in problems. I can squash them together if that would be a common desire. > These patches are very difficult to review. E.g. stat is not covered: > https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296448.html. I'll look into stat in a moment. > > Parts of FFmpeg still use fopen instead of av_fopen_utf8. Some of > these uses are > in examples or tests or inside #ifdef DEBUG blocks. > Others aren't: dvdsubdec.c:620 uses fopen and it is not covered by > your > previous patchset https://ffmpeg.org/pipermail/ffmpeg-devel/2022- > May/296189.html. I have left those out by intention because they are pending removal and are only for debugging. Thanks again, softworkz
> I have left those out by intention because they are pending removal > and are only for debugging. Is dvdsubdec.c parse_ifo_palette pending removal? What about - vf_pnsr.c init() - vf_vidstabdetect.c config_input() - vf_vidstabtransform.c config_input()?
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of nil- > admirari@mailo.com > Sent: Monday, May 16, 2022 10:45 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 0/2] Support long file names on > Windows > > > I have left those out by intention because they are pending removal > > and are only for debugging. > > Is dvdsubdec.c parse_ifo_palette pending removal? Just the debug part. Seems I missed the other one. > - vf_pnsr.c init() > - vf_vidstabdetect.c config_input() > - vf_vidstabtransform.c config_input()? I don't have the latter two included due to missing lib. No idea how I have missed the other two. Thanks for pointing out. Update will follow. Kind regards, softworkz