search

AlexHarker / AHarker_Externals

A Set of 80+ Externals for a variety of tasks in MaxMSP.
BSD 3-Clause "New" or "Revised" License
68 stars 5 forks source link

Breaking change (allow old behavior? (sfm & fftparams)) #20

Open rconstanzo opened 1 year ago

rconstanzo commented 1 year ago

After a bit of troubleshooting this morning I figured out why some old code (in C-C-Combine) wasn't working properly. The first part of this is that sfm now reports in dB, which is more useful, but not the same as before, and not the same as what the documentation says at the moment("a value between 0 1 - 1 indicates a very flat spectrum and hence more noisy..."). I think dB is a more useful metric here, and in my patch I was doing some atodb stuff with clamping to remove INFs, but maybe having an option to revert to old behavior (or at least stating the new behavior in the documentation) would be useful.

After going to reanalyze my files with this in mind I was greeted with a console window full of descriptors~: zero length file or segment. It seems that the updated version takes the fftparams as a minimum segment length so if I send descriptors~ a message of analyze buffer 1 0. 40. with fftparams 2048 128 (@44.1k) I get the zero length file or segment error. If I change my message to analyze buffer 1. 0. 46.44 then it works properly. This makes logical sense as that is how big that segment needs to be for the fftparams to analyze a full frame, but it does break existing patches as this was not clamped previously. I don't know if the previous version just zero-padded the end of something, but it did return values.

I can easily change my local code to take this in mind, but mainly thing that there's quite a bit of this code out there in the world which will no longer work.

AlexHarker commented 1 year ago

[It would be really helpful if you could split separate issues in future when there are multiple separate things to track - thanks. In this case that might have been one general one (breaking changes) and two specific ones]

descriptors objects are still alpha for these kinds of reasons - see here: https://github.com/AlexHarker/AHarker_Externals/blob/main/README.md

There are some things in the descriptors objects that were just plain wrong, and I won't be supporting old incorrect behaviours (e.g. incorrect calculations), but for some things there is an argument to either have the ability to support it, or notes as to what to do to adapt patches (docs are as yet not done, due to other commitments). There was a lot - full list of notes presented so as to make sense to my brain about 12 months ago - https://github.com/AlexHarker/AHarker_Externals/blob/main/descriptors/descriptors%7E/Notes.md.

sfm - Pretty sure there's now an additional parameter (beyond the documented ones) that is defaulted to 1 (on) for dB conversion - so just set it to 0 (off). I imagine that would have been obvious to you on reading this line of code which you definitely would have found really easily:

https://github.com/AlexHarker/AHarker_Externals/blob/ae924e91d100fd9b4ab862246b00cf8e5943fbb9/descriptors/descriptors%7E/modules/modules_spectral.hpp#L61

In worse news I think the numbers you now get will still be different to old numbers in some way that I don't exactly remember right now [edit - because of the use of power and not amplitudes].

Edge Handling - There's a setting to do with this in the underlying engine, but right now you can't interact with it at all right now, and it defaults to non-padding behaviour (none). There are three settings - none (only allow full frames), allow_overlap (allow reading frames starting in the given time but ending outside) and zero_pad (the old behaviour - I think). So this is on a to do list.

rconstanzo commented 1 year ago

Indeed, I'll break out future ones. Let me know if you want me to create new/sub issues for these. Initially I wanted to mention the window size thing, but in testing while creating the issue, the rest of the it got away from me.

Totally makes sense to not carry forward bad/broken computations, which will more-than-certainly lead to having to re-crunch numbers. Same goes for the sfm change (though that switched from me having to remove INFs to then giving me NANs, but that was due to me having another atodb in the mix, which spat out the NANs).

I guess the most salient issue, in this issue, is that of edge handling, as that was breaking the patch in a way that took me a lot of poking to narrow down, whereas the other problems gave me numbers, that didn't line up too well.