|
From: Jacek R. <j....@gm...> - 2019-02-24 15:22:30
|
Hi. I needed more control over my samples with polyphony and note_polyphony opcodes. I'm not familiar with linuxsampler internals, but I've managed to create this patch to add support for those. It works for me on Raspberry Pi. It's hosted on github as well: https://github.com/jarosz/linuxsampler-linuxsampler/tree/polyphony Best regards Jacek Roszkowski |
|
From: Christian S. <sch...@li...> - 2019-02-24 15:46:44
|
On Sonntag, 24. Februar 2019 16:22:11 CET Jacek Roszkowski wrote: > I needed more control over my samples with polyphony and > note_polyphony opcodes. I'm not familiar with linuxsampler internals, > but I've managed to create this patch to add support for those. It > works for me on Raspberry Pi. Please do me a favour, if you want something to be applied on our side, then please provide either a patch or link to an appropriate patch against our SVN trunk with exactly the changes you want to be applied. CU Christian |
|
From: Jacek R. <j....@gm...> - 2019-02-24 16:37:04
|
It seems that I'm also not familiar with mailing lists. Excuse me for writing replies to your personal e-mail. I've sent them once more to the list. Regards Jacek Roszkowski |
|
From: Jacek R. <j....@gm...> - 2019-02-24 16:30:34
Attachments:
polyphony.patch
|
niedz., 24 lut 2019 o 16:46 Christian Schoenebeck
<sch...@li...> napisał(a):
> Please do me a favour, if you want something to be applied on our side, then
> please provide either a patch or link to an appropriate patch against our SVN
> trunk with exactly the changes you want to be applied.
I'm sorry. I forgot to include the attachment file. Now I've also
found out that it compiles only with developer mode enabled. Otherwise
it fails, because the required interface is not public:
EngineChannel.cpp:250:52: error: ‘void
LinuxSampler::AbstractVoice::EnterReleaseStage()’ is protected within
this context
itVoice->EnterReleaseStage();
|
|
From: Christian S. <sch...@li...> - 2019-02-24 17:37:48
|
On Sonntag, 24. Februar 2019 17:30:15 CET Jacek Roszkowski wrote: > I'm sorry. I forgot to include the attachment file. Now I've also > found out that it compiles only with developer mode enabled. Otherwise > it fails, because the required interface is not public: > > EngineChannel.cpp:250:52: error: ‘void > LinuxSampler::AbstractVoice::EnterReleaseStage()’ is protected within > this context > itVoice->EnterReleaseStage(); Mja, the thing is that src/engines/sfz/EngineChannel.cpp is usually not the right place for doing such core engine event handling that you attempt with your patch. The source files dediciated for the individual formats (gig/sf2/ sfz) only cover the most minimalistic stuff required specifically only for the individual format itself, all the rest is shared, common code available to all engines, so when a new feature is added for one engine that it could still easily be adopted by all other engines as well (like it usually always happens after some time). But first things first. If I understand it correctly, you are already aware that there is already a feature for all our engines/formats which is called "exclusive groups" or "key groups", right? This feature allows to assign individual keys on a keyboard to individual "key groups", and within one key group there is always only 1 note intended to be active at a time, so if you trigger a key being member of a key group and there is already another note active of that same key group, then the previous note of that group will automatically be released. So that's a feature typically required for drum kits. Having said, your intention is to extend this existing feature for optionally raising the amount of active notes in a key group to another higher value than just 1, is that correct? CU Christian |
|
From: Jacek R. <j....@gm...> - 2019-02-24 18:27:17
|
niedz., 24 lut 2019 o 18:38 Christian Schoenebeck < sch...@li...> napisał(a): > Mja, the thing is that src/engines/sfz/EngineChannel.cpp is usually not the > right place for doing such core engine event handling that you attempt with > your patch. The source files dediciated for the individual formats (gig/sf2/ > sfz) only cover the most minimalistic stuff required specifically only for the > individual format itself, all the rest is shared, common code available to all > engines, so when a new feature is added for one engine that it could still > easily be adopted by all other engines as well (like it usually always happens > after some time). I agree. I've decided to try inside EngineChannel, because it contained required data (list of all playing voices) ordered by midi key. > But first things first. If I understand it correctly, you are already aware that > there is already a feature for all our engines/formats which is called > "exclusive groups" or "key groups", right? This feature allows to assign > individual keys on a keyboard to individual "key groups", and within one key > group there is always only 1 note intended to be active at a time, so if you > trigger a key being member of a key group and there is already another note > active of that same key group, then the previous note of that group will > automatically be released. So that's a feature typically required for drum > kits. I'm using this feature to mute cymbals, but in sfz with off_by opcode user can specify only one key, and it's implemented correctly in linuxsampler. It can accurately emulate drum kit pieces, except for mutable cymbals. If you specify a separate key to mute a cymbal it doesn't mute its consecutive hits leading to unnatural sound buildup. > Having said, your intention is to extend this existing feature for optionally > raising the amount of active notes in a key group to another higher value than > just 1, is that correct? Yes, it's correct. With sfz 2 format opcodes http://www.sfzformat.com/index.php?title=Polyphony http://www.sfzformat.com/index.php?title=Note_polyphony I can specify a mute key for a group with off_by opcode and decrease unnatural sound buildup with polyphony. My configuration is based on http://www.sfzformat.com/index.php?title=Cymbal_muting but even more sophisticated with many velocity layers and round robin hits. Regards Jacek Roszkowski |
|
From: Christian S. <sch...@li...> - 2019-02-24 22:34:42
|
On Sonntag, 24. Februar 2019 19:26:57 CET Jacek Roszkowski wrote: > > Having said, your intention is to extend this existing feature for > > optionally raising the amount of active notes in a key group to > > another higher value than just 1, is that correct? > > Yes, it's correct. With sfz 2 format opcodes > http://www.sfzformat.com/index.php?title=Polyphony > http://www.sfzformat.com/index.php?title=Note_polyphony > I can specify a mute key for a group with off_by opcode and decrease > unnatural sound buildup with polyphony. Then it might make sense to extend the existing key group handling code by optionally using the number provided by the patch. You might want to have a look at AbstractEngineChannel::HandleKeyGroupConflicts(). CU Christian |
|
From: Jacek R. <j....@gm...> - 2019-02-25 15:56:52
|
niedz., 24 lut 2019 o 23:34 Christian Schoenebeck <sch...@li...> napisał(a): > Then it might make sense to extend the existing key group handling code by > optionally using the number provided by the patch. You might want to have a > look at AbstractEngineChannel::HandleKeyGroupConflicts(). That method was my starting point, but I don't understand exactly what's going on inside: // send a release event to all active voices in the group RTList<Event>::Iterator itEvent = ActiveKeyGroups[KeyGroup]->allocAppend(pEngine->pEventPool); *itEvent = *itNoteOnEvent; This is my understanding 1. there's a new event created using pEventPool memory 2. it's appended to the ActiveKeyGroups at the KeyGroup index 3. it's set up as the copy of the note on event, which caused this voice to be launched 4. why does it make all active voices in the group killed? I suppose it's because the condition itEvent->Param.Note.Key != HostKey() is true in sfz::Voice::ProcessGroupEvent, but I don't understand the logic behind It's a good question whether extending key group handling is the way to go. I don't know if there are similar capabilities in gig or sf2 formats. In sfz format group opcode makes a group "exclusive" only if off_by is set to the same value. Setting group without off_by doesn't mute consecutive hits, but it does with polyphony=1. However, default polyphony is not 1, but is limited to sampler maximum voice limit (at least in sforzando vst player). If you can point out similar properties in other engines, i'll refactor the patch to use more common code. I'm going to implemet http://www.sfzformat.com/index.php?title=Note_selfmask opcode as well. Please tell me how to mute a voice using an event instead of direct calling EnterReleaseStage method. If there is no other way, would you accept declaring sfz::EngineChannel class as a friend inside sfz::Voice? Regards Jacek Roszkowski |
|
From: Christian S. <sch...@li...> - 2019-02-25 18:10:07
|
On Montag, 25. Februar 2019 16:56:33 CET Jacek Roszkowski wrote: > <sch...@li...> napisał(a): > > Then it might make sense to extend the existing key group handling code by > > optionally using the number provided by the patch. You might want to have > > a > > look at AbstractEngineChannel::HandleKeyGroupConflicts(). > > That method was my starting point, but I don't understand exactly > what's going on inside: No problem, that's what this email ist is for. > // send a release event to all active voices in the group > RTList<Event>::Iterator itEvent = > ActiveKeyGroups[KeyGroup]->allocAppend(pEngine->pEventPool); > *itEvent = *itNoteOnEvent; > > This is my understanding > 1. there's a new event created using pEventPool memory > 2. it's appended to the ActiveKeyGroups at the KeyGroup index > 3. it's set up as the copy of the note on event, which caused this > voice to be launched All correct. > 4. why does it make all active voices in the group killed? I suppose > it's because the condition itEvent->Param.Note.Key != HostKey() is > true in sfz::Voice::ProcessGroupEvent, but I don't understand the > logic behind Ok, looks like you already found all relevant code sections for this feature, up to the final point where the voices are finally released in Voice::ProcessGroupEvent(), the latter method is implemented by each one of the 3 engines separately by the way, so that way it is possible to implement behaviour individualization regarding key group conflicts for the individual engines/formats. When Voice::ProcessGroupEvent() is called, it is passing a copy of the original note-on event which triggered the key group conflict. The comparison with the key number you mentioned is currently intended to let it a) kill notes on other keys (of the same key group), but b) never kill notes on the same key. And when you look at the implementation of that method implementation for the gig engine (src/engines/gig/Voice.cpp), you find a fat comment where we did not agree how this behaviour should be precisely. In that source code comment I wrote that it would not sound natural for drumkits to kill notes of the same key. We had a discussion about that on the mailing list as well (probably even several). And that's already the main reason why I don't think that it would make sense if you would address this issue separately in the sfz engine's source files alone. Because this is an important issue that we will need to address for all engines, because the precise behaviour of key group conflicts needs to be configurable one day for all engines, not just for sfz. > It's a good question whether extending key group handling is the way > to go. I don't know if there are similar capabilities in gig or sf2 > formats. In sfz format group opcode makes a group "exclusive" only if > off_by is set to the same value. Setting group without off_by doesn't > mute consecutive hits, but it does with polyphony=1. However, default > polyphony is not 1, but is limited to sampler maximum voice limit (at > least in sforzando vst player). If you can point out similar > properties in other engines, i'll refactor the patch to use more > common code. I'm going to implemet As you might imagine, the sf2 engine is not really a priority, but for the gig engine there will definitely be new configuration options being added. They don't exist with the original gig format, but we are extending the original gig format all the time for important new features, and these options you are talking about are very important ones for natural drumkits. From what you described so far, I don't see why you would need to open a separate concept to the existing key group conflict concept. As far as I can see it, the key group concept just would need some additional options for allowing what you want to do. Additionally to what you are working on (amount of voices per key group, and whether a key group shall use a soft release or rather harsh kill), there will most probably be other options in future as well, for instance what to do on sustain pedal? Should a pressed sustain pedal ignore key group conflicts, yes/ no? Still sceptical? :) CU Christian |
|
From: Jacek R. <j....@gm...> - 2019-02-25 20:05:42
|
pon., 25 lut 2019 o 19:10 Christian Schoenebeck <sch...@li...> napisał(a): > From what you described so far, I don't see why you would need to open a > separate concept to the existing key group conflict concept. As far as I can > see it, the key group concept just would need some additional options for > allowing what you want to do. Thank you for your explanations. I'm a hobbyist drummer and hobbyist programmer and I don't aspire to bring a final solution for those issues. I suppose the common engine interface is not going to be released in near future. Therefore, I'll finish my sfz implementation using events, since that's what I need for now and send the patch to the mailing list. > Still sceptical? :) Not at all. Misunderstanding came from the name "Key Groups (a.k.a Exclusive Groups)" and actual implementation with no other possible behaviour. I think this sfz 2 opcode (default on) could be a sane default behaviour for each engine, even with sustain pedal on: http://www.sfzformat.com/index.php?title=Note_selfmask ("higher-velocity notes turn off lower-velocity notes, but lower-velocity notes do not turn off higher-velocity notes"). Best regards Jacek Roszkowski |
|
From: Christian S. <sch...@li...> - 2019-02-25 20:37:12
|
On Montag, 25. Februar 2019 21:05:22 CET Jacek Roszkowski wrote: > pon., 25 lut 2019 o 19:10 Christian Schoenebeck > > <sch...@li...> napisał(a): > > From what you described so far, I don't see why you would need to open a > > separate concept to the existing key group conflict concept. As far as I > > can see it, the key group concept just would need some additional options > > for allowing what you want to do. > > Thank you for your explanations. I'm a hobbyist drummer and hobbyist > programmer and I don't aspire to bring a final solution for those > issues. I did not suggest you to come up with a final solution. I just said that modifying the existing key group code makes more sense than adding a completely separate code inside the sfz classes which would handle the same thing as the existing key group code. It requires actually far less code to just modify the key group code than adding your previously suggested patch. Where do you see difficulties extending the key group code for your purpose? > I suppose the common engine interface is not going to be > released in near future. What do you mean with common interface released? We already do have a shared code base for all 3 engines. Almost all engine code is shared code, and all new engine features are always added to that shared code base for several years now. That's a continuous, non-formal process. If somebody needs to change some interface of the shared engine code for some new feature, then he simply does that. That's a not a public API, so we can quickly change things there without having to vote or something. We have put a lot of work in actually deduplicating all the code we had for the individual engines before; it does not make sense to turn back time and piling up yet again code inside the individual (non shared) engine code for things that would be needed in the other engines later on. CU Christian |
|
From: Jacek R. <j....@gm...> - 2019-02-25 22:00:02
|
pon., 25 lut 2019 o 21:37 Christian Schoenebeck <sch...@li...> napisał(a): > What do you mean with common interface released? By final solution and common interface I thought about adding parameters you have already mentioned (amount of voices per key group, and whether a key group shall use a soft release or rather harsh kill) to AbstractVoice instead of sfz::Voice. > That's a continuous, non-formal process. If somebody needs to > change some interface of the shared engine code for some new feature, then he > simply does that. That's a not a public API, so we can quickly change things > there without having to vote or something. That was actually my concern. Thank you again for your replies. |
|
From: Jacek R. <j....@gm...> - 2019-03-08 04:45:57
Attachments:
polyphony2.patch
test.sfz
|
Hi. This is my second approach to handling polyphony. I'm not submitting it into bugzilla, since I'm not sure if that's the way it should be resolved. It works for me, but it's not thoroughly tested even for my needs. These are my concerns. 1. It can't be implemented in AbstractEngineChannel, because it needs an access to voice pool, so it's in template EngineChannelBase. 2. It implements self-masking for sfz and gig (not sure how the sustain pedal issue should be resolved, so I left it intact) and handles group conflict in a different way (turned on by default only for sfz). If you want to test it with gig and sf2 (I haven't), just change the default value of NewAlgorithm to true. 3. It creates an interface for Voice classes to return KeyGroup (or off_by for sfz with an argument set to true). I don't like this solution. Maybe it should have two separate functions like GetKeyGroup, GetOffBy returning same values for gig and sf2 and different for sfz. 4. [premature optimization alert] It creates a separate Event for each voice and processGroupEvents iterates over all events. I think the complexity of this solution is O(n^2). It would be better with one event containing a std::set of VoiceIDs to check against, but it's not possible inside Event's union. 5. The function is huge. I don't like it. 6. signed/unsigned mismatch for sfz opcodes These are declared as int, even though most of them (according to http://www.sfzformat.com/legacy/) are unsigned with values from 0 to 4 Gb (4294967296), and the "end" opcode can even take values from -1 to 4 Gb. This spec is weird, since 32 bit ULONG_MAX is 4294967295. It's not such a big deal, but the patch depends on off_by default value to be -1, since AbstractVoice requires iKeyGroup to be non-zero to processGroupEvents() 7. My reference sfz engine is ARIA 1.933 with sforzando VST plugin. Unfortunately it behaves differently with note_polyphony opcode. My patch releases oldest voices first. ARIA releases most recent ones. I think it's a bug, not a feature, because if there are two regions triggered at the same time the first one is suppressed and only the second one is played. I've attached an example file to reproduce this ARIA bug (you must provide a Crash.wav file which is long enough to test polyphony). I've sent it here to get your opinion, but I suppose I'm not going to develop my concerns further, only bugfixing. |
|
From: Christian S. <sch...@li...> - 2019-03-08 13:25:24
|
On Freitag, 8. März 2019 05:45:38 CET Jacek Roszkowski wrote: > This is my second approach to handling polyphony. I'm not submitting > it into bugzilla, since I'm not sure if that's the way it should be > resolved. It works for me, but it's not thoroughly tested even for my > needs. Ok, I have reviewed your patch. There are several issues with your suggested patch, some which you already pointed out, but also you are doing things which are not real-time safe, and overall it simply looks like more changes than actually required. I understand that you already invested more time in this feature than you wanted to. So my suggestion: just open a report for now and attach your patch so it won't get lost, and a bit later I might come up with an alternative implementation to achieve this feature, which won't happen in the next couple weeks or so though. And one important thing: Please also describe (or provide links) in the report how the new sfz opcodes shall behave exactly in your opinion. Because I am not using sfz much. So I don't know the common behaviour of existing sfz opcodes of other players. And good (free) documentation about sfz is still a bit sparse. For the latter I once started this: http://doc.linuxsampler.org/sfz/ ( http://svn.linuxsampler.org/cgi-bin/viewvc.cgi/doc/docbase/sfz/ ) But as you can see it is also not more than a start ATM. > 6. signed/unsigned mismatch for sfz opcodes > These are declared as int, even though most of them (according to > http://www.sfzformat.com/legacy/) are unsigned with values from 0 to 4 > Gb (4294967296), and the "end" opcode can even take values from -1 to > 4 Gb. This spec is weird, since 32 bit ULONG_MAX is 4294967295. It's > not such a big deal, but the patch depends on off_by default value to > be -1, since AbstractVoice requires iKeyGroup to be non-zero to > processGroupEvents() I don't think that's an issue at all. I don't expect anybody to have a demand of so many key groups. You probably barely find an instrument with more than 88 key groups. And for the negative issue on our internal implementation side: for that purpose we do have the optional<> template class, which behaves (almost) identical to its equal named template class from the boost library and STL17: http://svn.linuxsampler.org/cgi-bin/viewvc.cgi/linuxsampler/trunk/src/common/ optional.h?view=markup > 7. My reference sfz engine is ARIA 1.933 with sforzando VST plugin. > Unfortunately it behaves differently with note_polyphony opcode. My > patch releases oldest voices first. ARIA releases most recent ones. I > think it's a bug, not a feature, because if there are two regions Yes, that's definitely not an appropriate behaviour. I don't know the ARIA engine much. But I know that many players don't have any real voice stealing algorithm implementation. So many engines simply pick random old voices instead of caring about to pick appropriate old voices. > I've sent it here to get your opinion, but I suppose I'm not going to > develop my concerns further, only bugfixing. No problem. I appreciate your effort though! CU Christian |
|
From: Jacek R. <j....@gm...> - 2019-03-08 22:10:59
|
> Ok, I have reviewed your patch. There are several issues with your suggested > patch, some which you already pointed out, but also you are doing things which > are not real-time safe, and overall it simply looks like more changes than > actually required. I think that there's no need for a voice to release itself and in the third iteration a public pure virtual function in AbstractVoice implemented in derived classes should be enough. The decision about releasing a voice can be made in EngineChannelBase (like in the patch) without group processing queue (which in my opinion is counter-intuitive with sfz format's group and off_by opcodes. > And one important thing: Please also describe (or provide links) in the report > how the new sfz opcodes shall behave exactly in your opinion. Because I am not > using sfz much. So I don't know the common behaviour of existing sfz opcodes > of other players. And good (free) documentation about sfz is still a bit > sparse. For the latter I once started this: I will submit it. > I don't think that's an issue at all. I don't expect anybody to have a demand > of so many key groups. You probably barely find an instrument with more than 88 > key groups. And for the negative issue on our internal implementation side: > for that purpose we do have the optional<> template class, which behaves > (almost) identical to its equal named template class from the boost library > and STL17: > > http://svn.linuxsampler.org/cgi-bin/viewvc.cgi/linuxsampler/trunk/src/common/ > optional.h?view=markup I think later on I can convert sfz engine to use this class and unsigned ints where appropriate to make linuxsampler more compatible with the spec (that's the only reason). I was using far more than 88 groups in my drumkit instrument because of no polyphony support. Imagine a ride cymbal hit, choose one zone (bow) out of three, multiply it by velocity layers (8) and in each layer there's a group of 4 round robin regions * 2 samples (overhead and room). It's 72 groups for ride cymbal. And there are 16 layers for snare, and positional sensing :) Now I can put all ride samples in one group, choke it with another off_by group and manage concurrent voices by note_polyphony (let's say 2) with note_selfmask=on which is great for cymbal hits, but setting polyphony to 6 for a group allows me to save resources. > No problem. I appreciate your effort though! Maybe in some (long?) time I will share the final result - something like a DIY poor man's Pearl Mimic Pro. |