CrendKing / avisynth_filter

DirectShow filters that put AviSynth and VapourSynth into video playing
MIT License
107 stars 8 forks source link

[Feature Request] Optional configuration through an .ini-file #20

Closed Nuihc88 closed 3 years ago

Nuihc88 commented 3 years ago

By first checking whether a specific .ini file exists, then loading settings from the file if it exists, or registry if it doesn't, AviSynth Filter could support both portable players and multiple configurations (player presets) on a single system.

Things to consider...

  1. Whether enabling portable mode should be available from the configuration menu or only documented in the wiki?
    • Users who want to make their configurations portable, generally don't mind reading through the documentation to set things up.
  2. What is the best location and format for the .ini file?
    • Most portable filters save their configurations next to their binaries, but there is a lot of variation in the exact format used.
CrendKing commented 3 years ago

Thanks for the suggestion. I think supporting "profiles" for the filter is good idea. However, unlike normal applications, I don't like the seemingly direct approach of reading ini file from video player's directory. Here is my reasoning.

DirectShow filters traditionally are registered in OS, via regsvr32. When registered, the filters are available system-wide. There is no user-specific installation. regsvr32 also does not provide argument to alter the behavior. Additionally, there is no way to provide "instance" data per registration or installation. This means, in Microsoft's design, DirectShow filters with the same version should behave isomorphically. Newer version of such filters override older version.

Some video players can load the filter without it been first registered. However, it basically performs a registration on the filter by itself and read back the changes made, then write to its own configuration. The nature of registration is the same.

With this in mind, it would seem inconsistent if suddenly a filter behaves differently according to which application loads it, or what the working directory is when it's loaded. madVR is a good example. It supports different configuration based on input video metadata, but all its configuration are in the same place (registry and its own directory).

So I think there are two alternative approaches. Either we store profile information in the registry (by expanding its current structure), or store in one ini file alongside the filter .ax file. This way it maintains the singleton nature.

Each profile should consist of two to three parts: a profile activation condition, all the profile-specific settings and an optional descriptive profile name. Initially the profile condition only tests for loading program's full path, but we could expand that in the future. Multiple profiles can activate if their conditions are all met. Conflict settings may be resolved in a sequential fashion (last appeared wins).

Any comment?

Nuihc88 commented 3 years ago

Truly portable program installations require always copying the filter somewhere under the player directory and then selecting the .ax manually from the host program. So to clarify: there could easily end up being 2(+) locations with a copy of the filter if the user has multiple portable programs using it; yet none of the copies would necessarily be registered to the host system. Thus would it not make sense to always store the .ini next to the .ax-file?

DirectShow filters traditionally are registered in OS, via regsvr32. When registered, the filters are available system-wide. There is no user-specific installation. regsvr32 also does not provide argument to alter the behavior. Additionally, there is no way to provide "instance" data per registration or installation. This means, in Microsoft's design, DirectShow filters with the same version should behave isomorphically. Newer version of such filters override older version.

Is there some situation where this would pose a problem?

Another benefit of allowing completely different configurations for each copy of the filter is that it could help speed up troubleshooting tasks, since user could take breaks with a working copy and then do file-comparisons between it and a non-working copy. An .ini file would simply be more convenient to back up and read from.

All that said, i think the profile approach has merit regardless of whether the user has a portable installation or not and whether the profile changes are applied based on program path, through keyboard shortcuts or based on more complex conditions later on. More flexibility will benefit the end user.

CrendKing commented 3 years ago

2(+) locations with a copy of the filter if the user has multiple portable programs using it; yet none of the copies would necessarily be registered to the host system

Why would anyone prefer to have multiple copies of the filter, if they all (at least should) aim to solve the same problem? Isn't it terrible to maintain? Remember back in the old days Windows used to have situation called DLL Hell. It is not exactly the same problem here, but having multiple versions of the same module scattering around the system is not generally good idea. You'd want one copy that every program loads, so that if that dll needs upgrade (for security or bug fix purposes), you know where to go.

Programs, on the other hand, is different, because a program is meant to self contain. To extreme, a statically linked exe file should be able to run anywhere and any OS you put. Adding an ini to it, you got a true portable application. I don't think dll is ever regarded to be able to be "portable".

So in a sense, this is a novel problem we are trying to solve. We can do whatever we want, but if designed badly there will be cost to fix.

An .ini file would simply be more convenient to back up and read from.

