CrendKing / avisynth_filter

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

Avisynth 3.5 vs 3.6+ #6

Closed chainikdn closed 3 years ago

chainikdn commented 3 years ago

Right now we're building with the latest AVS 3.6 headers. This resulting in AVSF is not working with AVS 3.5 installed solely due to AVISYNTH_INTERFACE_VERSION was bumped from 6 to 8. The filter doesn't depend on anything specific to the updated interface.

We (SVP) now have a problem: ffdshow doesn't work with AVS 3.6+ (https://github.com/AviSynth/AviSynthPlus/issues/178) and AVSF doesn't work with AVS 3.5.

So, may I suggest building AVSF with headers from 3.5.0 release? Again, I don't see any difference for AVSF now.

CrendKing commented 3 years ago

I could. What's you guys' long term plan? Staying on 3.5 forever, since ffdshow is unlikely to update anymore, or only temporary?

chainikdn commented 3 years ago

Long term - forget about ffdshow, I hope. step 1: ffdshow is still the default "engine" with AVSF installed alongside step 2: AVSF is default, ffdshow remains for compatibility step 3: AVSF only

CrendKing commented 3 years ago

I'll build with 3.5 next version.

chainikdn commented 3 years ago

0.7.5 - "Only a single prefetcher..." on AVS 3.5 when switching an audio track (MPC-HC, PotPlayer)

CrendKing commented 3 years ago

I went to destroy the whole IScriptEnvironment and unload the dll, stop all worker threads, then reload them back. The crash is still there. I don't know what persisted through the process.

CrendKing commented 3 years ago

@chainikdn I think I know why there's problem with the new filter version and tracking switching.

As I understand, the C++ interface of AviSynthPlus basically assumes there will be only one IScriptEnvironment. I say this because AVS_linkage is a global variable, and it is linked to the avs environment. So naturally I initialize it at the beginning of all logic and never touches it again.

When the player first loads the filter, it put it into the filter graph and hold a reference. The filter graph lives until the player shuts down. Here is the problem: when you switch a track, it instructs the filter to flush and release everything, via Inactive(). However, it never release the filter itself. Then it creates another instance of the filter, add to filter graph, and use the new instance from then on. Both instances get released with the graph at player shutdown. At this time, we have inconsistency: all filter logic uses states from the new instance, but all AVS cache still references to the old instance.

Obviously, there are ways to workaround:

  1. Make the filter a singleton. This should theocratically fix the problem, but the doc specifically says CreateInstance() should create new instance.
  2. Release filter at Inactive(). It works but I don't know if this would create new problem, as the reference count are wrong now.
  3. Delete avs environment at Inactive() and recreate new environment. Seems to be the cleanest solution, but because the old filter still need refers to the old AVS_linkage, when destroying the old filter undefined behavior could happen.

I don't know why does the filter graph not release filters when switching track. It could also be a bug in my code, like I forgot to release in a function.

chainikdn commented 3 years ago

... just wanted to mention that it starts to work after seeking. play -> switch tracks -> error -> seek -> ok

CrendKing commented 3 years ago

I fixed something in https://github.com/CrendKing/avisynth_filter/issues/18#issuecomment-719144423. Try it.

chainikdn commented 3 years ago

Ok, I just returned from a holiday trip... installed 0.8.0, and it now hangs when switching tracks regardless the Avisynth version used.

CrendKing commented 3 years ago

Oops, I miscounted this use case after fixing another bug. Sorry there were several issues at hand at the end of v.0.7.x line. It's like those "leaking bucket" rhetoric: filling up one hole opens another 😃.

Try AviSynthFilter.zip. Also try other cases, in case another hole is open.

chainikdn commented 3 years ago

thanks, looks much better