pinterf / masktools

MaskTools v2 fork
Other
48 stars 11 forks source link

mt_convolution crash on x64 with width=1 YV24 or width=2 YV12 at U=V=process #3

Closed pinterf closed 6 years ago

pinterf commented 6 years ago

Reopened here from https://github.com/pinterf/AviSynthPlus/issues/16

x64 version crash for YV12 (Y plane width = 2, U and V plane width = 1), but x86 version doesn't crash

BlankClip(120000,width=2,height=480,pixel_type="yv12")
mt_convolution("0 1 0", "1 1 0", y=3, u=3, v=3)

x64 version crash for YV24 (all planes have width = 1), but x86 version doesn't crash

BlankClip(120000,width=1,height=480,pixel_type="yv24")
mt_convolution("0 1 0", "1 1 0", y=3, u=3, v=3)

It seems x86 is fine?

pinterf commented 6 years ago

x86 did not crash, but it's only pure luck. You cannot do a 3x3 convolution on a width<3 clip. In masktools2 there was no check for this condition (plane dimensions are >= convolution size).

TbtBI commented 6 years ago
s=last
cLeft=2
StackHorizontal(Crop(cLeft, 0, cLeft, 0).mt_convolution("0 1 0", "0 1 1", y=3, u=3 , v=3),Crop(cLeft, 0, 0, 0))
a=last
s
cLeft=2
StackHorizontal(mt_convolution("0 1 0", "0 1 1", y=3, u=3 , v=3).Crop(cLeft, 0, cLeft, 0),Crop(cLeft, 0, 0, 0))
Compare(a, channels="yuv")

This shows no difference for uv/y planes. Doesn't that mean convolution is done even if plane dimensions are <= convolution size?

a=last
cLeft=2
StackHorizontal(Crop(cLeft, 0, cLeft, 0),Crop(cLeft, 0, 0, 0))
s=last
a
cLeft=2
StackHorizontal(Crop(cLeft, 0, cLeft, 0).mt_convolution("0 1 0", "0 1 1", y=3, u=3 , v=3),Crop(cLeft, 0, 0, 0))
Compare(s, channels="uv") #or Compare(s, channels="y")

This shows difference for uv/y planes.

pinterf commented 6 years ago

Yes, unfortunately convolution is done even when dimensions are too small.

When plane dimensions are less than convolution size heap/memory became corrupted. Filter tries to write to the array index -1 when the array itself starts from index 0. While x64 version was crashed (for me: silently exited w/o any error message), x86 version somehow survived but the memory corruption was still there. Since then there is a 2.2.17 version which has already a check and will show an error when - for example - a 3x3 convolution would be done on a plane with width<3

TbtBI commented 6 years ago

Thanks for the explanation and your time. I guess the issue is resolved already.

pinterf commented 6 years ago

Yes, by giving proper message