Merge pull request #830 from MMexicann/fix/midi-invalid-smpte-division

Reject undefined SMPTE frame rates in MIDI headers

crates/midi/src/smf.rs

@@ -122,7 +122,7 @@ impl TickConv {
             let [hi, lo] = division.to_be_bytes();
             let fps = -i128::from(hi as i8);
             let tpf = i128::from(lo);
-            if fps <= 0 || tpf == 0 {
+            if !matches!(fps, 24 | 25 | 29 | 30) || tpf == 0 {
                 return Err(MidiError::Invalid { offset, reason: "invalid SMPTE division" });
             }
             let two_ppq = 2 * i128::from(PPQ);

crates/midi/tests/smf.rs

@@ -148,6 +148,29 @@ fn division_conversion_96_and_smpte() {
     assert_eq!(smf.tracks[0].sequence.notes[0].length, 480);
 }
 
+#[test]
+fn smpte_division_rejects_undefined_frame_rates() {
+    let body = vec![0, 0x90, 60, 100, 40, 0x80, 60, 64, 0, 0xFF, 0x2F, 0];
+    for high in 0x80..=0xFF {
+        if [(-24i8) as u8, (-25i8) as u8, (-29i8) as u8, (-30i8) as u8].contains(&high) {
+            continue;
+        }
+        let division = u16::from_be_bytes([high, 40]);
+        assert!(matches!(read_smf(&file(0, division, std::slice::from_ref(&body))), Err(MidiError::Invalid { offset: 12, .. })), "{division:#06x}");
+    }
+}
+
+#[test]
+fn smpte_division_keeps_all_four_standard_frame_rates() {
+    for (fps, file_ticks, model_ticks) in [(24i8, 960, 1920), (25, 1000, 1920), (29, 1200, 1922), (30, 1200, 1920)] {
+        let body = [vec![0, 0x90, 60, 100], vlq(file_ticks), vec![0x80, 60, 64, 0, 0xFF, 0x2F, 0]].concat();
+        let division = u16::from_be_bytes([(-fps) as u8, 40]);
+        let smf = read_smf(&file(0, division, std::slice::from_ref(&body))).unwrap();
+        assert_eq!(smf.tracks[0].sequence.notes, vec![note(60, 0, model_ticks)], "{fps} fps");
+        assert!(read_smf(&file(0, u16::from_be_bytes([(-fps) as u8, 0]), std::slice::from_ref(&body))).is_err());
+    }
+}
+
 #[test]
 fn running_status_and_velocity_zero_note_off() {
     // Note-on with running status for both on and off (velocity 0).