Mikewando / vsfpng

fpng for VapourSynth
GNU Lesser General Public License v2.1
11 stars 0 forks source link

Library fails when it comes to unicode paths #1

Open jessielw opened 5 months ago

Mikewando commented 5 months ago

I just copied the path handling from https://github.com/vapoursynth/vs-imwri So can you reproduce the issue with imwri.Write() as well?

Either way if you provide example path and OS I can try to reproduce myself and see how easy/annoying a fix would be.

jessielw commented 5 months ago

Use an output path like example_output_ä as the directory to which the images would write. I just tested that library (vs-imwri) and it also appears to be failing writing with ä but just hangs and doesn't throw an error.

jessielw commented 5 months ago

I am posting a new comment instead of editing the old one so you get the notification.

Did some more testing:

This library fails with umlaut characters but handles long paths/network paths. Testing vs-imwri, it handles umlaut characters correctly but fails with long network paths.

Error from your library:

path: 
\\JLW-UNRAID\dl_share\downloads_nvme_temp\random_folder\downloads\Hostage.2005\Hostage.2005.Remastered.BluRay.1080p.DTS-HD.MA.5.1.AVC.HYBRID.REMUX-FraMeSToRä_temp\Hostage.2005.BluRay.720p.DD.2.0.x264-group_images\img_comparison\%da_source__%d.png

actual error:
vapoursynth.Error: Write: Frame could not be written. Ensure that output path exists and is writable

EDIT: Windows 10 and 11

Mikewando commented 5 months ago

I could not trivially reproduce failures when using umlaut.

How is the string being provided to the plugin? Maybe there is a python-level issue with the input rather than a plugin-level one 🤷 Could also be a vapoursynth/python version specific issue too. I'm still on R65 with python 3.11 since I haven't felt like upgrading to 3.12 yet 🙃

jessielw commented 5 months ago

I could not trivially reproduce failures when using umlaut.

How is the string being provided to the plugin? Maybe there is a python-level issue with the input rather than a plugin-level one 🤷 Could also be a vapoursynth/python version specific issue too. I'm still on R65 with python 3.11 since I haven't felt like upgrading to 3.12 yet 🙃

I'm using awsmfuncs ScreenGen. I looked at the code, and it doesn't look like anything in it would affect unicode chars.

I'll sit down here in a few minutes and do a direct test without ScreenGen to see if I can replicate the issue without it.

I'll report back.

jessielw commented 5 months ago

Here is an example script to reproduce

from pathlib import Path
import vapoursynth as vs

core = vs.core

for plugin in Path(r"E:\programming\img_plugins").iterdir():
    if plugin.is_file():
        core.std.LoadPlugin(plugin)

fp = r"\\JLW-UNRAID\encoding_share\example_path\encodes\Hostage.2005.BluRay.720p.DD.2.0.x264.mp4"

output_path = r"\\?\UNC\JLW-UNRAID\dl_share\downloads_nvme_temp\bhdstudio\downloads\Hostage.2005.Remastered.BluRay.1080p.DTS-HD.MA.5.1.AVC.HYBRID.REMUX-FraMeSToR\Hostage.2005.Remastered.BluRay.1080p.DTS-HD.MA.5.1.AVC.HYBRID.REMUX-FraMeSToRä_temp\Hostage.2005.BluRay.720p.DD.2.0.x264-images_out\img_comparison\test.png"
output_path2 = r"C:\Users\jlw_4\OneDrive\Desktop\test new packs\testing_img_outä\test.png"
output_path3 = r"C:\Users\jlw_4\OneDrive\Desktop\test new packs\testing_img_out\test.png"

clip = core.lsmas.LWLibavSource(source=fp)
clip = core.resize.Spline36(clip,
    format=vs.RGB24, matrix_in_s="709", dither_type="error_diffusion"
)
core.fpng.Write(clip, filename=output_path, overwrite=True, compression=2).get_frame(12000)

output_path and output_path2 both fails

Traceback:

Traceback (most recent call last):
  File "e:\programming\FrameForge\blahy.py", line 38, in <module>
    core.fpng.Write(clip, filename=output_path, overwrite=True, compression=2).get_frame(12000)
  File "src\\cython\\vapoursynth.pyx", line 2037, in vapoursynth.VideoNode.get_frame
vapoursynth.Error: Write: Frame could not be written. Ensure that output path exists and is writable

Tested on VapourSynth 64/65.

Mikewando commented 5 months ago
import vapoursynth as vs

output_path = r"C:\Users\mike\Downloads\example_output_ä\test.png"

clip = vs.core.std.BlankClip(format=vs.RGB24)
clip.fpng.Write(filename=output_path, overwrite=True, compression=2).get_frame(0)

🤔 this works for me so I'm really not sure where the problem lies. I'll leave this issue open, and I would accept PRs which address handling the filenames differently, but probably can't fix it myself without being able to reproduce it. I'm certainly no windows filename expert (hence copying the imwri code) so any guess I make at root cause won't be educated.

jessielw commented 5 months ago
import vapoursynth as vs

output_path = r"C:\Users\mike\Downloads\example_output_ä\test.png"

clip = vs.core.std.BlankClip(format=vs.RGB24)
clip.fpng.Write(filename=output_path, overwrite=True, compression=2).get_frame(0)

🤔 this works for me so I'm really not sure where the problem lies. I'll leave this issue open, and I would accept PRs which address handling the filenames differently, but probably can't fix it myself without being able to reproduce it. I'm certainly no windows filename expert (hence copying the imwri code) so any guess I make at root cause won't be educated.

I did a bit more testing, updated my VapourSynth from v64 to v65. Both long paths and Unicode characters both work with the exact same script on ALL 3 outputs with

core.imwri.Write(clip, "PNG24", output_path2, overwrite=True).get_frame(12000)

However, same bug with your library. I am using pip install VapourSynth-portable, perhaps it has something to do with it?

Regardless, I posted a bug to this issue over here as well. Perhaps once he fixes things on his end you'd be able to resolve this issue?

https://github.com/vapoursynth/vs-imwri/issues/11

jessielw commented 5 months ago

Wanted to update you. This library isn't causing the issue specifically.

I forked the repo and debugged the code. The file path (specifically the Umaut) characters are being mumbled before it ever reaches the library, but is correct going through python/vapoursynth.

My guess (i only ever messed with CPP today) is it has to do with the compiler. I compiled it with GCC and it has the same issues. I couldn't get it to compile with clang.

I feel I've wasted too much time. I'll create a work around to make temp folders on root of drive with non unicode characters to avoid this in my package, unless you are capable of compiling in clang or something for me to test?

Specifically the ä becomes ├ñ

Ultimately, it's not likely worth more of either of our time trying to sort it out. I will develop a work around in Python easy enough to avoid this issue all together and still be able to use the library.

You can close this when ever you'd like!