bestsource icon indicating copy to clipboard operation
bestsource copied to clipboard

Fix VS Source and Cache paths are parsed improperly

Open BoatsMcGee opened this issue 1 year ago • 0 comments

This PR intends to mostly address #59 with minimal code changes that would affect other functions and systems. However, these changes should continue to be expanded upon to improve other similar scenarios not considered by Issue #59.

Crucial Considerations

Before completing this PR, please consider the following:

  • There are temporary logging statements to help demonstrate the issue and how it is resolved should you wish to debug it. These should be removed or modified to fit the logging practices of the project before merging.
  • I do not have extensive knowledge of C++. If I made a mistake or if anything could be improved, please discuss it here and I will gladly update this PR with change(s).
  • Future changes could theoretically implement more of the standard library to improve the code.
  • Unfortunately, the standard library does not seem to specifically address the extended-length path prefixes, so a simple handling of that case is present in the new CleanFilePath(...) function.
  • Cache Path is not ensured to be a writable path. We should discuss where and how this could be handled. I suggest that if it fails to write to the disk then we should fallback to the default behavior. This should also be reflected in the documentation.

Changed Behavior

Example of new behavior indicating changes for a relative path:

C:\Path\To> vspipe.exe -p -c y4m .\script.vpy -s 0 -e 1 .\result.y4m
Information: CLEANED SOURCE: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Initial Cache Path: not_real\..\temporary\split\cache.json
Initial Source Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Mangled Relative Cache Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Initial Cache Path: not_real\..\temporary\split\cache.json
Initial Source Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Mangled Relative Cache Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Script evaluation done in 3.19 seconds
Output 2 frames in 0.17 seconds (11.97 fps)
C:\Path\To> vspipe.exe -p -c y4m .\loadscript.vpy -s 0 -e 1 .\result.y4m
Information: CLEANED SOURCE: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Initial Cache Path: not_real\..\temporary\split\cache.json
Initial Source Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Mangled Relative Cache Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Script evaluation done in 0.31 seconds
Output 2 frames in 0.16 seconds (12.43 fps)

Script used is a modified version of the one references in #59. For the above example it looks like the following:

from vapoursynth import core
core.max_cache_size=1024
core.bs.VideoSource("\\\\?\\C:\\Path\\To\\[SomeTag] $SomeName With-Dashes_And+Underscores\\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv", cachepath="not_real\\..\\temporary\\split\\cache.json").set_output()

Example of new behavior indicating changes for an absolute path as requested in #59:

C:\Path\To> vspipe.exe -p -c y4m .\loadscript.vpy -s 0 -e 1 .\result.y4m
Information: CLEANED SOURCE: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Initial Cache Path: \\?\C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Initial Source Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Mangled Absolute Cache Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Initial Cache Path: \\?\C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Initial Source Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Mangled Absolute Cache Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Script evaluation done in 3.14 seconds
Output 2 frames in 0.16 seconds (12.27 fps)
C:\Path\To> vspipe.exe -p -c y4m .\loadscript.vpy -s 0 -e 1 .\result.y4m
Information: CLEANED SOURCE: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Initial Cache Path: \\?\C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Initial Source Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\[Some.Tag] Some_Name - S01E01 [BD 1080p SomeTag][Some-Tag].mkv
Mangled Absolute Cache Path: C:\Path\To\[SomeTag] $SomeName With-Dashes_And+Underscores\temporary\split\cache.json
Script evaluation done in 0.30 seconds
Output 2 frames in 0.16 seconds (12.60 fps)

Note the Source file path is also correct and the program successfully accesses the file. Also notice that the Cache Path is correct and upon the 2nd execution the script evaluation time period is much smaller than the 1st. This is an indication that the cache was successfully written in the 1st execution and retrieved in the 2nd, resolving the performance concerns raised in #59.

BoatsMcGee avatar Jul 19 '24 01:07 BoatsMcGee