...So either I am confused, or the code I'm reading is confused, hard to say
;-)
anyway, after piecing together the legato logic scattered over various places,
it really looks like that flying "pos" -> "posb" was never a good idea....
Right now, the "posb" note is copy constructed, including a copy construction
of all the embedded components (like Envelope, Filter,...). But it remains
dormant. And there is special magic to avoid that dormant note to be discarded.
When the next legato note starts playing, all the pointers to those newly
created sub-components are just assigned with the corresponding components
of the "pos"-Note, i.e. both notes are now sharing those components (shudder!).
And, even worse, all those clone-copied sub component objects are never used
and just happily leaked!!!
Then, later, when the chain ends, there is special magic built into the
note-destructor to prevent clean-up of the sub components, based again on
the (NoteStatus != NOTE_DISABLED). Which, as far as I can tell, can only
be reached for legato notes (since for legato notes, the Envelope does not
proceed into the release phase, and thus never reaches the envelope end,
where for normal notes the NoteStatus is set to NOTE_DISABLED.
Needless to say, all of this is not implemented once, but three times,
(for AD, SUB and PAD) very similar but with minor differences. Yikes.
Anyway, it seems feasible to try out a very "defensive" approach here.
I will not touch any of the general setup, or the logic to allocate the "pos"
and "posb" slots. And also I will retain the sequence of actions in NoteON.
However...
- the idea is not to create the /next/ legato note in advance, but only
when it is really needed, i.e. when it starts to play. Thus no special
treatment for "dormant" notes any more.
- the second change will be: when a legato note is finished, it goes
into NOTE_DISABLED just like a regular note. And it will be just
deallocated likewise, without special conditions.
- then the most prominent change would be: switch the pos and posb
slots on each further legato note, so that pos is always the new note
and posb always holds the previous note, which just fades away. Moreover
do not keep those at fixed slots, but just allocate a new "pos" slot for
each new note; this way, legato notes can overlap within the limits of
POLYPHONY.
- after those changes, the logic of the copy ctor and the legatoFadeIn()
can be combined and collapsed. And the legatoFadeOut() can be largely
simplified, since it does not have to copy / clone anything, because
now it will be invoked on the current ongoing note, instead of
teleporting all of the note state over to another note instance.
Why do I want to try that out? Because, after this first analysis, the changes
seem straight-forward and "defensive". And in any case for me now simpler to
grasp than having to build special contrived logic to pass an ongoing PAD
wavetable cross-fade on-the-fly from the "pos" to the "posb" note, and all of
this threadsafe and without leaking memory (since NoteON happens concurrently).
-- Hermann
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
