Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Avisynth compilation and update headers #161

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

arch1t3cht
Copy link
Collaborator

Main changes:

  • Update build system to fix compilation on Windows with AviSynth enabled
  • To fix a lot of crashes when opening various videos, use updated headers from AviSynthPlus, which are obtained from a subproject as proposed in [win] Reopen and update Avisynth support #134

With this, Aegisub can open arbitrary .avs files, as well as many ordinary video files using DirectShowSource. Since it still crashes for some videos (and because of the other questions below), I've made this a draft for now.

The biggest annoyance in this setup is that AviSynth is now pulled twice:

  • During compilation to generate the headers
  • When building the installer, the DLL's for AviSynth and some of its plugins are pulled from the most recent release. This is done regardless of whether AviSynth is enabled or not.

That's also why I've pinned the version of the AviSynth headers to the current latest release for now.
I'm not sure if there's any canonical better way to do this. One could also make meson compile AviSynthPlus as well as the necessary plugins, and then only bundle those with the installer if they exist. But that also feels slightly weird to do for an optional dynamically linked dependency.

Still crashes when loading a video.
- Build the latest AviSynth headers using CMake, as proposed in #134
- Add and initialize AVS_Linkage to fix video loading

Video and audio playback now works in most cases, but still crashes for
some files.
This makes meson detect changes to files like default_config.json
and rebuild libresrc.cpp when necessary.
- This refers to AviSynth < 2.5.6, which is from before 2005
- With the current setup (using GetAVSLinkage, these versions wouldn't
  work anyway)
- The latest AviSynthPlus is bundled with the installer anyway
@arch1t3cht
Copy link
Collaborator Author

There's one commit 635503a in here that I needed for this, but which is useful regardless of AviSynth. Previously, meson wouldn't pick up on default_config.json being changed between builds (I also noticed this with #150 , for example). I added all the files listed in manifest.respack as inputs for the target building the default config. The downside, though, is that this uses the read() function in the fs module, which is only available for meson >= 57.0 . I updated the minimum version, but that's another reason why this is a draft for now.

@arch1t3cht arch1t3cht marked this pull request as draft July 5, 2022 23:41
@arch1t3cht
Copy link
Collaborator Author

After downgrading Meson to 62.2, the CI ran through for me on Windows with AviSynth enabled: https://github.com/arch1t3cht/Aegisub/runs/7222739058?check_suite_focus=true


resrc += custom_target('default_config.{cpp,h}',
command: [respack, '@INPUT0@', '@OUTPUT@'],
input: [files(resmanifest), resmanifest_files],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use the depend_files kwarg for this:

files (str, file, or the return value of configure_file() that this target depends on but are not listed in the command keyword argument. Useful for adding regen dependencies.

@arch1t3cht
Copy link
Collaborator Author

Just to wrap this up, I believe this is more or less complete now. I added support for compiling with Avisynth on Linux and included wangqr's fixes to the Avisynth providers, with some fixes and cleanup. Some video files still crash or freeze Aegisub, but these behave the same on other builds with Avisynth support.

Most importantly, opening video or audio from .avs files works, so it's theoretically possible to use sources like LSMASHSource via Avisynth in Aegisub if FFMS2 is causing issues again.

I'll keep other changes I make on my own fork for now, since I saw that feature additions aren't the priority right now. But since this PR was already opened, I wanted to at least add the missing parts here.

@arch1t3cht arch1t3cht marked this pull request as ready for review August 9, 2022 01:10
arch1t3cht and others added 2 commits August 13, 2022 01:06
Co-authored-by: wangqr <wangqr@wangqr.tk>
Because... let's maybe not let users make Aegisub call arbitrary
symbols in avisynth.dll/so just by editing the config.json.
Starting with AVISYNTH_INTERFACE_VERSION=5, this is how script
environments should be deleted. The previous code was causing crashes
when unloading AviSynth in certain scenarios, such as when failing to
open a file due to an incorrect path.
Since FileNotFound exceptions don't abort the provider search, opening
an invalid path would show errors such as "avisynth not found" when
Avisynth wasn't installed, even if Avisynth wasn't selected as the video
provider.
When Avisynth is not installed or not functional, this would otherwise
cause a crash when trying to initialize Avisynth more than once, since
after the first time the refcount would have been incrased anyway.
These are broken in some edge cases, such as smb mounts on Windows.
Paired with d96fc1f.
Proper fix to the issue that f5a730f
was trying to fix.

Fixes #61 .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants