[Neo] FFT3D / DFTTest

[MeteorRain]

This thread serves Neo-FFT3D and Neo-FFTTest.

Neo filters family is not to be confused with AviSynth-Neo. Neo filters family is not affiliated in any way with AviSynth-Neo.

[Neo] FFT3D → GitHub

FFT3DFilter is a 3D Frequency Domain filter - strong denoiser and moderate sharpener. It was originally written by Alexander G. Balakhnin aka Fizick, and later modified by martin53 for AviSynth 2.6 and later modified by Ferenc Pintér aka pinterf for further improvement, high bit depth, and more. Kudos to them for creating and improving this fantastic tool.

Difference compared to pinterf/FFT3DFilter

• Dual interface, supporting AviSynth+ and VapourSynth
• Removed YUY2
• Removed multiplane options, added y, u, v options for plane control
• Removed existing 3DNow and SSE code
• Fixed an incorrect coefficient in Apply2D
• Added SSE, AVX and AVX512 routine
• Added multi-threading, both internal and AVS-MT prefetch

Result bit-identicalness

• Neo-FFT3D C - FFT3DFilter C: identical except for a few situations
• Except: bt=2 and sharpen and dehalo where the incorrect coefficient is fixed
• Except: bt=4, 5 where the floating point numbers are added in different way ( (fp2r + fpr) + (fcr + fnr) in Neo-FFT3D, (fp2r + fpr + fcr + fnr) in FFT3DFilter ) resulting rounding error
• Except: bt=0 and pfactor > 0 causing crash, need your investigation and fixes
  ===== ===== ===== =====
• Neo-FFT3D SSE - C: identical
• Neo-FFT3D AVX - C: identical
• Neo-FFT3D AVX512 - C: theoretically identical

[Neo] DFTTest → GitHub

DFTTest is a 2D/3D Frequency Domain denoiser. It was originally written by tritical, and later modified by Ferenc Pintér aka pinterf for further improvement, high bit depth, and more. VapourSynth-DFTTest was ported to VapourSynth interface by HolyWu with further clean ups and efficient SSE2 and AVX2 SIMD routines. Kudos to them for creating and improving this fantastic tool.

Difference compared to pinterf/dfttest

• Dual interface, supporting AviSynth+ and VapourSynth
• Parameter names and opt levels follow VapourSynth-DFTTest
• Removed Y, U, V options, added y, u, v options for plane control
• Removed hacked 16 bit I/O
• Replaced internal multi-threading with C++ Parallel STL, removed Windows specific APIs
• Improved precision of floating point reciprocal using Newton Raphson method
• sigma/sigma2/sbsize/sosize/tbsize etc. has different default value

Difference compared to HomeOfVapourSynthEvolution/VapourSynth-DFTTest

• Dual interface, supporting AviSynth+ and VapourSynth
• Added dither option from AVS counterpart
• Added internal multi-threading
• Replaced division by Newton Raphson method reciprocal in SSE2
• Replaced reciprocal approximation by Newton Raphson method in AVX2

Result bit-identicalness

• Neo-DFTTest C - (AVS)DFTTest C: identical except when nlocation is provided, due to order of floating point multiplication
  Neo-DFTTest: data = data * (scale * (wscale2 / wscale) * alpha)
  (AVS)DFTTest: data = data * scale * (wscale2 / wscale) * alpha
• Neo-DFTTest C - (VS)DFTTest C: identical
  ===== ===== ===== =====
• Neo-DFTTest SSE - C: identical except when dither > 2, due to rounding errors in dithering
• Neo-DFTTest AVX - C: identical except when dither > 2, due to rounding errors in dithering

[feisty2]

I took a peek at your dual interface, I think the argument fetching part, particularly the part dealing with array parameters could be improved.
I don't think it's a good idea to assume array parameters are std::vector<T>, and to erase the vector while fetching the arguments.
people might not use vector to store array parameters, it could also be std::array or a pointer to some pre-allocated memory or something else for various reasons.
it's also not a good practice to erase the parameter container, the container might hold more default values than the number of values specified by the user. you lose all default values when you erase the container and the user loses the flexibility to specify a partial array, leaving the rest of default values untouched.

