SoundFont instruments make their voices before opening the bank

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)

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)

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()