Bug: maxFFTsize seems not to be checked when resizing out of bound upwards

[tremblap]

Please tell us what you were doing! You can include code and files by drag and dropping them into the text area.

I am messing about with FFT, at times randomly asking for a window larger than the default maxFFTsize

{ var source = SinOsc.ar(222,mul: 0.1); [source, FluidSTFTPass.ar(source,LFNoise0.kr(1).range(10,10000))] }.play

when the window goes above 8192 I get an assert crash.

What was the expected result?

to give a warning and cap

What was the actual result?

a crash

What operating system were you using?

Mac

Operating system version

Monterey

FluCoMa Version

latest

[tremblap]

ok good bad news - @AlexHarker asked me to check if it was the same in max. I can confirm it is:

----------begin_max5_patcher----------
591.3ocwW1riaBCDG+L7TX4iszHLf4idpuGUUUNfSVuhXi.SZVsZyydMCP1M
6RTHDusWvhIi8+42LlwNO65fWqNvavnui9Ixw4YWGGvTmAmg2cv6XGxKYMfa
3c7lF1VN1q+2z7CZv9lM5FtVKjaaPzDZvnCkBIOW0JAuBGLJa2IjkbMrjACF
qX57GLy+207bceHERxV46gBhncCT+tmQFSne85BoZ0iqDYvZuI8SU79kAiOM
AQADsp0O9svLbmsWbc6d3MS9k7+Xl7GvuplWwkEn2jFvSAK4xvFjP53ijDCC
98nGXIZihsHse0Dd3arVNhGMnaHLZozI51JMEfTKBXMSVn1gxxVNmAocCoDq
yYjE4bGWWqLESzOX4ZwdNhrTb8gBJgDtTdWyjamFXhEA9KGQ9qtYHIgPCnfL
3SynfkxXiXqjUNc2nz++Tlj8oSYhEorfkebgUxvPfwvjqvn+DHXyFo4OkWxO
Zh.+EVrHovghwoeFEKa1oYSYqnXUiditx34wa8vww8lTXH8Z6MCtBtdW5NAm
cHB3AbCl2cEIH.6reddnQ0VmOpzPqKOzqwXAuwby.lVnju0ondmlLWOakhmg
Rla7X1wbxIUcAuF1k+OPZ5YNcRZx8IMcN42XajeiliRTKnj4ffYjNsASllwy
TIx8pzbpSigy8ozbpSuKEaoMiyS5K7cf+GZ6vpp1yqaFlNnpoQ6iJv8TO3Ug
r+UJ7ZMeuXz+dKrZSeTsoIZaMDo3CwQ39opLBKaECXa30HIzDWxL+EuJVOZP
ud2Wb+Khz7yN
-----------end_max5_patcher-----------

[AlexHarker]

It's possible/likely that these crashes have different causes.

For max what you are seeing here is another example of the effects of flucoma/flucoma-max#386.

If I compile against my work-in-progress on thread safe parameters the crash/assert disappear. Unless I am very much mistaken SC is single-threaded.

[tremblap]

maybe but they do yield at the exact same assert with the exact same code, so I'm curious.

[AlexHarker]

That would have been easier to be sure of if you'd actually given the line for the assert. I am not set up yet to test in SC, but I am fairly sure that the source of the crash is different, because there should be no threading concerns in SC, and if max is made thread safe the assert does not happen.

[AlexHarker]

That would have been easier to be sure of if you'd actually given the line for the assert. I am not set up yet to test in SC, but I am fairly sure that the source of the crash is different, because there should be no threading concerns in SC, and if max is made thread safe the assert does not happen.

[tremblap]

line 51 of core/algo/stft.h stft - resize
assert(windowSize <= mMaxWindowSize &&
"STFT: Window Size greater than Max");

in Max. In SC, same:

Thread 10 Crashed:: com.apple.audio.IOThread.client
0 libsystem_kernel.dylib 0x7ff809a46ffe __pthread_kill + 10
1 libsystem_pthread.dylib 0x7ff809a7d1ff pthread_kill + 263
2 libsystem_c.dylib 0x7ff8099c8d24 abort + 123
3 libsystem_c.dylib 0x7ff8099c80cb __assert_rtn + 314
4 FluidSTFTPass.scx 0x1068955a1 fluid::algorithm::STFT::resize(long, long, long) (.cold.1) + 33 (STFT.hpp:51)
5 FluidSTFTPass.scx 0x10688f82c fluid::algorithm::STFT::resize(long, long, long) + 364 (STFT.hpp:51)
6 FluidSTFTPass.scx 0x10688f184 fluid::client::STFTBufferedProcess::setup(fluid::client::FFTParams) + 404 (BufferedProcess.hpp:320)
7 FluidSTFTPass.scx 0x10688eae5 void fluid::client::STFTBufferedProcess::process<float, void fluid::client::stftpass::BaseSTFTClient::process(std::__1::vector<fluid::FluidTensorView<float, 1ul>,

[AlexHarker]

setParameterValuesRT() places no restriction such that the mMaxFFTSize field of an FFTParams can't be replaced. So what is happening right now is that the max fit is constantly being reset to -1 and then the code generates what looks like a sensible size based on the window size that fails further down.

There are multiple places where we could consider this an error but the side-effects of changing each one are opaque to me (some would be in core and some would be in wrappers). For instance, we could consider that:

flucoma-core/include/clients/common/ParameterTypes.hpp

Line 277 in c8ff5a9

mMaxFFTSize = p.mMaxFFTSize;

and

flucoma-core/include/clients/common/ParameterTypes.hpp

Line 286 in c8ff5a9

mMaxFFTSize = p.mMaxFFTSize;

are incorrect, because these state that the max FFT size is mutable once the object has been constructed. However, that might be necessary. I wonder if making it mutable only if not -1 would work, but checking this in practice is complex.

I think the other stuff this relates to is covered by LongRuntimeMaxParam, which looks to me like how all other initialisation max, vs non-initialisation params are dealt with. I am unsure if there is a way of specifying what must be set at initialisation time, or not (other than this type and also the max fit size parameter which one assumes this applies to). [Are there any others?] As far as I can see LongRuntimeMaxParam requires the wrappers to ensure correct containing (I'm not sure I like this pattern, but it seems to be the design that has been made).

Sorry for the dump of thoughts, but this is a complex problem and this is not my code. I am doubtful of my ability to be confident in any fix without substantial time input on investigating further. It may be that @weefuzzy can guide us more quickly, but otherwise the quick options are somewhat shot in the dark with the assumption that it will not have bad side-effects (and possibly lots of testing by someone else) or we leave as is knowing the bug exists...

[AlexHarker]

OK - I can confirm that fixing in FFTParams operator=() is not viable without significant rework because these are used elsewhere.

Max fixes this by using x->mInitialized and reading the max FFT size out if already initialised. A similar fix for SC would be possible, but I cannot say it would be the design choice that I would make, as to my mind the client layer should be handling such issues. However, because of the ways constraints are currently designed it is hard to see how that could be done.

[tremblap]

thanks for the investigation. I'm relieved it is not a HISStools stft problem. We'll see how we fix it or gaffer tape it in the 3 wrappers for now I reckon

[AlexHarker]

The HISTools FFT is just an FFT. The STFT part is provided by flucoma.