I think it's best if the argument object itself behaves like a container (it does not necessarily to be an actual container) and let the filter developer decide how the arguments should be handled.

// this is bad
args.read("sigma_array", sigma_array);

// this is better, and allows the user to specify a partial array
for (auto x : range{ args["sigma_array"].size() })
    sigma_array[x] = args["sigma_array"][x];

// or if you don't need the flexibility to specify a partial array
for (auto sigma : args["sigma_array"])
    sigma_array.push_back(sigma);

the dual interface also doesn't yet seem to handle frame properties for vaporsynth, I have completed this functionality and a Rec601ToRGB example is also available. if you plan to add support for frame properties, the syntax should be the same as, or at least very similar to the aforementioned "arguments" thing for the sake of consistency.

[MeteorRain]

Thanks for taking your time to look at the code. You are absolute right about parameter processing. Part of those code was copied from my previous projects years ago, and there's lots of room for improvement.

At this moment I'm trying to make scaffolding as quick as possible (this wrapper was made after your wrapper, so you know it's still very young). And my hope is if anyone has interests they can send merge requests to improve it. Of course, at this moment I'd aim at C++17 and not 20 or 23. Let's wait until C++20 or 23 becomes mainstream before moving forward.

Your point about array makes a lot sense. I'll need some time to think about it. For now vectors are adequate to my needs, but I'll try to make it better. As you suggested, begin() and end() would be a good start.

[MeteorRain]

Please test dfttest beta1.

Example: neo_dfttest(ftype=0, sigma=8.0, sigma2=12.0, pmin=0.0, pmax=500.0, sbsize=4, smode=1, sosize=2, tbsize=1, tmode=0, tosize=0, opt=0, zmean=True, y=3, u=3, v=3, nlocation="0,0,20,40", slocation="0.0:3.0, 0.3 14.0 1.0,10.0", ssystem=1, threads=6)

Syntax is similar to original AVS dfttest except:

• Y/U/V=true/false is now y/u/v=3/2/1 where 3=process, 2=copy, 1=garbage.
  
• nstring is now alpha and nlocation.
  For example, (nstring="0,0,20,40 a=5.2") is now (nlocation="0,0,20,40", alpha=5.2).
  
• sstring is now ssystem and sstring.
  For example, (sstring="$ 0.0:3.0 1.0:10.0") is now (sstring="0.0,3.0,1.0,10.0", ssystem=1).
  
• sstring, ssx, ssy, sst supports any delimiter of "," or " " or ":".
  For example, (ssx="0.0:3.0 1.0:10.0") equals (ssx="0.0,3.0,1.0,10.0").
  
• opt is now 0=auto, 1=C, 2=SSE2, 3=AVX2.
  
• No hacked 16bit IO. Only native HBD is supported.
  
• threads=X only applies to SSE2 and AVX2 routine, C code always runs at single thread.
  
• sigma and sigma2 defaults to 8.0. Manually set to 16 to match original result.
  
• C/SSE2/AVX2 now produces bit identical results except for random dithered 8bit, which gets some rounding errors.

On VapourSynth, nlocation, slocation, ssx, ssy, sst are still arrays.

Let me know any issues.

[StainlessS]

which gets some rounding errors.

Maybe in future use term "rounding differences", allay [lessen] user worries about errors.

[MeteorRain]

dither > 2 uses Mersenne Twister random number generator to dither data, so outputs would never be the same every time. Whether there are rounding "errors" or "differences" no longer matters, I think.

[StainlessS]

Its just that some use a translator to read docs, and sometimes translators can change meaning a little and may result in much exaggerated significance.

[ChaosKing]

Originally Posted by MeteorRain

Please test dfttest beta1.

Tested a bit in VS:
- 8-16 + 32 bit depth works fine
- clip.neo_dfttest.DFTTest(ftype=0, sigma=8.0, sigma2=12.0, pmin=0.0, pmax=500.0, sbsize=4, smode=1, sosize=2, tbsize=1, tmode=0, tosize=0, opt=0, zmean=True, nlocation="0,0,20,40", slocation="0.0:3.0, 0.3 14.0 1.0,10.0", ssystem=1, threads=6)
produces

