Khanattila / KNLMeansCL

An optimized OpenCL implementation of the Non-local means de-noising algorithm
GNU General Public License v3.0
107 stars 22 forks source link

Use newVideoFrame2 for free plane copy #12

Closed myrsloik closed 9 years ago

myrsloik commented 9 years ago

https://github.com/Khanattila/KNLMeansCL/blob/master/KNLMeansCL/KNLMeansCL.cpp#L691

You don't have to use vs_bitblt to copy planes, you can use newVideoFrame2 and you simply reference the old planes.

See this line for an example: https://github.com/vapoursynth/vapoursynth/blob/master/src/core/lutfilters.cpp#L310

Khanattila commented 9 years ago

I got it.

myrsloik commented 9 years ago

Btw, you should pass the source frame instead of NULL to newVideoFrame(2) too or the frame properties won't be propagated properly. Did I annoy you yet?

myrsloik commented 9 years ago

Oh and I think https://github.com/Khanattila/KNLMeansCL/blob/master/KNLMeansCL/KNLMeansCL.cpp#L712 is a double free of the src frame since you free it again later

Khanattila commented 9 years ago

The basic structure I had copied from another plugin. I must have seen NULL and then left. Btw do a thing like this:

src = vsapi->getFrameFilter(n, d->node, frameCtx);
src = vsapi->getFrameFilter(n-1, d->node, frameCtx);
vsapi->freeFrame(src);
src = vsapi->getFrameFilter(n, d->node, frameCtx);
vsapi->freeFrame(src);

It does not create memory leaks?

myrsloik commented 9 years ago

Every getFrameFilter() must have a matching freeFrame() call. I looked at your code again and I guess I got confused by the variable reuse. I think it doesn't leak frames.