io: import a MIDI file with stray bytes after its last track (#1528) (#1540)

* io: import a MIDI file with stray bytes after its last track (#1528)

Importing a MIDI file read it with craft-midi's strict read_smf, which
refuses any damage. The file in #1528 is whole, its one track ending with
End of Track at its declared length, but two line feeds follow it. The
strict reader took them for the start of another chunk and refused the
file with "unexpected end of data at byte 45281".

Read imported files with read_smf_lenient instead, as other MIDI readers
do: it reads a sound file exactly as read_smf does, ignores bytes after
the last chunk, and keeps what makes sense of a damaged track, bounded by
the same event and track limits. Its warnings are logged. Something that
is not a Standard MIDI File is still refused, and the file is still read
in full before the session changes (#911).

Test: a complete file followed by two line feeds imports its note; it
failed with the issue's error before.

* io: a MIDI file past the reader's limits stays refused (#1528)

Read leniently, a file with more events or tracks than craft-midi reads
kept what came before the limit and left the rest out with only a log
warning, where the strict reader refused it. Import now reads a file
strictly first: a sound file reads as before, a file past the event or
track limit is refused as before, and only other damage (stray bytes
after the last track) falls back to the lenient reader.

Test: a file one track past the limit, its last track holding a note and
stray bytes after it, is refused and leaves the session empty.

* io: refuse a MIDI file past the event limit after a damaged track (#1528)

The strict reader stops at the first damage. When a damaged track came
before the one that goes past MAX_EVENTS, the strict error was not a
limit error, the import fell back to the lenient read, and that read
kept what came before the limit and left the rest out with a warning:
the file was imported cut short.

The lenient read now says when a limit left part of the file out
(LenientRead::over_limit, storytold/craft-libs#1308), and the import
refuses such a file before the session changes, warnings full or not.

The six craft-libs crates pinned at 6a86adb move together to that PR's
commit, as craft-scraft-format uses craft-midi's and craft-audio's
types. They move to its merge commit once it merges.

* deps: keep only the craft-libs source changes in Cargo.lock (#1528)

Moving the craft-libs pins also re-resolved several windows-sys edges
(0.61.2 to 0.52.0) and tempfile's getrandom edge (0.4.3 to 0.3.4).
Restore main's resolution for those, so the lockfile only changes the
source of the craft-libs crates.

* deps: pin craft-libs to the merge of craft-libs#1308 (#1528)

Move the six craft-libs crates from the PR's commit 4db60c4 to its merge
into craft-libs main, 45a8e16. None of these crates, nor craft-zip which
craft-scraft-format uses, changed between the two.

.github/workflows/nix.yml

@@ -23,43 +23,5 @@ jobs:
     uses: storytold/craft-common-actions/.github/workflows/nix.yml@main
 
   git-sources:
-    name: Git dependencies come from storytold
-    runs-on: ubuntu-latest
-    timeout-minutes: 5
-    steps:
-      - name: Harden runner
-        uses: step-security/harden-runner@1d88384fd173af6ecb012dfd0264a306b711241a # v2.22.4
-        with:
-          egress-policy: audit
-      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-      - name: Git dependencies come from storytold
-        run: |
-          set -euo pipefail
-          test -f Cargo.lock || { echo "Cargo.lock is missing"; exit 1; }
-          # Source replacement, patches and path overrides in cargo config never show up
-          # in Cargo.lock. Parsed rather than grepped: TOML has many spellings of one key.
-          # Cargo still reads the legacy .cargo/config.
-          python3 - <<'PY'
-          import os, sys, tomllib
-          for path in (".cargo/config", ".cargo/config.toml"):
-              if not os.path.exists(path):
-                  continue
-              config = tomllib.load(open(path, "rb"))
-              replaced = [name for name, source in config.get("source", {}).items()
-                          if isinstance(source, dict) and "replace-with" in source]
-              if replaced or "patch" in config or "paths" in config:
-                  print(f"{path} replaces, patches or overrides a source")
-                  sys.exit(1)
-          PY
-          # Fails when Cargo.lock is out of date with the manifests.
-          cargo metadata --locked --format-version 1 > /dev/null
-          python3 - <<'PY'
-          import sys, tomllib
-          lock = tomllib.load(open("Cargo.lock", "rb"))
-          bad = sorted({p["source"] for p in lock.get("package", [])
-                        if p.get("source", "").startswith("git+")
-                        and not p["source"].startswith("git+https://github.com/storytold/")})
-          if bad:
-              print("Git dependencies outside github.com/storytold:", *bad, sep="\n")
-              sys.exit(1)
-          PY
+    # Tracks @main so suite-wide CI fixes reach every app without a PR in each repository.
+    uses: storytold/craft-common-actions/.github/workflows/git-sources.yml@main

Cargo.lock

@@ -1064,7 +1064,7 @@ dependencies = [
 [[package]]
 name = "craft-audio"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "craft-media",
  "log",
@@ -1109,7 +1109,7 @@ dependencies = [
 [[package]]
 name = "craft-media"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "serde",
  "thiserror 2.0.21",
@@ -1118,7 +1118,7 @@ dependencies = [
 [[package]]
 name = "craft-midi"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "serde",
  "thiserror 2.0.21",
@@ -1127,7 +1127,7 @@ dependencies = [
 [[package]]
 name = "craft-scraft-format"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "craft-audio",
  "craft-midi",
@@ -1141,7 +1141,7 @@ dependencies = [
 [[package]]
 name = "craft-time"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "serde",
  "thiserror 2.0.21",
@@ -1198,7 +1198,7 @@ dependencies = [
 [[package]]
 name = "craft-video"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "craft-media",
  "rayon",
@@ -1208,7 +1208,7 @@ dependencies = [
 [[package]]
 name = "craft-zip"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=6a86adb58a6e8e1929484a4514600ed54bca36dc#6a86adb58a6e8e1929484a4514600ed54bca36dc"
+source = "git+https://github.com/storytold/craft-libs?rev=45a8e16669e70dadb780dfbc759c882dd5b7ea1c#45a8e16669e70dadb780dfbc759c882dd5b7ea1c"
 dependencies = [
  "flate2",
  "thiserror 2.0.21",

Cargo.toml

@@ -51,16 +51,16 @@ cpal = "0.18"
 craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "ea24422ae9859dc5da398d0462bc87f8e02e0d10" }
 
 # Video containers and bitstreams, shared with the other craft apps.
-craft-media = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6a86adb58a6e8e1929484a4514600ed54bca36dc" }
+craft-media = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "45a8e16669e70dadb780dfbc759c882dd5b7ea1c" }
 # H.264 and ProRes codecs shared with the other Craft apps.
-craft-video = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6a86adb58a6e8e1929484a4514600ed54bca36dc", default-features = false, features = ["h264-decode", "prores"] }
+craft-video = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "45a8e16669e70dadb780dfbc759c882dd5b7ea1c", default-features = false, features = ["h264-decode", "prores"] }
 # Audio file formats, codecs and waveform overviews, shared with the other craft apps. It shares
 # types with craft-media, so both stay on one rev.
 # Standard MIDI Files and the note/controller model, shared with the other craft apps.
-craft-midi = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6a86adb58a6e8e1929484a4514600ed54bca36dc", features = ["serde"] }
-craft-audio = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6a86adb58a6e8e1929484a4514600ed54bca36dc", features = ["decode", "lossless-encode", "peaks", "serde"] }
-craft-time = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6a86adb58a6e8e1929484a4514600ed54bca36dc" }
-craft-scraft-format = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6a86adb58a6e8e1929484a4514600ed54bca36dc" }
+craft-midi = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "45a8e16669e70dadb780dfbc759c882dd5b7ea1c", features = ["serde"] }
+craft-audio = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "45a8e16669e70dadb780dfbc759c882dd5b7ea1c", features = ["decode", "lossless-encode", "peaks", "serde"] }
+craft-time = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "45a8e16669e70dadb780dfbc759c882dd5b7ea1c" }
+craft-scraft-format = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "45a8e16669e70dadb780dfbc759c882dd5b7ea1c" }
 # craft-update: app-specific pretend versions apply to startup and manual checks.
 craft-update = { version = "=0.1.0", git = "https://github.com/storytold/craft-libs", rev = "ea24422ae9859dc5da398d0462bc87f8e02e0d10", features = ["egui"] }
 

crates/engine/src/io/midi.rs

@@ -12,8 +12,35 @@ pub(super) const MAX_MIDI_TRACKS: usize = 512;
 /// The file is checked in full before the session changes, so a refused file (too many tempo or
 /// meter changes or tracks, a track longer than a day) leaves the session as it was: callers that
 /// keep their own undo step, like the web shell, never see a half-applied import.
+///
+/// A sound file is read strictly. A damaged one is read leniently, as other MIDI readers do: with
+/// stray bytes after the last track (#1528), a track length its End of Track contradicts or a
+/// track cut short, what can be read is imported and what could not is logged. A file past the
+/// reader's event or track limit is still refused, also when other damage comes first: read
+/// leniently it would lose the rest without saying so. Something that is not a Standard MIDI File
+/// is refused.
 pub fn import_midi_bytes(e: &mut Engine, bytes: &[u8], at: Samples, tempo_map: bool) -> Result<Vec<TrackId>> {
-    let smf = soundcraft_midi::read_smf(bytes).map_err(|err| EngineError::Io(err.to_string()))?;
+    let smf = match soundcraft_midi::read_smf(bytes) {
+        Ok(smf) => smf,
+        Err(err @ (soundcraft_midi::MidiError::TooManyEvents(_) | soundcraft_midi::MidiError::TooManyTracks(_))) => {
+            return Err(EngineError::Io(err.to_string()));
+        }
+        Err(_) => {
+            let read = soundcraft_midi::read_smf_lenient(bytes).map_err(|err| EngineError::Io(err.to_string()))?;
+            // The strict reader stops at the first damage, so a limit further on shows only here.
+            if read.over_limit {
+                return Err(EngineError::Io(format!(
+                    "MIDI file: more than {} events or {} tracks",
+                    soundcraft_midi::MAX_EVENTS,
+                    soundcraft_midi::MAX_TRACKS
+                )));
+            }
+            for warning in &read.warnings {
+                log::warn!("MIDI import: {warning}");
+            }
+            read.smf
+        }
+    };
     if tempo_map {
         // Each meter is placed by walking the ones before it.
         if smf.meters.len() > MAX_MIDI_METERS {

crates/engine/src/io/render.rs

@@ -177,6 +177,71 @@ mod name_tests {
         }
     }
 
+    #[test]
+    fn a_midi_file_with_stray_bytes_after_its_last_track_is_imported() {
+        // #1528: a complete format-0 file followed by two line feeds, too short to be a chunk.
+        let note = soundcraft_midi::Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start: 0, length: 960 };
+        let sequence = soundcraft_midi::Sequence { notes: vec![note], ..Default::default() };
+        let smf = soundcraft_midi::Smf { format: 0, tracks: vec![soundcraft_midi::SmfTrack { name: String::new(), sequence }], ..Default::default() };
+        let mut bytes = soundcraft_midi::write_smf(&smf).unwrap();
+        bytes.extend_from_slice(b"\n\n");
+        let mut e = Engine::default();
+        let ids = import_midi_bytes(&mut e, &bytes, 0, true).unwrap();
+        assert_eq!(ids.len(), 1);
+        match &e.session().track(ids[0]).unwrap().clips()[0].content {
+            ClipContent::Midi { sequence } => assert_eq!(sequence.notes, vec![note]),
+            _ => panic!("expected a MIDI clip"),
+        }
+        // Something that is no MIDI file is still refused, and leaves the session as it was.
+        let mut e = Engine::default();
+        assert!(import_midi_bytes(&mut e, b"RIFF\0\0\0\0WAVE", 0, true).is_err());
+        assert!(e.session().tracks.is_empty());
+    }
+
+    #[test]
+    fn a_midi_file_past_the_track_limit_is_refused_even_with_stray_bytes() {
+        // Read leniently, the tracks past the limit would be left out without a word: the note in
+        // the last one would be lost. The file stays refused, and the session as it was.
+        let tracks = soundcraft_midi::MAX_TRACKS + 1;
+        let mut bytes = b"MThd\0\0\0\x06\0\x01".to_vec();
+        bytes.extend_from_slice(&u16::try_from(tracks).unwrap().to_be_bytes());
+        bytes.extend_from_slice(&960u16.to_be_bytes());
+        for _ in 1..tracks {
+            bytes.extend_from_slice(b"MTrk\0\0\0\x04\0\xFF\x2F\0");
+        }
+        // A note on, a quarter note later its note off, then End of Track.
+        bytes.extend_from_slice(b"MTrk\0\0\0\x0D\0\x90\x3C\x64\x87\x40\x80\x3C\x40\0\xFF\x2F\0");
+        bytes.extend_from_slice(b"\n\n");
+        let mut e = Engine::default();
+        let err = import_midi_bytes(&mut e, &bytes, 0, true).unwrap_err();
+        assert!(err.to_string().contains("too many tracks"), "{err}");
+        assert!(e.session().tracks.is_empty());
+    }
+
+    #[test]
+    fn a_midi_file_past_the_event_limit_is_refused_after_a_damaged_track() {
+        // The strict reader stops at track 1's damage, before track 2 reaches the event limit; read
+        // leniently, track 2's note after the limit (pitch 72) would be lost without a word.
+        let mut damaged = b"\0\x3C\x40".to_vec(); // a data byte with no status before it
+        damaged.extend_from_slice(b"\0\xFF\x2F\0");
+        let mut long = b"\0\x90\x3C\x64".to_vec();
+        for _ in 0..soundcraft_midi::MAX_EVENTS {
+            long.extend_from_slice(b"\0\x3C\0"); // running status: note off
+        }
+        long.extend_from_slice(b"\0\x90\x48\x64\x87\x40\x80\x48\x40\0\xFF\x2F\0");
+        let mut bytes = b"MThd\0\0\0\x06\0\x01\0\x02\x03\xC0".to_vec();
+        for body in [&damaged, &long] {
+            bytes.extend_from_slice(b"MTrk");
+            bytes.extend_from_slice(&u32::try_from(body.len()).unwrap().to_be_bytes());
+            bytes.extend_from_slice(body);
+        }
+        assert!(!matches!(soundcraft_midi::read_smf(&bytes), Err(soundcraft_midi::MidiError::TooManyEvents(_)) | Ok(_)));
+        let mut e = Engine::default();
+        let err = import_midi_bytes(&mut e, &bytes, 0, true).unwrap_err();
+        assert!(err.to_string().contains("events"), "{err}");
+        assert!(e.session().tracks.is_empty());
+    }
+
     #[test]
     fn midi_longer_than_a_day_is_refused() {
         let held = soundcraft_midi::Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start: 0, length: (1 << 28) - 1 };

crates/midi/src/lib.rs

@@ -2,8 +2,9 @@
 //!
 //! - [`Note`], [`CtrlEvent`] and [`Sequence`]: the MIDI content of a region / track, in ticks at
 //!   [`PPQ`] (960) ticks per quarter note; [`read_smf`] / [`write_smf`]: Standard MIDI File import
-//!   and export. These come from `craft-midi` in craft-libs, shared with the other Craft apps, and
-//!   are re-exported so the rest of SoundCraft names them from here.
+//!   and export, and [`read_smf_lenient`] for files that are slightly damaged. These come from
+//!   `craft-midi` in craft-libs, shared with the other Craft apps, and are re-exported so the rest
+//!   of SoundCraft names them from here.
 //! - [`ops`]: MIDI editing operations (quantize, transpose, velocity, duration, split).
 //! - [`note_name`], [`gm_drum_name`], [`gm_program_name`]: display names.
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
@@ -12,6 +13,8 @@ mod names;
 pub mod ops;
 mod rng;
 
-pub use craft_midi::{CtrlEvent, CtrlKind, MAX_EVENTS, MAX_TRACKS, MidiError, Note, PPQ, Sequence, Smf, SmfTrack, read_smf, write_smf};
+pub use craft_midi::{
+    CtrlEvent, CtrlKind, LenientRead, MAX_EVENTS, MAX_TRACKS, MidiError, Note, PPQ, Sequence, Smf, SmfTrack, read_smf, read_smf_lenient, write_smf,
+};
 pub use names::{gm_drum_name, gm_program_name, note_name};
 pub use rng::XorShift;

crates/ui-egui/src/edit_window/clips.rs

@@ -482,7 +482,7 @@ pub(super) fn clip_marks(clip: &Clip, show_rating: bool) -> String {
 
 /// Gap between a pill and the clip outline's inner edge (so the pill sits 1 pt inside the outline).
 pub(super) const PILL_INSET: f32 = 2.0;
-pub(super) const PILL_HEIGHT: f32 = 13.0;
+pub(super) const PILL_HEIGHT: f32 = 14.0;
 /// Horizontal padding between a pill's edge and its text.
 pub(super) const PILL_PAD: f32 = 3.0;
 /// The narrowest name pill worth drawing: its padding plus about one letter.