SpotlightKid / python-rtmidi

Python bindings for the cross-platform MIDI I/O library RtMidi
https://spotlightkid.github.io/python-rtmidi/
Other
358 stars 65 forks source link

Question about sequencer example #93

Open dfkettle opened 3 years ago

dfkettle commented 3 years ago

Hi!

This is the first time I've tried using python-rtmidi. I tried to run the sequence example (sequencer.py), but the last three notes didn't play. On a hunch, I added a pause in the 'finally' block and that took care of the problem. Is this expected behaviour or a bug?

I'm running under Python 3.8 and Windows 10.

try:
    seq.start()
    time.sleep(60. / seq.bpm * 4)
    seq.bpm = 150
    time.sleep(60. / seq.bpm * 6)
finally:
    time.sleep(5)    # added pause
    seq.stop()
    midiout.close_port()
    del midiout

Thanks.

SpotlightKid commented 3 years ago

It's not exactly expected, but I wouldn't classify it as a bug. It is probably a result of two factors: 1) the Windows MM backend is synchroneous, i.e. when you use send_message() is blocks until the message is sent out, whereas on other platforms the backends there are asynchroneous, i.e send_message returns immediately. Combined with 2) that time.sleep() resolution on Windows is not as accurate as elsewhere, the midiout.close_port() comand might just come a few milliseconds to early and prevent the last notes to be played.

dfkettle commented 3 years ago

It's not a big problem, I can just add a pause at the end of every song. And as you say, it's probably specific to Windows. But it's off by much more than a few milliseconds. The last few notes get cut off without the pause at the end. Maybe it's the way MIDI data is buffered in Windows?

mfncooper commented 2 years ago

One problem with the sequencer example is that events can be left in the pending queue after seq.stop() is called. That method causes run() to break out of its loop without checking to see what state the queues are in. Thus it's easy for a NOTE_OFF at the end to be missed (which I see on my Mac), or for the situation that @dfkettle is seeing to occur. The solution is an easy fix. Change line 155 from this:

        while not self._stopped.is_set():

to this:

        while not self._stopped.is_set() or pending:

With this fix, it's no longer necessary to artificially sleep at the end of the sequence, and line 249 (the following) can be deleted:

    time.sleep(60. / seq.bpm * 6)
SpotlightKid commented 1 year ago

The ability to stop a sequence when it is not finished is a feature, IMO. Maybe there should be a flag SequencerThread.stop(play_pending=False), but that would also require changes to the while condition in the main loop.

PRs welcome.