pinterf / mvtools

mvtools plugin for avisynth
155 stars 17 forks source link

Crash with MCompensate using a small resolution video and certain combination of blocksize, overlap and divide #19

Closed zorrozork closed 6 years ago

zorrozork commented 6 years ago

Using MAnalyze + MCompensate with a small resolution video causes a crash, the error is "Integer divide by zero at ". The largest resolution where this error occurs was 154x86, anything larger will not trigger the crash. I used BicubicResize to scale the video before passing it to MVTools.

The crash also depends on MAnalyse parameters blocksize, overlap and divide. BlockSizes 32 and 48 crash while smaller ones work. Blocksize 64 also works but the resulting video has a green line at the top and left sides. With blocksize 32 the overlap values that work are 0, 14, and 16 while anything else triggers the crash. Divide must be 0 for the crash to occur. I didn't find any other parameters that affect the crashing.

Attached is a small script which triggers the crash always on the first frame. I tested it on two different sources so it shouldn't depend on the chosen source video. Tests were done in YV12 but the crash did happen with other formats as well.

My system is Windows 10, Avisynth v.2.6.0 ST, 32-bit, MVTools version is the latest v2.7.35.

resized.zip

pinterf commented 6 years ago

Thanks for the report. Yep, overlap window operation assumes at least 3 usable blocks. At specific frame size/block size/overlap values this is not fulfilled. For your test clip now the following error message is given (with some hints): MCompensate: number of Y blocks [2 = (Height-OverlapY)/(nBlkSizeY-OverlapY)] should be > 2! H=68 BlkSizeY=32,OverlapY=4 I'm not doing a release now just because of this parameter checking, maybe we'll have other issues. Or tell me if you need a new build.

zorrozork commented 6 years ago

Thanks for the quick response. I'm wondering if an error message is the optimal behaviour in this case. Isn't it true that no matter what resolution the edge blocks are not able to use the overlap window operation due to only having one neighbour block? And when the size gets smaller then at some point all that is left is these edge blocks. In my opinion that would not be an error in a similar vein than using temporal=true is not an error with a clip with length of one frame.

So would it be difficult in this case to just ignore the overlap window operation and return a result without it? That would be very helpful in my case since I'm trying to optimize a script (using AvisynthOptimizer) where I'm testing different scales for the source along with all kinds of MVTools parameter combinations. I could work around this issue by defining complicated limits for the parameters but it would not be trivial.

pinterf commented 6 years ago

Why not? Overlap option was primarily invented because of the possible quality gain. I can see no problem that when the dimensions are not eligible for overlap operation the whole process would fall back to the normal overlapless case. But it affects not only the analysis section but those parts and filters as well in which vectors are used. Not to mention the divide option so I have to be careful and it will take a little more time than a couple of hours.

zorrozork commented 6 years ago

Yes, take your time and do it right. Because if you don't you-know-who is going to file another bug report... ;)

I assumed you could just silently set overlap=0 when this three block condition is not fulfilled, but apparently it's not that simple. And why does it need three blocks, after all you should be able to overlap two blocks too?

pinterf commented 6 years ago

Done, I think. Could you check this one please: https://drive.google.com/open?id=14H8LY5uMqZGUrwWJhRYxpm-Rt-4We6DM

zorrozork commented 6 years ago

Looks good! No more errors on the test script. I also ran some validations and confirmed the results are the same (gives the same SSIM metric than before). Thanks for your continuing support!