fix #22336 - mute technique incorrectly maps to pizz strings #22693
Conversation
| @@ -997,7 +997,9 @@ inline const ArticulationMapping& articulationSounds(const mpe::PlaybackSetupDat | |||
| } | |||
|
|
|||
| if (setupData.category == mpe::SoundCategory::Winds) { | |||
| return WINDS; | |||
| if (setupData.id == mpe::SoundId::Horn || setupData.id >= mpe::SoundId::Tuba) { | |||
There was a problem hiding this comment.
slight kludge, but at least it prevents all known woodwind instruments from using "muted trumpet" sound when marked as muted.
wizofaus
force-pushed
the
fix_mute_sound_mapping
branch
4 times, most recently
from
aa579e9
to
940c1a4
Compare
| @@ -3842,7 +3842,7 @@ | |||
| "Mute": [ | |||
| { | |||
| "arrangementPattern": { | |||
| "durationFactor": 8000, | |||
| "durationFactor": 9900, | |||
There was a problem hiding this comment.
no reason for muted notes not to have standard playback duration (you couldn't tell when it was using pizzicato strings!)
| @@ -1000,7 +1000,10 @@ inline const ArticulationMapping& articulationSounds(const mpe::PlaybackSetupDat | |||
| } | |||
|
|
|||
| if (setupData.category == mpe::SoundCategory::Winds) { | |||
| return WINDS; | |||
| soundId = setupData.soundId(); | |||
| if (soundId == mpe::SoundId::Horn || (int)soundId >= (int)mpe::SoundId::Tuba) { | |||
There was a problem hiding this comment.
It would be better to add std::unordered_set<SoundId> and explicitly specify there SoundId for which we want to use BRASS
There was a problem hiding this comment.
I agree, just that that's a bigger change that would require more discussion. Happy to change if you can supply a list of instruments that everyone agrees it makes sense to use the "mute trumpet" sound for.
There was a problem hiding this comment.
This change is not that big... You already specified the list, but you did it implicitly using this condition: (int)soundId >= (int)mpe::SoundId::Tuba. However, it is rather unreliable because it depends on the order of items in the SoundId enum
| @@ -1000,7 +1000,10 @@ inline const ArticulationMapping& articulationSounds(const mpe::PlaybackSetupDat | |||
| } | |||
|
|
|||
| if (setupData.category == mpe::SoundCategory::Winds) { | |||
| return WINDS; | |||
| soundId = setupData.soundId(); | |||
There was a problem hiding this comment.
This doesn't do anything
There was a problem hiding this comment.
it mostly certain does!
There was a problem hiding this comment.
are you saying that at this point, soundId already has that value? When I looked that didn't seem to be true in all cases
There was a problem hiding this comment.
Yes, it has the correct value. We declare it here:
wizofaus
force-pushed
the
fix_mute_sound_mapping
branch
2 times, most recently
from
e982c46
to
c0b0992
Compare
|
Tested on MacOS 14, Windows 11, Ubuntu 22.04.3. Approved |
wizofaus
force-pushed
the
fix_mute_sound_mapping
branch
from
c0b0992
to
991b9e9
Compare
Resolves: #22336
Fix issue with MS Basic where by "mute" playing technique caused strings to play back as pizzicato (also fixed woodwinds not to play back as muted trumpet)