dubhater / vapoursynth-mvtools

Motion compensation and stuff
181 stars 27 forks source link

Compensate gives bogus output #54

Closed mysteryx93 closed 3 weeks ago

mysteryx93 commented 2 years ago

The Compensate function returns green horizontal lines -- completely bogus output. This function works with the 32-bit plugin and gives crap output in 8-bit or 16-bit.

def SpotLess(c: vs.VideoNode, radt: int = 1, thsad: int = 10000, thsad2: Optional[int] = None, pel: int = 2, chroma: bool = True, blksize: int = 8, overlap: Optional[int] = None, truemotion: bool = True, pglobal: bool = True, blur: float = 0.0, ref: Optional[vs.VideoNode] = None) -> vs.VideoNode:
    if radt < 1 or radt > 3:
        raise ValueError("Spotless: radt must be between 1 and 3")
    if not pel in [1, 2, 4]:
        raise ValueError("Spotless: pel must be 1, 2 or 4")

    icalc = c.format.bits_per_sample < 32
    S = core.mv.Super if icalc else core.mvsf.Super
    A = core.mv.Analyse if icalc else core.mvsf.Analyse
    C = core.mv.Compensate if icalc else core.mvsf.Compensate
    pad = max(blksize, 8)

    sup = ref or (c.std.Convolution(matrix=[1, 2, 1, 2, 4, 2, 1, 2, 1]) if blur > 0.0 else c)
    sup = S(sup, hpad=pad, vpad=pad, pel=pel, sharp=2)
    sup_rend = S(c, hpad=pad, vpad=pad, pel=pel, sharp=2, levels=1) if ref or blur > 0.0 else sup

    th1 = thsad
    th2 = (thsad + thsad2)/2 if radt==3 else thsad2
    th3 = thsad2

    bv1 = A(sup, isb=True, delta=1, blksize=blksize, overlap=overlap, chroma=chroma, truemotion=truemotion, pglobal=pglobal)
    fv1 = A(sup, isb=False, delta=1, blksize=blksize, overlap=overlap, chroma=chroma, truemotion=truemotion, pglobal=pglobal)
    if radt >= 2:
        bv2 = A(sup, isb=True, delta=2, blksize=blksize, overlap=overlap, chroma=chroma, truemotion=truemotion, pglobal=pglobal)
        fv2 = A(sup, isb=False, delta=2, blksize=blksize, overlap=overlap, chroma=chroma, truemotion=truemotion, pglobal=pglobal)
    if radt >= 3:
        bv3 = A(sup, isb=True, delta=3, blksize=blksize, overlap=overlap, chroma=chroma, truemotion=truemotion, pglobal=pglobal)
        fv3 = A(sup, isb=False, delta=3, blksize=blksize, overlap=overlap, chroma=chroma, truemotion=truemotion, pglobal=pglobal)

    bc1 = C(c, sup_rend, bv1, thsad=th1)
    fc1 = C(c, sup_rend, fv1, thsad=th1)
    if radt >= 2:
        bc2 = C(c, sup_rend, bv2, thsad=th2)
        fc2 = C(c, sup_rend, fv2, thsad=th2)
    if radt >= 3:
        bc3 = C(c, sup_rend, bv3, thsad=th3)
        fc3 = C(c, sup_rend, fv3, thsad=th3)

    ic =          core.std.Interleave([bc1, c, fc1])           if radt == 1 else \
             core.std.Interleave([bc2, bc1, c, fc1, fc2])      if radt == 2 else \
        core.std.Interleave([bc3, bc2, bc1, c, fc1, fc2, fc3])

    output = core.tmedian.TemporalMedian(ic, radius=radt)
    return output.std.SelectEvery(radt*2+1, radt)  # Return middle frame

Also, any chance you could implement MultiVec features? Allowing radius greater than 3

This would allow writing SpotLess with much less code

adworacz commented 3 weeks ago

I've confirmed this bug locally. After some debugging, it's related to overlap being set to None. If you specify overlap=4, then the green/bogus output goes away. I suspect that overlap isn't being calculated properly by default.

I'm going to read through the code now to see what I can see.

adworacz commented 3 weeks ago

I suspect the issue is somewhere in here: https://github.com/adworacz/vapoursynth-mvtools/blob/7ebc72e745ef4575dfeb2084ff4ce48e7f6eabc1/src/MVCompensate.c#L225-L257

I'm not 100% sure though, but the Avisynth and SF versions of MVtools have a few distinct differences, not the least of which is taking subsampling into account:

More research is required, as I'm not exactly sure what this code is even doing, aside from a general idea that it's making strategic copies.

adworacz commented 3 weeks ago

I found the root cause of the issue, and it's a line that @sekrit-twc added to the copy code a few years ago:

It was added in this commit: https://github.com/dubhater/vapoursynth-mvtools/commit/093cb0927e2dfd0bd4de40b6c51b033f255597a3

That commit was entirely focused on SAD, but the CopyCode is used a few places in MVTools, with Compensate being one of them.

I'm not entirely sure of its purpose, so I need to do a bit more reading to see what's safe to remove/alter. But simply deleting that line fixes Compensate with overlap = 0.

Stefan-Olt commented 3 weeks ago

Great work figuring this out!

adworacz commented 3 weeks ago

Great work figuring this out!

Sure thing! It was bugging me since everything looked pretty legit when I first glanced at the code. Glad I was able to hunt it down with a little bit of effort.