search

ar1st0crat / NWaves

.NET DSP library with a lot of audio processing functions
MIT License
462 stars 71 forks source link

Alternatives to float[] (opened as 'Implementing IEnumerable<float> on DiscreteSignal') #3

Closed GataullinRR closed 4 years ago

GataullinRR commented 5 years ago

What about implementing IEnumerable on DiscreteSignal?

GataullinRR commented 5 years ago

Sorry. I've just noticed that this project doesn't updates for 8 months

ar1st0crat commented 5 years ago

Hi! Despite the sad fact that I haven't committed to this repo for a long time, I'm going to keep on working on it.

Regarding your suggestion - could you, please, give examples of scenarios when it's useful or necessary? I guess it'll really make sense in some scenarios, and I need clear use-cases for a better understanding and code design.

This lib originally included only offline processing algorithms. I created the DiscreteSignal class just as a wrapper around plain array of floats, so that I could focus more on DSP algorithms and transforms. At this moment the library has already some online processing stuff that operates with plain arrays of floats (IOnlineFilter, BlockConvolver.Process, FirFilter.Process, IirFilter.Process). And currently signals can be created from any enumerable collections (that will ultimately be cast to array anyway. This is because indexing and BlockCopy (memcpy) are heavily used in the lib. So I think if we decide to switch to something more general, the IList interface is more appropriate than IEnumerable. And maybe it's "oldschool C++ guy" talking in me :smiley:, but I'm quite OK with arrays so far).

MysteryDash commented 5 years ago

Hello, Adding a suggestion here instead of creating an issue since the topic is the same. What do you think about Span? Could it be a suitable tool to enhance the performances instead of relying on IEnumerable and ToArray (which create copies)?

ar1st0crat commented 5 years ago

Hi!

Thank you for the suggestion! Actually, I've missed some new features in C#, and now I'm quite surprised (in a good way) about Span<T> and Memory<T> types. It seems like Span could be perfectly used in some parts of the library. However, if I understand correctly, Span<T> cannot be used as the class member or a property (but it can be used in intermediate operations on stack).

In the nearest future, most likely, I'll be focused on other things, but I think Span<T> is definitely worth a try and I'll leave this issue opened.

PS. I'd also like to point out that currently memory is (re)used more or less efficiently in the library (as for me, the central part is DSP operations, not the DiscreteSignal wrapper). New memory is allocated, in most cases, only when it needs to. Two cases of unneccessary allocation/copy are:

If these two cases become a significant bottleneck, then it's better to use plain arrays in operations (i.e. not wrap them in DiscreteSignal class).

MysteryDash commented 5 years ago

I'm currently using your library in a project (not really ready to be released on Github) that involves computing a lot of FFTs (between 10 to 100k) and one of the bottlenecks was creating new arrays to store the data where the FFT is computed (considering that I pre-load the entire data in a single array, it's better to point to the place where the data is rather than copying it to a new buffer). I got a huge performance boost by simply taking your Fft.cs and replacing void Direct(float[] re, float[] im) with void Direct(Span<float> re, Span<float> im) (now, is it better to just replace them or create a brand new class for Span, I'm not really sure). Here's the full file I'm currently using.

EDIT: As a comparison, I tried using NAudio's FastFourierTransform, but I quickly gave up seeing that it'd take minutes (with your implementation it takes around one second to process everything) because of the Complex struct used (instead of using two float arrays).

ar1st0crat commented 5 years ago

Yes, your scenario clearly requires Span. Out of interest, what was the performace gain? In the "no-span version" did you create only one temp buffer and copy data there each time via BlockCopy() (or NWaves.Utils.FastCopyTo()) method? I'm sure Span is faster; I'm asking, because I'm just curious.

Still, in many cases (I believe, in most cases) temporary buffers must be created anyway, since: 1) either the initial array/signal must be preserved; 2) or frames may overlap (it's common in spectral analysis). Also, as far as I understand, each particular FFT routine will run slightly faster with arrays (Span adds little overhead for inner loops in array, from what I have read). Of course, in scenarios like yours the overhead of memory copying will be much more significant, but otherwise, array version would be preferred.

So, as for adding Span support to the lib, users should be able to choose between array version and Span version, thus it's better to add a new class, just like you did. However, I'm still contemplating, should it really be done right now... I don't want to retarget project to .NET Standard 2.1 so far; neither I don't want to add dependency to System.Memory package. I guess a lot of folks use older versions of VS and .NET (just like me myself )))...