From 705a74953be311ea1e1b57595fb4f007b9864096 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:48:41 -0500 Subject: [PATCH] SoundFont instruments make their voices before opening the bank A sampler's voices are one block of memory. Opening a large bank first can leave plenty free but no single gap that big, so create() failed with MemoryError on an 8 MB heap once the sampler's voices grew. Both soundfont and soundfont_drums now make their own pool first, and free it again if the bank won't open. --- lib/audioinstruments/soundfont.py | 22 ++++++-- lib/audioinstruments/soundfont_drums.py | 19 ++++--- tests/test_cpython_soundfont.py | 72 +++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 12 deletions(-) diff --git a/lib/audioinstruments/soundfont.py b/lib/audioinstruments/soundfont.py index 5923cb18..5385c038 100644 --- a/lib/audioinstruments/soundfont.py +++ b/lib/audioinstruments/soundfont.py @@ -1377,7 +1377,11 @@ def _sampler(sample_rate, channel_count, voice_count=MAX_VOICES): def voices_for(sample_rate, channel_count, voice_pool=None, voices=None): """`voice_pool` checked against the instrument's format, or a pool of - its own of `voices` voices (`MAX_VOICES` when None).""" + its own of `voices` voices (`MAX_VOICES` when None). + + Call it before `open_bank`: a sampler's voices are one block, and once + a bank has filled the heap with its sample cache there may be plenty + free but no single gap that big.""" if voice_pool is None: return Voices(sample_rate, channel_count, MAX_VOICES if voices is None else voices, shared=False) @@ -1390,13 +1394,24 @@ def voices_for(sample_rate, channel_count, voice_pool=None, voices=None): return voice_pool +def _drop_own(pool, voice_pool): + """Free a pool `voices_for` made, when the bank it was for won't open.""" + if voice_pool is None: + pool.deinit() + + def create(sample_rate, channel_count=2, transport=None, bank=None, voice_pool=None, voices=None): """The bank's melodic presets as one instrument; `bank` is a path, an open `SoundFont`, or None to look in the usual places. `voice_pool` is a `Voices` to share with other instruments, or None for its own of `voices` voices (`MAX_VOICES` when None).""" - loaded = open_bank(bank) + pool = voices_for(sample_rate, channel_count, voice_pool, voices) + try: + loaded = open_bank(bank) + except BaseException: + _drop_own(pool, voice_pool) + raise melodic = loaded.melodic_bank() programs = loaded.programs(melodic) patches = {} @@ -1409,7 +1424,6 @@ def create(sample_rate, channel_count=2, transport=None, bank=None, name = "Program %d (empty)" % program patches[program] = (name, DEFAULT_MACROS) targets[program] = (melodic, program) - player = _Player(loaded, voices_for(sample_rate, channel_count, - voice_pool, voices), melodic) + player = _Player(loaded, pool, melodic) return SoundFontInstrument(player, patches, targets, MACRO_LABELS, transport=transport) diff --git a/lib/audioinstruments/soundfont_drums.py b/lib/audioinstruments/soundfont_drums.py index 78ae80da..ab2d10a4 100644 --- a/lib/audioinstruments/soundfont_drums.py +++ b/lib/audioinstruments/soundfont_drums.py @@ -101,19 +101,22 @@ def create(sample_rate, channel_count=2, transport=None, bank=None, voice_pool=None, voices=None): """The bank's kits as one instrument; `bank`, `voice_pool` and `voices` as for `soundfont`.""" - loaded = _soundfont.open_bank(bank) - kits = loaded.programs(_soundfont.PERCUSSION_BANK) - if not kits: - raise ValueError("the SoundFont has no percussion bank (128)") + pool = _soundfont.voices_for(sample_rate, channel_count, voice_pool, + voices) + try: + loaded = _soundfont.open_bank(bank) + kits = loaded.programs(_soundfont.PERCUSSION_BANK) + if not kits: + raise ValueError("the SoundFont has no percussion bank (128)") + except BaseException: + _soundfont._drop_own(pool, voice_pool) + raise patches = {} targets = {} for index, (program, name) in enumerate(kits): patches[index] = (name, _soundfont.DEFAULT_MACROS) targets[index] = (_soundfont.PERCUSSION_BANK, program) - player = _soundfont._Player( - loaded, _soundfont.voices_for(sample_rate, channel_count, voice_pool, - voices), - _soundfont.PERCUSSION_BANK) + player = _soundfont._Player(loaded, pool, _soundfont.PERCUSSION_BANK) return _soundfont.SoundFontInstrument( player, patches, targets, MACRO_LABELS, note_map=NOTE_MAP, transport=transport, bank_select=False) diff --git a/tests/test_cpython_soundfont.py b/tests/test_cpython_soundfont.py index 6c6734dc..c97d05f8 100644 --- a/tests/test_cpython_soundfont.py +++ b/tests/test_cpython_soundfont.py @@ -939,5 +939,77 @@ def test_a_new_bound_takes_a_new_arena(self): self.assertGreater(kit.sounding(), 0) +class VoicesBeforeBankTest(unittest.TestCase): + """An instrument's own voices are made before its bank is opened: the + sampler's voices are one block, and a heap the bank has filled may + have no gap that big left.""" + + @classmethod + def setUpClass(cls): + cls.tmp = tempfile.mkdtemp(prefix="sf2order") + cls.path = os.path.join(cls.tmp, "tiny.sf2") + sf2_builder.tiny_bank().write(cls.path) + + @classmethod + def tearDownClass(cls): + shutil.rmtree(cls.tmp, ignore_errors=True) + + def setUp(self): + self.calls = [] + real_sampler = soundfont._sampler + real_open = soundfont.open_bank + + def sampler(*args, **kwargs): + self.calls.append("sampler") + return real_sampler(*args, **kwargs) + + def open_bank(*args, **kwargs): + self.calls.append("bank") + return real_open(*args, **kwargs) + + soundfont._sampler = sampler + soundfont.open_bank = open_bank + self.addCleanup(setattr, soundfont, "_sampler", real_sampler) + self.addCleanup(setattr, soundfont, "open_bank", real_open) + + def test_soundfont_makes_its_voices_first(self): + inst = audioinstruments.create("soundfont", SR, bank=self.path) + self.addCleanup(inst.deinit) + self.assertEqual(self.calls, ["sampler", "bank"]) + + def test_drums_make_their_voices_first(self): + kit = audioinstruments.create("soundfont_drums", SR, bank=self.path) + self.addCleanup(kit.deinit) + self.assertEqual(self.calls, ["sampler", "bank"]) + + def test_a_bank_that_wont_open_frees_the_voices(self): + freed = [] + real_deinit = soundfont.Voices.deinit + + def deinit(pool): + freed.append(pool) + real_deinit(pool) + + soundfont.Voices.deinit = deinit + self.addCleanup(setattr, soundfont.Voices, "deinit", real_deinit) + missing = os.path.join(self.tmp, "missing.sf2") + for name in ("soundfont", "soundfont_drums"): + with self.assertRaises(OSError): + audioinstruments.create(name, SR, bank=missing) + self.assertEqual(len(freed), 2) + + def test_a_shared_pool_is_left_alone(self): + pool = soundfont.Voices(SR, 2, voice_count=4) + self.addCleanup(pool.deinit) + missing = os.path.join(self.tmp, "missing.sf2") + with self.assertRaises(OSError): + audioinstruments.create("soundfont", SR, bank=missing, + voice_pool=pool) + inst = audioinstruments.create("soundfont", SR, bank=self.path, + voice_pool=pool) + inst.note_on(60, 100) + self.assertGreater(inst.sounding(), 0) + + if __name__ == "__main__": unittest.main()