Make static linking work in Windows

[CrendKing]

To use the generated .lib file, I made the following changes to my own program:

1. Put #define AVS_STATIC_LIB before #include <avisynth.h>.
2. Obviously, remove all the AVS_linkage stuff.
3. Replace LoadLibrary() with AvsAllocTls().
4. Replace FreeLibrary() with AvsFreeTls().
5. Add the new dependencies. Can be done in MSVC project config too:

#pragma comment(lib, "AviSynth.lib")
#pragma comment(lib, "imagehlp.lib")
#pragma comment(lib, "msacm32.lib")
#pragma comment(lib, "vfw32.lib")

I don't know what CAVIFileSynth is, and I don't use it, so I didn't change anything about DllGetClassObject and DllCanUnloadNow.

[pinterf]

What is the status of this PR? Did it change things on non-Windows platforms when AVS_STATIC_LIB is not defined?

[CrendKing]

Oh hi printerf. I was waiting for qyot27 to give another round of review. I think he/she might be busy with some Haiku issues or forgot about it.

As to your question about non-Windows platforms, since all my changes are related to __declspec(dllimport) and DllMain(), two of Windows-specific constructs, I think non-Windows platforms should be untouched. But I don't have a Linux at hand to test, so if any of you have, may I ask your help to try it?

[pinterf]

A minimal skeleton demo would be welcome to demonstrate static linking. Not if I wouldn't do that but to spare time. Sorry, I had no time either to go any deeper into the subject and to check whether if it works O.K. for this specific needs but may cause surprises in another area.

[qyot27]