First, I used registry mainly because all DirectShow filters I've used before do the same. You could call it momentum or laziness. Second, I'm not completely sure if ini is easier to read and understand for every computer user. We technical people may be accustomed to text files and editors, but registry editor is a GUI tool designed to change key-value settings. Plus registry has type system, plain text file does not. For example, not everybody on the earth knows that in computer, you use quotes to designate strings.

I agree with the the backup part.

More flexibility will benefit the end user.

It depends on how much complexity we need to introduce for that flexibility. Ideally, I'd like to introduce profile in a way that user can understand and use it without we developing a GUI tool to help. Like you said, if we can be sure only power users will use this feature, ini might be the way to go. If not, we need to consider some automation. And if so, it wouldn't matter how are the profiles are stored.

In general, if a feature can be introduced through abstraction, it is always better so than exposing the raw backend data structures.

Nuihc88 commented 3 years ago

Why would anyone prefer to have multiple copies of the filter, if they all (at least should) aim to solve the same problem?

Let's say you have a portable video player that you have optimized for use with Musicvideos, another optimized for Movies & TV-shows & maybe even one just for Anime, some 32bit and some 64bit, each of them self-contained on an USB-stick. Having multiple independent players might otherwise be unnecessary for players that have Preset-functionality, except that there are features not implemented in some and even some external filters which haven't been upgraded for 64bit (DC-DSP Filter for example) or have compatibility issues with let's say Win10. In other words, there are dozens of novel use cases that can simultaneously be addressed by having multiple portable players with their own filter configurations.

Isn't it terrible to maintain?

In practice it is possible to replace multiple copies of a file at once using hardlinks, but that also has the downside of potentially introducing new bugs into all players at once. First testing things out on player(s) with default settings, before migrating files to stable portable players can save a lot of trouble. That approach is also good for general troubleshooting tasks; if you encounter a problem with one and need to do some diagnostics, then you can first try loading it on the other players to see if any works and then compare the differences between them.

Remember back in the old days Windows used to have situation called DLL Hell. It is not exactly the same problem here, but having multiple versions of the same module scattering around the system is not generally good idea.

It is without portable programs that things can more easily descend into something close to 'DLL Hell', usually through misconfigured registry. I think multiple copies is still going to happen one way or another. The user can start making weekly backups or take other precautions such as having another player on standby somewhere. The only real alternative is that the user simply stops updating their software until new formats make it obsolete and then they would have to start configuring everything again from scratch.

You'd want one copy that every program loads, so that if that dll needs upgrade (for security or bug fix purposes), you know where to go.

Using hardlinks makes upgrading multiple copies of a file on a single drive quite easy; if you need to switch OS often, move around a lot or need to do troubleshooting for a script that relies on a bunch of filters, etc. portable players actually end up being much easier to maintain.

Programs, on the other hand, is different, because a program is meant to self contain. To extreme, a statically linked exe file should be able to run anywhere and any OS you put. Adding an ini to it, you got a true portable application. I don't think dll is ever regarded to be able to be "portable".

Most of the software on PortableApps.com (for example) have dynamically linked .dlls contained within them, some have directshow filters as well. A .dll by itself might not ever be regarded as a portable application, but adding an .ini option makes it a cleaner solution than adding some elaborate and hard to read third-party registry wrapper around it.

Second, I'm not completely sure if ini is easier to read and understand for every computer user. We technical people may be accustomed to text files and editors, but registry editor is a GUI tool designed to change key-value settings. Plus registry has type system, plain text file does not. For example, not everybody on the earth knows that in computer, you use quotes to designate strings.

I think that most of the time people wouldn't even need to edit anything on it as long as a configuration interface is available, but let's say you have 'Portable Player 1' which is configured for Anime, 'Portable Player 2' which is configured for Live Action, as well as 'Portable Player 3' which has an improved Live Action configuration, but the thing is crashing... Loading files of all the players in WinMerge would allow the user to quickly see the differences highlighted and allow them to quickly copy-paste each difference on line by line basis. Also, about 15 years ago practically all computer technicians were telling people to never edit their system registries as they thought users would mess their systems up by trying to make their computers go faster.

It depends on how much complexity we need to introduce for that flexibility. Ideally, I'd like to introduce profile in a way that user can understand and use it without we developing a GUI tool to help. Like you said, if we can be sure only power users will use this feature, ini might be the way to go. If not, we need to consider some automation. And if so, it wouldn't matter how are the profiles are stored.

