Am 20.05.22 um 18:08 schrieb Will Godfrey:
It seems to be connected with legato.
Yoshimi segfaults when you press the second key if you are still holding
the first one. There is no problem if mode is switched to either Mono or
Poly.
Am 20.05.22 um 20:22 schrieb Ichthyostega:
In a nutshell, it came in with the rework of Legato / Portamento.
The Segfault happens when we invoke performPortamento(Note) but the this*
(i.e. an ADnote object) is NULL.
...
Thus seemingly the organisation of "pos" and/or "currItem" is derailed here,
so we're invoking the wrong note slot.
I will take a close Look with the debugger tomorrow, to find out how this can
happen.
Hi Yoshimi-devs,
this bug was caused by a mistake in the legato changes: I separated out
Legato-Portamento as a distinct new case, and while doing so, I failed to handle
Kit-Mode properly. Seemingly I also failed to test that flavour in Kit Mode.
For context, during the "padthread" effort I was forced to rearrange the legato
handling in order to be able to add cross-fading between old and new PADSynth
wavetable. While previously legato notes used a fixed pair of "pos/posb" notes,
now the legato notes do allocate a new free position similar to normal notes.
Only Legato-Portamento notes stand out as a special case now, insofar they
continue to use a single note instance on the existing note pos and just
manipulate the frequency to slide towards the new note frequency.
As we all know, this whole feature "Legato" has a host of problems, and this
refactoring does noting to address these fundamental / conceptual problems;
basically I just rearranged and simplified the existing code systematically
and removed some redundancy between the cases.
Likewise, the fix for this current Sefgault problem does nothing to address
those fundamental/conceptual problems, it just fills in state handling, which
I had failed to adapt to the new special case "Legato-Portamento".
Especially when some kit item has a limited note range and the legato/portamento
crosses this range boundary, all kinds of weird behaviour can be provoked.
Simply because we only test the condition (muted, note range), but we have
no way to find out how the previous kit item note related to these conditions
and if it even belongs to the same kit item as activated now.
In a nutshell: in Kit Mode we allocate the actually playing notes into
"slots" in the kitItem array, and whenever new notes were created, we
have to increment the partnote[pos].itemsplaying counter, so the handling
for the next kit item will use the next "slot" above.
With the fix attached, the crash is gone for me, please verify.
-- Hermann
PS: the patch was created with git format-patch,
and thus can easily be applied with
git am 0001-Bugfix-Legato-Portamento-in-Kit-Mode.patch
From 81874c93e7f01b2ddcff12bf720312cd4d4ebc9a Mon Sep 17 00:00:00 2001
From: Ichthyostega <p...@ichthyostega.de>
Date: Sun, 22 May 2022 01:30:19 +0200
Subject: [PATCH] Bugfix: Legato-Portamento in Kit-Mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
During the »padthread« effort it became necessary to rearrange
the handling of Legato notes.
see: 4fa7686c46f7ca
As part of this refactoring, portamento on lagato notes was identified
as a special case and extracted from the regular legato handling:
these notes are now the only ones to stick to a fixed note pos and
re-use a note object.
During that refactoring, I failed to handle Kit mode properly.
In Kit mode, for each actually used kit item a "slot" in the kitItem
array is reserved and the counter partnote[pos].itemsplaying is incremented,
so the next kit item to handle will use the next "slot" above.
Since legato-portamento notes now re-use an existing note-pos without first
clearing / deleting this pos, we actually need to reset the itemsplaying counter.
WARNING: this whole scheme behaves strangely when the note range of some
kit items is limited. This used to be so in the old code, and the whole
refactoring and also this bugfix here does nothing to address this issue.
# Conflicts:
# src/version.txt
---
src/Misc/Part.cpp | 49 ++++++++++++++++++++++++++++-------------------
src/Misc/Part.h | 1 +
2 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/src/Misc/Part.cpp b/src/Misc/Part.cpp
index 59955551..24c744d5 100644
--- a/src/Misc/Part.cpp
+++ b/src/Misc/Part.cpp
@@ -370,15 +370,23 @@ namespace { // Helpers to handle the tree kinds of KitItemNotes uniformly...
template<class NOTE>
inline void connectNewLegatoNote(NOTE*& oldNote
,NOTE*& newNote
- ,Note note)
+ ,Note noteData)
{ if (oldNote)
{ // spawn new note as clone from previous note
newNote = new NOTE(*oldNote);
// instruct both notes to perform a short "legato" crossfade
- newNote->legatoFadeIn(note);
+ newNote->legatoFadeIn(noteData);
oldNote->legatoFadeOut();
}
}
+
+ template<class NOTE>
+ inline void activateLegatoPortamento(NOTE*& activeNote, Note noteData)
+ {
+ if (activeNote)
+ activeNote->performPortamento(noteData);
+ }
+
}//(End)KitItemNote helpers
@@ -404,11 +412,7 @@ void Part::startNewNotes(int pos, size_t item, size_t currItem, Note note, bool
(kit[item].Psendtoparteffect < NUM_PART_EFX)? kit[item].Psendtoparteffect
: NUM_PART_EFX; // direct to Part-output
- if ( partnote[pos].kitItem[currItem].adnote
- ||partnote[pos].kitItem[currItem].subnote
- ||partnote[pos].kitItem[currItem].padnote
- )
- partnote[pos].itemsplaying++;
+ incrementItemsPlaying(pos,currItem);
}
@@ -434,12 +438,7 @@ void Part::startLegato(int pos, size_t item, size_t currItem, Note note)
: NUM_PART_EFX; // direct to Part-output
partnote[prevPos].status = KEY_RELEASED; // treat legato crossfade similar to envelope-release
-
- if ( partnote[pos].kitItem[currItem].adnote
- ||partnote[pos].kitItem[currItem].subnote
- ||partnote[pos].kitItem[currItem].padnote
- )
- partnote[pos].itemsplaying++;
+ incrementItemsPlaying(pos,currItem);
}
@@ -447,18 +446,28 @@ void Part::startLegato(int pos, size_t item, size_t currItem, Note note)
void Part::startLegatoPortamento(int pos, size_t item, size_t currItem, Note note)
{
if (kit[item].Padenabled)
- partnote[pos].kitItem[currItem].adnote->
- performPortamento(note);
+ activateLegatoPortamento(partnote[pos].kitItem[currItem].adnote, note);
if (kit[item].Psubenabled)
- partnote[pos].kitItem[currItem].subnote->
- performPortamento(note);
+ activateLegatoPortamento(partnote[pos].kitItem[currItem].subnote, note);
if (kit[item].Ppadenabled)
- partnote[pos].kitItem[currItem].padnote->
- performPortamento(note);
+ activateLegatoPortamento(partnote[pos].kitItem[currItem].padnote, note);
+
+ incrementItemsPlaying(pos,currItem);
}
+// After allocating a new note or activating Legato/Portamento: keep track of the kitItem-Slots actually activated
+void Part::incrementItemsPlaying(int pos, size_t currItem)
+{
+ if ( partnote[pos].kitItem[currItem].adnote
+ ||partnote[pos].kitItem[currItem].subnote
+ ||partnote[pos].kitItem[currItem].padnote
+ )
+ partnote[pos].itemsplaying++;
+}
+
+
// Modified velocity for the given kit item to blend the overlap with the neighbouring item
float Part::computeKitItemCrossfade(size_t item, int midiNote, float inputVelocity)
{
@@ -621,6 +630,7 @@ void Part::NoteOn(int note, int velocity, bool renote)
partnote[pos].note = note;
partnote[pos].keyATtype = PART::aftertouchType::off;
partnote[pos].keyATvalue = 0;
+ partnote[pos].itemsplaying = 0;
if (performLegato)
{
@@ -666,7 +676,6 @@ void Part::NoteOn(int note, int velocity, bool renote)
}
else
{// start regular notes or a new chain of legato notes
- partnote[pos].itemsplaying = 0;
if (Pkitmode == 0)
// non-Kit mode: init Add-, Sub and PAD-notes...
startNewNotes(pos,0,0, Note{note,noteFreq,vel}, portamento);
diff --git a/src/Misc/Part.h b/src/Misc/Part.h
index 5202844e..ac10cec4 100644
--- a/src/Misc/Part.h
+++ b/src/Misc/Part.h
@@ -184,6 +184,7 @@ class Part
void startLegato (int pos, size_t item, size_t currItem, Note);
void startLegatoPortamento(int pos, size_t item, size_t currItem, Note);
float computeKitItemCrossfade(size_t item, int midiNote, float inputVelocity);
+ void incrementItemsPlaying(int pos, size_t currItem);
Samples& tmpoutl;
Samples& tmpoutr;
--
2.20.1
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
