ryohey / signal

Online MIDI Editor: signal
https://signal.vercel.app
MIT License
1.2k stars 136 forks source link

Midi files with a low PPQ cause timing issues when imported into signal #334

Closed Thysbelon closed 1 month ago

Thysbelon commented 2 months ago

Describe the bug

If a midi file with a low PPQ is opened in signal, adding triplet notes can cause the notes to be placed at non-integer tick positions, such as 1045.33333333. When a signal project like this is exported back to midi, the notes with non-integer tick positions, as well as all the notes around them in the same track, will have their timing altered more than just rounding their positions; causing notes to play off beat.
If there are many notes with non-integer tick positions in one track, these notes can have their position changed by as much as a quarter note when the song is exported to midi.

To Reproduce

Steps to reproduce the behavior:

  1. Create a midi file using a program that exports midi with a low PPQ (example: LMMS)
  2. Open the midi file in Signal
  3. Place a triplet note in any position that is not lined up with the vertical rhythm grid lines
  4. Open the event list and observe that the note is placed at a non-integer tick
  5. Place any note in any position (integer or not) after the triplet note(s)
  6. Click File > save to export to midi
  7. Open the exported midi file in any music editor and observe that all notes that had non-integer positions have been changed to integer positions; observe that all notes that play after the triplet note(s), even if they had an integer position, have also had their positions altered with a difference of ≥1 tick.

Expected behavior

The position of notes does not change when exporting, or any change in position that occurs has a difference of <1 tick.

Screenshots

Before exporting from Signal:

Screenshot from 2024-04-11 18-05-19 Screenshot from 2024-04-11 18-04-12

After exporting:

Screenshot from 2024-04-11 18-05-23 Screenshot from 2024-04-11 18-04-55

Desktop:

Additional context

The exact version of LMMS I experienced this issue with was 1.3.0-alpha.1.542+g3f5ac806e (Linux/x86_64, Qt 5.9.5, GCC 7.5.0) AppImage.
Maybe this could be fixed by altering all imported midi files to use the same PPQ as midi files created in Signal.

robertnhart commented 2 months ago

It might be clearer to describe the issue as "Note duration that is not an integer number of ticks causes positions of following notes to shift earlier upon saving".

Using triplet notes will be a problem for any PPQ resolution value that is not evenly divisible by 3. The problematic position shifting will accumulate more quickly the lower the PPQ value is.

In other words, when attempting to use triplet notes, I think the problem is not "low PPQ" by itself, but rather "a PPQ that isn't divisible by 3 -- and it's worse the lower it is".

.

Here are more specific numbers for the example shown:

The screenshots show a MIDI file that uses a resolution of 128 ticks per quarter note and a time signature of 4/4.

After opening this MIDI file in Signal, the pencil tool was used to fill the first six measures with a sequence of triplet half notes, then continued with some quarter notes.

Initially, the Signal Event List shows the triplet half notes with a duration of 170 2/3 ticks each and at positions that are multiples of 170 2/3 ticks. After the first 6 measures, the next note has a position of 3072 ticks.

After saving and re-opening the MIDI file, Signal's Event List now shows the triplet half notes have a duration of 170 ticks each and are at positions that are multiples of 170 ticks. After the first 6 measures, the next note now has a position of 3060 ticks.

.

If the initial MIDI file had a resolution of 24 ticks per quarter note (a low resolution value that is divisible by 3), the same example sequence of triplet half notes would have durations of exactly 32 ticks each and their tick positions would remain the same after saving and re-opening the file.

On the other had, if the initial MIDI file has a resolution of 32 ticks per quarter note (a low resolution value that is not divisible by 3), the same example sequence of triplet half notes for six 4/4 measures will cause the position of the next note after that to shift almost an eighth note earlier.

robertnhart commented 2 months ago

Here is my suggested fix:

In the file src/common/helpers/toRawEvents.ts in the function addDeltaTime change the following line

deltaTime: e.tick - prevTick,

to this instead

deltaTime: Math.round(e.tick) - Math.round(prevTick),

.

I tested this fix using the same example: Starting with a MIDI file with a resolution of 128 ticks per quarter note and a time signature of 4/4, inserting a 6-measure sequence of triplet half notes followed by some quarter notes, then saving and re-opening the file.

Before saving, the event list still showed the triplet half notes with durations of 170 2/3 ticks and positions that are multiples of 170 2/3 ticks. But during the save process the durations in the saved file became a repeating sequence of 171, 170, and 171 ticks for each measure. After 6 measures, the next note position remained at the correct position of 3072 ticks.

ryohey commented 2 months ago

Thank you for the very detailed explanation! I will check it later.

robertnhart commented 1 month ago

If you want to try out my change above, I wrote some code you can use to patch Signal using the browser console or a JavaScript bookmark.

Signal Customizations

ryohey commented 1 month ago

@robertnhart It would be very helpful! By the way, is it possible for you to send me a PR? I think it is important that it is clear what you have contributed.