daala icon indicating copy to clipboard operation
daala copied to clipboard

Published 20 hours ago • verified publisher icon xiph

Memory leak in FastSSIM when image dimensions are not multiples of 32

Open abhinaukumar opened this issue 4 years ago • 3 comments

In the attached screenshots, I am running FastSSIM with the same image as input and output, of size 627x482. So, we expect that at every level, upon downsampling, the images still have the same entries in corresponding locations. Wherever different, I have printed the column number, row number and the corresponding entries in the two images. The junk values (sometimes negative) at the borders of the image demonstrate the memory leak. The first line of the output shows an attempt to access row index 120 of the image, while the dimensions show that upon downsampling twice, the heights of the image at the first two levels are 482/2 = 241 and 241/2 = 120. So, there should not be an attempt to access row index 120.

daala_fastssim_mem_leak daala_fastssim_mem_leak2

abhinaukumar avatar Apr 21 '20 00:04 abhinaukumar

The main implementations of ssim in the daala repo are the dump_ssim and dump_msssim implementation (I wrote the latter after discovering similar issues in fastssim). Arguably we should delete the fastssim implementation as I don't think there is any reason to use it instead of the other two.

tdaede avatar Apr 21 '20 01:04 tdaede

I see. Considering how much faster it is than dump_ssim and dump_msssim, I think it is worth holding on to. I have created a PR with a simple fix that should help.

abhinaukumar avatar Apr 21 '20 01:04 abhinaukumar

Neat, if it's delivering advantages over the other two we should definitely keep it around.

tdaede avatar Apr 21 '20 03:04 tdaede