Natooz / MidiTok

MIDI / symbolic music tokenizers for Deep Learning models 🎶
https://miditok.readthedocs.io/
MIT License
665 stars 81 forks source link

Incorrect Placement of Time Signatures #131

Closed EterDelta closed 8 months ago

EterDelta commented 8 months ago

Recently, I've been testing the new version 3.0.0, particularly with MIDIs that feature multiple time signature changes.

There seems to be a regression related to the time_signature_range parameter. An issue arises when supporting more time signatures than those present in a song. In this case, the time signatures are incorrectly placed.

Steps to Reproduce

As you can see, the first signature (2/2) in the original song lasts 2 beats, but in the processed file, it now lasts for 4. If we remove the extra signature from the configuration, the problem is fixed.

This issue is also present in different signatures and songs.

example_song.zip

Natooz commented 8 months ago

Hello, 👋

Thank you very much for the bug report! 🙏 It's fixed in #132, I'll wait for the tests to pass and merge it! You then will be able to get the fix by installing from git: pip install -U git+https://github.com/Natooz/MidiTok

The error came from the time division used in the time signature preprocessing method. It used the tokenisers one (which is used when decoding tokens for example) instead of the MIDi's one, which can be different, resulting in the time signature shifts.