Perhaps a tray-icon that opens up the 'AviSynth Filter Properties' window, which could have a Profiles-tab with a sidepanel-menu listing the profiles and a Create-button; then some simple options for conditional loading, etc. on the selected profile, would be the way to go.

In general, if a feature can be introduced through abstraction, it is always better so than exposing the raw backend data structures.

I think providing both and thus rendering the exposed backend data structures redundant to the end user (except for troubleshooting), would provide the benefits of both approaches; people generally prefer to use visual interfaces if given the option.

CrendKing commented 3 years ago

I can understand why someone wants to have multiple video players for different purposes, but shouldn't they all add the one avisynth_filter_32.ax as external filter? Why would anyone wants to, say "my PotPlayer should have avsf 0.7.1, and my MPC-BE should use avsf 0.7.5"? If there is a problem in 0.7.1, create a ticket and let's fix it. Otherwise, you are missing all the bug fixes and new features.

It would be another story if for example avsf introduce a backward incompatible change in 0.8.0, and some of your script can't use it. For temporary workaround, you might want to keep two versions. But we haven't done any BI change. And even if we do, there should be grace period for people to upgrade.

I do agree if we want to make the USB stick use case possible, there should be file, not registry, that serves configuration to avsf. How you manage the versions is up to you.

Let's say we rule out registry and go for conf file. And there are multiple avsf copies in the system, even though we don't know that fact. Is it reasonable to assume people would add one avsf copy to multiple players as external filters, and expect them all behave differently or the same?

  1. If they behave differently, we need to store per-player profile in each conf.
  2. If they behave the same, options changed in "player 2" would overwrite the earlier changes made in "player 1".

Design 1 encourages people to use as few copies of avsf as possible, as profile has already built in. And if people deviate from that principle, soon they might be overwhelmed by the matrix of "avsf version - player program".

Design 2 forces people to make copies per player if they want a proper profile system. If they actually have no need to multi-version avsf, they need to hardlink or symbolic link the .ax file, which I assume most users don't have the knowledge to. So they might be overwhelmed by versions.

Design 3 is load the conf file not alongside the avsf file, but from player directory. Basically an alternative design 2, but solves the "force multi-version" problem. However, when user upgrades or moves the player itself, they might overlook and delete an seemingly unrelated file.

Design 4 could be registry, and store per-player override there. Essentially a weaker design 1. But we don't want registry, so forget about this.

Any other options?

CrendKing commented 3 years ago

A preview of design 3: AviSynthFilter.zip

Create an avisynth_filter.ini in the same directory of the video player. Don't put any section. Key names and value are exactly the same as registry, except RemoteControl, which should be true or false now (registry does not have boolean concept).

If you change anything through settings, new values will be written back to ini, regardless if they are originally read from registry or not. Registry is only updated if ini file can't be written due to system failure, access error, etc.

I think design 3 is the easiest way to support 1) profile per player; 2) simple ini file structure. Suggestions are welcome of course.

Nuihc88 commented 3 years ago

The build above seems to be working as intended; i should however clarify that the concept i have in mind works something like this:

  1. System Wide Mode [Default] - User loads the filter using a player without an empty .ini-file: AviSynth Filter continues using the registry as before.

  2. Semi-Portable Mode - User loads the filter using a player with an empty .ini-file: AviSynth Filter copies initial settings from the registry and then stops syncing regular settings to and from the registry, while still continuing to sync profiles to and from the registry; however, only if the filter is closing cleanly should new profiles get copied back from .ini to registry.

  3. True Portable Mode - User loads the filter using a player with an .ini-file with 'Portable = true' line added: AviSynth Filter no longer writes or reads from the registry under any circumstances.

Regarding Semi-Portable Mode: To avoid accidental data loss and make syncing work reliably, each profile would need to include a 'last modified' timestamp and an unique identifier. To remove a synced profile, you could prefix the timestamp with a 'minus', then in the configuration window, grey out those shared profiles that another player has removed. To restore syncing for a registry copy of the profile, use another player's configuration window to add 'plus' to the timestamp of this player's .ini and make this copy's profile name text bold, then on close remove the 'minus' from the registry.

So in summary:

Regarding True Portable Mode: Sometimes it can be important to keep portable installation's profiles isolated from profiles shared between programs through the system registry. Like let's say for example: the user is running it from an USB stick at a friends house and the friend also has AviSynth Filter installed to their system. One of them could end up messing up the other's profiles unless one copy is guaranteed to have total isolation. (UIDs would also mitigate this problem, but not for users who have previously been sharing profiles.)

