craffel / pretty-midi

Utility functions for handling MIDI data in a nice/intuitive way.
MIT License
856 stars 151 forks source link

Type Mismatch in Calling `fluidsynth.py` #220

Closed Yao-Lirong closed 1 year ago

Yao-Lirong commented 1 year ago

In fluidsynth of instrument.py (attached as the bottom last cell), there is this following function call to fluidsynth.py (attached as the bottom second cell). The last line calls sfload, which should take in an argument of c_char_p type. However, sf2_path, according to spec of this function, is a python str. This causes the error:

return fluid_synth_sfload(self.synth, filename, update_midi_preset)
ArgumentError: argument 2: <class 'TypeError'>: wrong type

A simple fix would be to add the following line before calling sfload: (A pull request will be opened soon to provide this simple fix)

        sf2_path = c_char_p(sf2_path.encode())
        sfid = fl.sfload(sf2_path)

The following is the definition of sfload in fluidsynth.py:

    fluid_synth_sfload = cfunc('fluid_synth_sfload', c_int,
                           ('synth', c_void_p, 1),
                           ('filename', c_char_p, 1),
                           ('update_midi_presets', c_int, 1))
    def sfload(self, filename, update_midi_preset=0):
        """Load SoundFont and return its ID"""
        return fluid_synth_sfload(self.synth, filename, update_midi_preset)

The following is the original definition of fluidsynth in instrument.py.


    def fluidsynth(self, fs=44100, sf2_path=None):
        """Synthesize using fluidsynth.

        Parameters
        ----------
        fs : int
            Sampling rate to synthesize.
        sf2_path : str
            Path to a .sf2 file.
            Default ``None``, which uses the TimGM6mb.sf2 file included with
            ``pretty_midi``.

        Returns
        -------
        synthesized : np.ndarray
            Waveform of the MIDI data, synthesized at ``fs``.

        """
        # If sf2_path is None, use the included TimGM6mb.sf2 path
        if sf2_path is None:
            sf2_path = pkg_resources.resource_filename(__name__, DEFAULT_SF2)

        if not _HAS_FLUIDSYNTH:
            raise ImportError("fluidsynth() was called but pyfluidsynth "
                              "is not installed.")

        if not os.path.exists(sf2_path):
            raise ValueError("No soundfont file found at the supplied path "
                             "{}".format(sf2_path))

        # If the instrument has no notes, return an empty array
        if len(self.notes) == 0:
            return np.array([])

        # Create fluidsynth instance
        fl = fluidsynth.Synth(samplerate=fs)
        # Load in the soundfont
        sfid = fl.sfload(sf2_path)
        ...
Yao-Lirong commented 1 year ago

A pr was created to fix the described issue #221.

DavidDiazGuerra commented 1 year ago

Hello!

I think this issue was based on a quite old version of pyfluidsynth and PR #221 crashes with the newer versions. In the first message it's said that the definition of sfload in fluidsynth.py is:

    fluid_synth_sfload = cfunc('fluid_synth_sfload', c_int,
                           ('synth', c_void_p, 1),
                           ('filename', c_char_p, 1),
                           ('update_midi_presets', c_int, 1))
    def sfload(self, filename, update_midi_preset=0):
        """Load SoundFont and return its ID"""
        return fluid_synth_sfload(self.synth, filename, update_midi_preset)

But since https://github.com/nwhitehead/pyfluidsynth/commit/8c9bbaabe1688160f6be201170b26335746cc66e in 2017, the definition of 'sfload' includes a call to .encode() so filename should be a str and not a c_char_p:

    def sfload(self, filename, update_midi_preset=0):
        """Load SoundFont and return its ID"""
        return fluid_synth_sfload(self.synth, filename.encode(), update_midi_preset)

Which fixes the issue reported at the beginning of this threat and crashes with PR #221: AttributeError: 'c_char_p' object has no attribute 'encode'.

I think PR #221 should be reverted and, in any case, indicate the minimum version of pyfluidsynth compatible with pretty-midi in the documentation.