adjust timing of note events according to tempo changes #427
Conversation
There was a problem hiding this comment.
Very nice and swift bug fix! There are three points that could be addressed:
- Also adjusting the time of time and key signature messages, as well as meta messages
- For clarity, use the helper method
midi_ticks_to_seconds(to make clear what the line is computed - Adding typing annotations to
adjust_time
partitura/io/importmidi.py
Outdated
|
|
||
| perf = performance.Performance( | ||
| id=doc_name, | ||
| performedparts=pps, | ||
| ) | ||
| return perf | ||
|
|
||
| def adjust_time(tick, tempo_changes, ppq): |
There was a problem hiding this comment.
Would it be possible to add typing annotations to this function? (we should try to include this in more things going forward)
partitura/io/importmidi.py
Outdated
| for change_tick, mpq in tempo_changes: | ||
| if tick < change_tick: | ||
| break | ||
| time += (change_tick - last_tick) * (last_mpq / (ppq * 10**6)) |
There was a problem hiding this comment.
Nice! Maybe we can use the midi_ticks_to_seconds (from partitura.utils.music) method here, and also below, so that it is clear what this line is computing?. It would be something like
time += midi_ticks_to_seconds(midi_ticks=(change_tick - last tick), mpq=last_mpq, ppq=ppq)
(This would be mostly to make clear what this method is computing, but if you think that it makes things less clear, feel free to leave it as it is)
There was a problem hiding this comment.
Thanks for the review! I added all the changes accordingly and extended the test case to also check for changing time and key signatures.
…moved to pt util function for converting ticks to seconds
No description provided.