RingBuffer Refactor

[TechnoPorg]

Putting this here to act as a reminder to us that it needs to be done.

Enhancement Summary

Ringbuffers are an important data structure for realtime audio processing, and LMMS makes extensive use of them in various places throughout the codebase. Currently, they are implemented through the ringbuffer submodule, LMMS' own RingBuffer class, which has heavy and unnecessary coupling to the engine, and a LocklessRingBuffer type as well.

It would be good to consolidate all of those into one ring buffer implementation that is fast, flexible, and limited in scope to just being a data structure.

Justification

It will help simplify the codebase and make behaviour more consistent.

[Snowiiii]

We should update the RingBuffer dependency as well

[he29-net]

[...] implementation that is fast, flexible, and limited in scope

And realtime-safe. https://gist.github.com/JohannesLorenz/5e1ebb6dd0af5efbff41ead2619210ae

Not sure what's the state of our local RingBuffer, but the LocklessRingBuffer (made by Johannes) was made specifically with that in mind, so I suspect it would be the better candidate to keep.

[michaelgregorius]

Searching for references to RingBuffer in the code only yields MultitapEchoEffect as a client of that class. So if that plugin could be switched to use LocklessRingBuffer then the non-realtime safe RingBuffer could be removed completely.

As was already stated LocklessRingBuffer is a wrapper around the underlying ringbuffer submodule. I guess it will add some functionality which is specific to LMMS so that class should be kept.

[DomClark]

I think the real-time safety comment is a bit misleading. Both ringbuffers appear to be real-time safe for single-threaded use of an appropriate subset of their operations. (Of course, construction and resizing require dynamic memory allocation and can't be real-time safe.) The difference is that, for multi-threaded use, the RingBuffer class requires external locking, which is not real-time safe, whereas LocklessRingBuffer is explicitly designed for multi-threaded use. Thus, assuming both are operated in a real-time safe manner, what LocklessRingBuffer actually offers over RingBuffer is thread safety. (LocklessRingBuffer also offers memory locking, so its underlying buffer doesn't get paged out, but IMO that would be better handled by a custom allocator that any class, including standard library containers, could use.)

I don't think there's a problem with maintaining both - they're designed to solve different problems. LocklessRingBuffer is useful for passing data between threads, where one of those threads needs to be real-time safe. It's used in this fashion in the LV2 code for implementing the worker feature, and in the vectorscope and spectrum analyser plugins for passing audio data to the analysis or rendering thread. RingBuffer, on the other hand, is designed to store audio data for single-threaded use, and is useful for delays. This is what the multitap echo plugin uses it for. Yes, a single class could be used for both, but that won't necessarily improve the code. LocklessRingBuffer comes with the additional complexity of using readers and writers, and the overhead of using atomics for tracking offsets, which aren't necessary for single-threaded use. Similarly, RingBuffer comes with an assortment of member functions that make working with audio easier, but are not useful or even meaningful for more general use.

As was already stated LocklessRingBuffer is a wrapper around the underlying ringbuffer submodule. I guess it will add some functionality which is specific to LMMS so that class should be kept.

IIRC it's to address some difficulty exporting symbols. We want to link the core statically to the ringbuffer library, but plugins require access to it too. Exporting symbols included from a static library is tricky with our current CMake version, so we export a wrapper instead.

[Rossmaxx]

I still think scoping down RingBuffer class to the Multitap Echo plugin has a benefit, though minimal.

From #7213: I wouldn't even move it for now - it likely has utility in the core for something like latency compensation.

Well, the file is badly written in itself so if anyone plans to use it for another purpose like latency compensation, it would need a refactor for it to make it work better.

[Rossmaxx]

Nah, better to leav it alone