No, I've been slammed with other things I've had to take care of. The two things I was mostly concerned with was the fact it adds API entries (which should be done as part of a larger bump so we're not constantly incrementing things) and that some of the tests on Windows were resulting in unstable initialization in ffmpeg.

[CrendKing]

it adds API entries

Changed the code a bit so that it only adds API when AVS_STATIC_LIB is on.

some of the tests on Windows were resulting in unstable initialization in ffmpeg

Anywhere I can check these myself?

A minimal skeleton demo

I can make this for Windows later. As to test for stability on non-Windows platform, what needed to be done? I don't see any unit or integration test in repo, so if I make a Linux virtual machine, what are the steps you recommend to try?

[pinterf]

some of the tests on Windows were resulting in unstable initialization in ffmpeg.

Was it with static linking avisynth to ffmpeg?

[pinterf]

This whole Thread Local Storage (TLS) allocation is guarded by a
#define XP_TLS
in avs/config.h

All this magic was needed because of XP support; XP had zero thread safety mechanism regarding __declspec(thread), which simply did not work for XP and crashed. I have already spent quite a few days on XP and thread init (that's why our XP builds should specify an additional /ZcThreadSafeInit- hack)

I recommend not using #define XP_TLS for non-XP builds.

Secondly TlsAlloc bounds to DLL_PROCESS_ATTACH which can occur only once, DLL loads are reference counted, DLL_PROCESS_ATTACH can occur only once. For DLL unload: reference counter is decremented and when reaches zero, DLL_PROCESS_DETACH is called which in turn is calling TlsFree when XP-style TLS is defined.

It's not that easy, clearly I do not want to support XP in this regard (neither in other aspects :) ). No wonder that clang, boost and others have stopped xp support for ages. There is a possibility that it still does not work with an undefined XP_TLS, but at least we have to search problems in other areas.

[CrendKing]

About XP stuff, I don't think I changed anything related to it, did I? Without XP_TLS, AvsAllocTls() and AvsFreeTls() are empty functions. We could make "not using #define XP_TLS for non-XP builds" in a different commit, right? About call only once, sure this simple commit can't guarantee the new AvsAllocTls() is only called once. Do you mean to introduce additional thread-safe OS-independent approach to safe guard that? I thought since these are completely new "APIs", we could afford to just document them saying "callers: only call these once".

[pinterf]

On non-XP we don't need TlsAlloc, the language will do the needed stuff automatically by __declspec(thread)
See the many places where I had to put this special treatment.

[CrendKing]

OK. I understand you now. Here is what I went through.

XP_TLS is defined in config.h, guarded by AVS_WINDOWS, which is created by _WIN32, which is defined in all Windows builds. So XP_TLS and AVS_WINDOWS both are defined on all Windows.

I'm testing on Windows 10. https://github.com/AviSynth/AviSynthPlus/blob/master/avs_core/core/main.cpp#L125 is always executed, which defines dwTlsIndex as 0. Then whenever I call CreateScriptEnvironment(), https://github.com/AviSynth/AviSynthPlus/blob/master/avs_core/core/avisynth.cpp#L2213 always throws if I don't call TlsAlloc() before.

What am I missing? Is there some CMake magic that checks Windows version and overrides the definition of XP_TLS?

[pinterf]

Well, at the moment simply uncomment define XP_TLS and build the whole avisynth without it.
Later I'll decide that in the future we'll omit it selectively for non-XP builds. But for now, just make the test without it.
Neither TlsAlloc, nor dwTlsIndex variable exists in this scenario

[CrendKing]

OK. Now the change is simpler.

And here is the minimum demo that just creates environment and print version string. I tested on my Windows 10 environment (using some MSBuild trick to make sure the header and lib files are in path). Do you want me to test it on a Linux?

#include <iostream>

#define WIN32_LEAN_AND_MEAN
#include <windows.h>

#define AVS_STATIC_LIB
#include <avisynth.h>


#pragma comment(lib, "AviSynth.lib")
#pragma comment(lib, "imagehlp.lib")
#pragma comment(lib, "msacm32.lib")
#pragma comment(lib, "vfw32.lib")
#pragma comment(lib, "winmm.lib")

auto main(int argc, char *argv[]) -> int {
    IScriptEnvironment *env = CreateScriptEnvironment();
    if (env == nullptr) {
        std::cerr << "Failed to create AVS environment" << std::endl;
        return EXIT_FAILURE;
    }

    std::cout << "AVS version: " << env->Invoke("Eval", AVSValue("VersionString()")).AsString() << std::endl;
    env->DeleteScriptEnvironment();
    return EXIT_SUCCESS;
}

[pinterf]

Thanks. I wonder how it behaves if you stress test it and do the core create/delete parallelly from N threads.

[pinterf]

These commits finally - if they work - will be put together into a single good one. For example this latter config.h commit disables the XP_TLS handling from Avisynth+ globally, which is an unwanted step at the moment, I intended to ask for doing it only in your test bench if it works or not.

[CrendKing]

I intended to ask for doing it only in your test bench if it works or not.

Oh, I know. Was just showing what I changed for the record. Will update before final.

I wonder how it behaves if you stress test it and do the core create/delete parallelly from N threads.

Will do.

[CrendKing]

Here's the parallel demo. Haven't noticed any failure.

#include <array>
#include <iostream>
#include <thread>

#define WIN32_LEAN_AND_MEAN
#include <windows.h>

#define AVS_STATIC_LIB
#include <avisynth.h>


#pragma comment(lib, "AviSynth.lib")
#pragma comment(lib, "imagehlp.lib")
#pragma comment(lib, "msacm32.lib")
#pragma comment(lib, "vfw32.lib")
#pragma comment(lib, "winmm.lib")

auto main(int argc, char *argv[]) -> int {
    std::array<std::thread, 16> threads;
    for (int i = 0; i < threads.size(); ++i) {
        threads[i] = std::thread([]() -> void {
            while (true) {
                IScriptEnvironment *env = CreateScriptEnvironment();
                if (env == nullptr) {
                    std::cerr << "Failed to create AVS environment" << std::endl;
                } else {
                    std::cout << "AVS version: " << env->Invoke("Eval", AVSValue("VersionString()")).AsString() << std::endl;
                    env->DeleteScriptEnvironment();
                }
                Sleep(10);
            }
        });
    }
    for (int i = 0; i < threads.size(); ++i) {
        threads[i].join();
    }

    return EXIT_SUCCESS;
}

[pinterf]

Seems good. What does that syntax in 'main' line do? I mean that -> construct, some c++20 feature?

[CrendKing]

Trailing return style is actually C++11. Main take for me is the aligned function names. Imagine

    auto GetInputFormat() const -> Format::VideoFormat;
    auto GetOutputFormat() const -> Format::VideoFormat;
    auto ReloadAvsFile(const std::wstring &avsFile) -> void;
    auto GetVideoSourcePath() const -> std::wstring;
    auto GetVideoFilterNames() const -> std::vector<std::wstring>;
    auto GetAvsState() const -> AvsState;

instead of

    Format::VideoFormat GetInputFormat() const;
    Format::VideoFormat GetOutputFormat() const;
    void ReloadAvsFile(const std::wstring &avsFile);
    std::wstring GetVideoSourcePath() const;
    std::vector<std::wstring> GetVideoFilterNames() const;
    AvsState GetAvsState() const;

in the header file.

[qyot27]

Since I had time to run some tests again, I discovered that the loadtime instability with FFmpeg was actually present in 3.7.0, and it was in one of the core filters (Invert) the test script was using. Further, issues with the 64-bit GCC build also existed with 3.7.0, so fixing that should be a separate issue from this anyway.

Given that, I squashed the changes in this PR into a single commit and pushed it to master.