search

hydrogen-music / hydrogen

The advanced drum machine for Linux, macOS, and Windows
http://www.hydrogen-music.org
GNU General Public License v2.0
1.04k stars 173 forks source link

Components break note rendering when switching drumkits #1903

Closed theGreatWhiteShark closed 3 weeks ago

theGreatWhiteShark commented 10 months ago

Hydrogen version * : 0.9.7 - 1.2.X

The introduction of DrumkitComponent and InstrumentComponent into the code base seems to have been done without taking a look on how drumkits are switched in Hydrogen. This causes potential audio artifacts (at best) as well as crashes (see #1901).

How rendering of notes is done

Each note contains a pointer to the associated instrument using which the samples is accessed during rendering. In addition, a number of parameters, like pan, gain, ADSR, etc. are queried from the current song, it's drumkit components, preferences etc.

How switching drumkits works

When switching a drumkit all components and instrument lists within the song are replaced by the ones present in the drumkit the user is about to load. This means new samples and new parameter settings.

For the notes of all patterns instruments are exchanged. E.g. the first row of the pattern editor contained a snare prior to the switch and a kick afterwards. Due to the switch the instruments of all notes already present have their associated instrument changed as well.

Now the important part: to allow for a more seamless switching of kits the instruments of notes in the queues of both AudioEngine and Sampler are not remapped. They still contain the pointer to their original instrument and will be rendered using the original sample. Those instruments are not delete right away when switching the kit but enqueued in a so called Hydrogen::m_instrumentDeathRow. They will only be discarded if no note is queued for them anymore (each instrument will keep track of this using Instrument::__queued).

How switching of songs works

When switching an entire song, a lot of parameters used in the rendering routine would change too. That's why transport is stopped and all note queues are flushed before remapping instruments etc. There is no seamless song switching.

What's the problem now?

With the component patch a new class was introduced into the instrument architecture. Instrument > InstrumentComponent > InstrumentLayer > Sample.

An InstrumentComponent does hold all InstrumentLayers, a component gain, and the id of the associated DrumkitComponent. The latter will contain all parameters used for rendering.

Now, when switching drumkits those InstrumentComponents won't be able to find their original DrumkitComponents anymore. Instead, they attempt to use the newly loaded ones which sometimes cause crashes.

Where to go from here?

We should make the Instrument self-contained again. This way a Note will, again, have all information required to render it.

Maybe the DrumkitComponent can be included as a shared ptr next to/instead of its ID in the InstrumentComponent. Since we are using shared pointer for most basic stuff now, we can probably skip introducing a "drumkit component death row" and let the garbage collector handle things.

theGreatWhiteShark commented 3 months ago

On second thought it is probably not a good idea to let samples from the old kit ring till the end when switching kits. Instead, the release() part of their ADSR's should be triggered to stop their audio in a smooth transition.

For most kits this probably has no big impact. But image a kit with long samples (e.g. minutes) or even just a long tailed ride bell. When switching to another kit, this long sample keeps ringing and ringing and the user has to stop transport using the panic shortcut to make it go away. This is rather awkward since switching the kit gave a clear intention of not using the former samples anymore.