pretty-midi icon indicating copy to clipboard operation
pretty-midi copied to clipboard

Writing and reading back in should be roughly lossless

Open craffel opened this issue 10 years ago • 11 comments

I.e. if you write a MIDI file out and read it back in, you should get at least the same collection of notes, time signature changes, tempo changes, pitch bends, key signatures, etc. (all of the data that pretty_midi stores)

craffel avatar Nov 25 '15 02:11 craffel

I was going to file a bug but I'll attach it to this bug. In TimeSignature, you don't keep around the notated 32nd-notes in a MIDI quarter-note (24 MIDI Clocks) so when you write a file, this information is lost. An incoming TimeSignature might be:
midi.TimeSignatureEvent(tick=0, data=[6, 3, 36, 8]). If you load that into pretty_midi and write it back out you get: midi.TimeSignatureEvent(tick=0, data=[6, 3, 0, 0]),

If you don't want to store info, you could at least set some reasonable default such as you do when you write the default 4/4 time signature: timing_track += [midi.TimeSignatureEvent(tick=0, data=[4, 2, 24, 8])] (24 midi clocks per metronome click, 8 32nd notes in a midi qn)

That seems like a reasonable default. I personally don't care all that much. I just don't want these files to break some sequencer software because of 0 values in these fields.

douglaseck avatar Mar 09 '16 21:03 douglaseck

Also it's probably not a good idea to always add a default time signature. At the very least only add a time signature if there's not already a time signature defined at time 0.

douglaseck avatar Mar 09 '16 22:03 douglaseck

In TimeSignature, you don't keep around the notated 32nd-notes in a MIDI quarter-note (24 MIDI Clocks) so when you write a file, this information is lost.

Ah, interesting, I hadn't noticed that we were missing that information when writing out (here). Do you know if this is totally meta-information, or will they ever effect playback?

Also it's probably not a good idea to always add a default time signature. At the very least only add a time signature if there's not already a time signature defined at time 0.

I think I read somewhere that it's bad practice in a MIDI file to not include any time signature changes, but it turns out about 10% of the MIDI files I've found "in the wild" have no time signature events. So, not sure which the best choice is, but I agree that we shouldn't add a default time signature at time 0 if there's already one there.

craffel avatar Mar 10 '16 02:03 craffel

I doubt if it affects playback. It broke a unit test that verifies that a PM to proto to PM loop creates the same prettymidi info. Seems worthwhile doing the minimally invasive fix for now. Those kinds of tests are useful and in complex pipelines your could end up with a lot of 4/4 meter events :-). I should learn to really work with git and submit a fix myself. But as before the workflow is so different from Google that I forget. On Mar 9, 2016 6:02 PM, "Colin Raffel" [email protected] wrote:

In TimeSignature, you don't keep around the notated 32nd-notes in a MIDI quarter-note (24 MIDI Clocks) so when you write a file, this information is lost.

Ah, interesting, I hadn't noticed that we were missing that information when writing out (here https://github.com/craffel/pretty-midi/blob/master/pretty_midi/pretty_midi.py#L919). Do you know if this is totally meta-information, or will they ever effect playback?

Also it's probably not a good idea to always add a default time signature. At the very least only add a time signature if there's not already a time signature defined at time 0.

I think I read somewhere that it's bad practice in a MIDI file to not include any time signature changes, but it turns out about 10% of the MIDI files I've found "in the wild" have no time signature events. So, not sure which the best choice is, but I agree that we shouldn't add a default time signature at time 0 if there's already one there.

— Reply to this email directly or view it on GitHub https://github.com/craffel/pretty-midi/issues/52#issuecomment-194618052.

douglaseck avatar Mar 10 '16 03:03 douglaseck

Seems worthwhile doing the minimally invasive fix for now.

For sure. Since this issue is kind of a far-reaching one, I created a separate issue, which I can resolve soon #61.

craffel avatar Mar 10 '16 03:03 craffel

Out of curiosity, I grabbed 10,000 random MIDI files and computed the relative occurrence of different values in the third and fourth data slots of TimeSignatureChange events:

Number of MIDI ticks in a metronome click
24: 72.33%
12: 10.18%
96:  9.14%
0:   1.50%
36:  1.46%
48:  1.42%
6:   0.82%
...
Number of notated 32nd notes in a MIDI quarter note
8:   97.00%
42:   0.45%
161:  0.24%
239:  0.17%
10:   0.12%
227:  0.12%
82:   0.11%
...

Thank goodness 8 appears in slot 4 97% of the time, I'm curious about the files for which it doesn't (161 32nd notes in a quarter note in 24 different time signature change events??)... I would think that slot 3 only effects metronomes, so can be safely ignored (with a suitable warning saying it is always set to 24 on write-out), but I am a little suspicious that having a "number of 32nd notes per quarter note" which is not 8 will effect playback. I will investigate, at some point.

craffel avatar Mar 10 '16 04:03 craffel

bach-js_kunst_der_fuge_01_(c)unknown[2].mid:

image

After writing and reading back artifact appear (very long notes).

bzamecnik avatar Jul 17 '17 15:07 bzamecnik

Can you post this MIDI file somewhere so we can test it out?

craffel avatar Jul 17 '17 16:07 craffel

https://filebin.ca/3ThfIaDkkjvD/bach-js_kunst_der_fuge_01_cunknown2.mid

bzamecnik avatar Jul 17 '17 17:07 bzamecnik

I created a separate issue #131 with more details about the cause.

bzamecnik avatar Jul 18 '17 05:07 bzamecnik

Related: #161 "Inconsistent behaviour for short (zero-duration) notes"

cifkao avatar Feb 06 '19 14:02 cifkao