I'm not sure if System Wide Mode would ever still be useful once Semi-Portable portable is working, but this is how i envisioned it.

CrendKing commented 3 years ago

Why not simplify as this:

  1. If when the filter is loaded there is no ini, all settings are read from registry. When user changes, settings are written back to registry. It doesn't matter if ini file is created in the midst of the process.
  2. If when the filter is loaded there is ini, all settings are read and write to ini, regardless of if the ini is deleted during the process (new file is created).

Bear in mind that there is no "sync" currently in avsf. We don't unconditionally write anything to registry if user doesn't change anything. Basically lazy mode. So the "Semi-Portable Mode" doesn't make much sense. When writing back all settings are overwritten with the values in memory.

I think the key is simplicity. If two modes can solve the problem, I don't want to introduce a 3rd.

Here is POC you can play with: AviSynthFilter.zip

Example ini:

AvsFile = C:\test.avs
Formats = 2047
OutputThreads = 1
RemoteControl = false
LogFile = C:\avsf.log
Nuihc88 commented 3 years ago

Bear in mind that there is no "sync" currently in avsf. We don't unconditionally write anything to registry if user doesn't change anything. Basically lazy mode. So the "Semi-Portable Mode" doesn't make much sense. When writing back all settings are overwritten with the values in memory.

I think the key is simplicity. If two modes can solve the problem, I don't want to introduce a 3rd.

With Semi-Portable Mode, i was just trying to plan ahead for problems that introducing profile support in the future is likely to bring. Personally i don't mind either way, as copy-pasting profiles across .ini-files manually isn't a big deal for me. However, i think many users would find it more convenient to sync profiles selectively while having a backup and safely-buffer to avoid making all players inoperable with a single bad change. In many ways Semi-Portable Mode would offer best of both System Wide Mode and True Portable Mode, with the only drawback being that non-shared profiles would take up a few kilobytes in the .ini-file.

Registry is only updated if ini file can't be written due to system failure, access error, etc.

This is a problem for true portability; as long as a True Portable Mode is working, most of the other specifics are a non-issue for me.

CrendKing commented 3 years ago

You said yourself that only power users are likely to use the portable mode. I trust if you know how to edit ini, you should know how to Ctrl-C Ctrl-V a backup? Plus everybody should already have regular system-wide backups. Plus currently there are only 2 user-facing settings, and I don't think nor want to introduce tons of settings in the future.

So you think the last upload is ideal?

Nuihc88 commented 3 years ago

Still seems to be creating an empty registry key when editing settings. Tested with PotPlayer 32bit & MPC-HC 64bit.

CrendKing commented 3 years ago

Nice catch.

AviSynthFilter.zip

Nuihc88 commented 3 years ago

Seems to be working perfectly now, i'll let you know if i encounter anything strange. ...If i don't find anything strange in two days, i'll close the issue...

Nuihc88 commented 3 years ago

The build over here seems to have introduced a new hanging bug. Previous build over here is working fine.

Minimum script to reproduce:

AvsFilterSource() Prefetch(1,8) OnCPU(16)

Although less frequently, it also happens with this:

AvsFilterSource() Prefetch(1,1) Prefetch(1,2)

Steps to reproduce:

  1. Load video player.
  2. Load a video.
  3. Seek to middle.
  4. Repeatedly seek backwards in quick succession.

Tested with Potplayer 32bit, MPC-HC (32bit & 64bit) & MPC-BE (32bit & 64bit).

CrendKing commented 3 years ago

When you say "introduce", I suppose the latest upload also has the problem?

Nuihc88 commented 3 years ago

Yep.

CrendKing commented 3 years ago

I tried many previous versions, and so far all of them hang. I've never used the OnCPU function before. Isn't it introduced in the Neo version? How is it supposed to be used? Should it be used in conjunction with Prefetch or mutually exclusive? If I only use one of them it works.

The hang comes from the script clip waiting for its thread pool to terminate, which never happens. I have not changed any logic around that since long time.

I remember avs document mentioned Prefetch() should be placed at the end. Wouldn't that OnCPU invalidate, or worse conflict, with that?

In any case, I think this is a completely separate issue. You should create separate ticket with detailed information (use the template please).

Nuihc88 commented 3 years ago

Ok, portability seems to be in working order, so i'm closing this issue.