dubhater / vapoursynth-mvtools

Motion compensation and stuff
181 stars 27 forks source link

Fix bug in Cross Search. #72

Closed adworacz closed 1 month ago

adworacz commented 2 months ago

This is a very old bug, possibly back to the dawn of this plugin.

I originally found this in the Avisynth version, and I've been meaning to port the fix back here.

Ref: https://github.com/pinterf/mvtools/issues/56

Stefan-Olt commented 1 month ago

Great! Are there any other fixes (or new features) that could be ported from the AviSynth version?

adworacz commented 1 month ago

Oh I’m sure there’s loads, not the least of which is better high bit depth support for MMask, block sizes of 3,6,12, etc, and a bunch more.

But all of it takes time, and the code bases have diverged a fair bit.

I started working on the API4 port last night, and it seems to be going okay. It will take me a while longer to have something worthy of a PR, but we’ll get there.

Stefan-Olt commented 1 month ago

Some plug-ins like ffms2 build VapourSynth and AviSynth plugins from the same source. Would that be a possible option for mvtools as well to prevent diverging code bases or are the APIs to different (for example regarding multithreading)? Or do the mvtools creators don't have any interest in supporting VapourSynth?

adworacz commented 1 month ago

You ask good questions, but the answers are a little difficult.

From my last audit of this code base, it looked like the code diverged in a few key areas, with the biggest one being multithreading support, as you call out. The multithreading implementations in AVS vs VS are quite different, and the AVS version of MVTools makes extensive use of custom threading constructs. All of that was ripped out (rightfully so) in this VS implemenation.

While the AVS and VS APIs are different, as you call out, a number of plugins support both. In theory, it may be possible to get an implementation of MVTools that supports both AVS and VS, but as I call out above, the multithreading differences are a larger hurdle than the API differences.

However, my understanding of the AVS multithreading model is a bit limited, so its entirely possible that parts of the AVS MVtools' multithreading implementation are now unnecessary. I just don't know for sure.

If I'm honest, my long term hope is to rewrite MVtools from scratch in a cross platform (OS and architecture) way. This may include AVS support eventually, but I'd really need to understand its multithreading needs a lot more before committing to it.

It will be a while before I can devote the kind of time required though, and MVTools is quite possibly the most complicated plugin that exists in the AVS/VS space, so it won't be an overnight affair either.

Stefan-Olt commented 1 month ago

This is what I expected, because MVTools being a really complicated plugin and AVS multithreading being really complicated (at least back then). Great to hear that you want to put so much effort into mvtools, maybe the people working on AVS mvtools are also interested in a rewrite (especially when for example the MT code is obselete and it can be done in a better way)