Core freed but 5 filter instance(s) still exist
Core freed but 5 filter instance(s) still exist
Core freed but 537600 bytes still allocated in framebuffers
Core freed but 537600 bytes still allocated in framebuffers

EDIT:
The parameters above triggers

ValueError: invalid literal for int() with base 10: '0,0,20,40'

- No warnings with a simple

neo_dfttest.DFTTest(sigma=8, sigma2=8)

- speed:
-- vapoursynth dfttest 7.9 fps
-- vapoursynth neo dfttest 5.5 fps

[MeteorRain]

Yea I haven't tested thoroughly on VS.

On VS nlocation and slocation and ss? are still arrays. I should have said those changes only apply to AVS. On VS array inputs are the same as vapoursynth-dfttest.

I'll look on memory leaks later.

Speed difference is probably because I've set it as serialized filter (so single threaded from VS point of view) for debugging purpose. Will change to parallel on release.

[ChaosKing]

So this would be correct?

neo_dfttest.DFTTest(nlocation=[0,0,20,40])

It crashes immediately

[MeteorRain]

beta2
Fixes fmParallel
Fixes nlocation crashes

When nlocation does not crash, there seems no memory leaks found.

[ChaosKing]

Originally Posted by MeteorRain

beta2
Fixes fmParallel
Fixes nlocation crashes

When nlocation does not crash, there seems no memory leaks found.

Works good now. The only thing I found was a msg

Core freed but 1 filter instance(s) still exist
Core freed but 1 filter instance(s) still exist

if you specify an incomplete parameter like this DFTTest(nlocation=[8]). Idk if this counts as an error.

[MeteorRain]

Supplying incorrect parameters may trigger an error with some temporary data not being freed correctly. I'll check where it leaks, but I don't think it's a big issue.

[real.finder]

I did some tests

ColorBars(width=640, height=480, pixel_type="yv12")
admfilter(custom_filter="neo_fft3d(Sigma=(adSigma+1.0)/f)")
Prefetch(4)

with https://forum.doom9.org/showthread.p...72#post1904672

I get


and with https://forum.doom9.org/showthread.p...16#post1906016

it's just freeze! and sometimes



and sometimes same as the 1st one

[MeteorRain]

I can't get your script work. I assume I should use "Advanced Denoising and anime bob v1.87"?

I get -- I don't know what 'MotionRampadc' means.

[real.finder]

Originally Posted by MeteorRain

I can't get your script work. I assume I should use "Advanced Denoising and anime bob v1.87"?

I get -- I don't know what 'MotionRampadc' means.

you only need https://github.com/realfinder/AVS-St...Denoising.avsi and https://github.com/realfinder/AVS-St...RF_Shared.avsi

and from that error you get seems you didn't use any runtime MT fixed avs+ (both test avs+ builds I post)

edit: also you maybe missed this GRunT update pinterf made years ago

[MeteorRain]

Okey grunt it is. Now I can see it crashed. Let me check.

That's easy to find. Filter initialization was executed concurrently, crashed the fftw library. I don't know if initialization should be thread safe or not. I'll add a mutex then.

[real.finder]

Originally Posted by MeteorRain

Okey grunt it is. Now I can see it crashed. Let me check.

That's easy to find. Filter initialization was executed concurrently, crashed the fftw library. I don't know if initialization should be thread safe or not. I'll add a mutex then.

DFTTest was also crashed, but pinterf fix it, didn't try neo DFTTest yet, but last DFTTest by pinterf work

edit: it fixed since this one https://forum.doom9.org/showthread.p...77#post1904777

[MeteorRain]

Yes my solution is similar to that. Usually initialization is single threaded, so you don't have to think about thread safety. In this case multiple instances were initialized so there needs to be a global mutex there. pinterf used static global, and I used a static local, the result would be similar.

beta3 thread-safe initialization.

[real.finder]

Originally Posted by MeteorRain

beta3 thread-safe initialization.

it's seems work

btw, how you end-up with this case? https://forum.doom9.org/showthread.p...43#post1905043

and aside from that, do you plan to did the same with all/most/some https://github.com/HomeOfVapourSynthEvolution ? (Dual interface with neo renames)