Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Thysbelon opened this issue Apr 11, 2024 · 5 comments
Closed

Comments

@Thysbelon
Copy link

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:

  • OS: Ubuntu 23.10
  • Browser: Firefox 124.0.2 (64-bit)

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
Copy link
Collaborator

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
Copy link
Collaborator

robertnhart commented Apr 12, 2024

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
Copy link
Owner

ryohey commented Apr 18, 2024

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

@robertnhart
Copy link
Collaborator

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
Copy link
Owner

ryohey commented May 11, 2024

@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.

@ryohey ryohey closed this as completed in 965766f May 12, 2024
ryohey added a commit that referenced this issue May 12, 2024
@ryohey ryohey mentioned this issue May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants