Load what we can from broken/truncated MIDI files.

We're still a very long way from tolerant of weird SMF files (libsmf takes a
"crash if input is not exactly perfect" philosophy, if we're going to be polite
and elevate such a thing to "philosophy"), but at least we'll get what's there
from files truncated by old broken versions of Ardour or other situations.

diff --git a/libs/evoral/evoral/midi_util.h b/libs/evoral/evoral/midi_util.h
index 5c72fb86c903918a6fdbc73508ac8ce65ed3961f..f32e816321e878d980fef7f1a014d8b0cf98c08d 100644 (file)
--- a/libs/evoral/evoral/midi_util.h
+++ b/libs/evoral/evoral/midi_util.h
@@ -95,7 +95,9 @@ midi_event_size(const uint8_t* buffer)
                int end;
                
                for (end = 1; buffer[end] != MIDI_CMD_COMMON_SYSEX_END; end++) {
-                       assert((buffer[end] & 0x80) == 0);
+                       if ((buffer[end] & 0x80) != 0) {
+                               return -1;
+                       }
                }
                assert(buffer[end] == MIDI_CMD_COMMON_SYSEX_END);
                return end + 1;
@@ -118,6 +120,11 @@ midi_event_is_valid(const uint8_t* buffer, size_t len)
        if (size < 0 || (size_t)size != len) {
                return false;
        }
+       for (size_t i = 1; i < len; ++i) {
+               if ((buffer[i] & 0x80) != 0) {
+                       return false;  // Non-status byte has MSb set
+               }
+       }
        return true;
 }
 

diff --git a/libs/evoral/src/SMF.cpp b/libs/evoral/src/SMF.cpp
index a989d0135331b066d066377e13e2db2d1f44597d..218e2851ff65019d15a472ae2c634522b6e0ba57 100644 (file)
--- a/libs/evoral/src/SMF.cpp
+++ b/libs/evoral/src/SMF.cpp
@@ -286,7 +286,11 @@ SMF::read_event(uint32_t* delta_t, uint32_t* size, uint8_t** buf, event_id_t* no
                memcpy(*buf, event->midi_buffer, size_t(event_size));
                *size = event_size;
 
-               assert(midi_event_is_valid(*buf, *size));
+               if (!midi_event_is_valid(*buf, *size)) {
+                       cerr << "WARNING: SMF ignoring illegal MIDI event" << endl;
+                       *size = 0;
+                       return -1;
+               }
 
                /* printf("SMF::read_event @ %u: ", *delta_t);
                   for (size_t i = 0; i < *size; ++i) {

diff --git a/libs/evoral/src/libsmf/smf_load.c b/libs/evoral/src/libsmf/smf_load.c
index 26ab0e6a1f82bd6479579b3e6639a9be9dac124f..1db09a7a458ecda1ca0a3b88084d4f79d0fa1ba9 100644 (file)
--- a/libs/evoral/src/libsmf/smf_load.c
+++ b/libs/evoral/src/libsmf/smf_load.c
@@ -83,7 +83,6 @@ next_chunk(smf_t *smf)
 
        if (smf->next_chunk_offset > smf->file_buffer_length) {
                g_critical("SMF error: malformed chunk; truncated file?");
-               return (NULL);
        }
 
        return (chunk);
@@ -396,7 +395,10 @@ extract_sysex_event(const unsigned char *buf, const size_t buffer_length, smf_ev
 
        status = *buf;
 
-       assert(is_sysex_byte(status));
+       if (!(is_sysex_byte(status))) {
+               g_critical("Corrupt sysex status byte in extract_sysex_event().");
+               return (-6);
+       }
 
        c++;
 
@@ -439,7 +441,10 @@ extract_escaped_event(const unsigned char *buf, const size_t buffer_length, smf_
 
        status = *buf;
 
-       assert(is_escape_byte(status));
+       if (!(is_escape_byte(status))) {
+               g_critical("Corrupt escape status byte in extract_escaped_event().");
+               return (-6);
+       }
 
        c++;
 
@@ -787,6 +792,7 @@ static int
 parse_mtrk_chunk(smf_track_t *track)
 {
        smf_event_t *event;
+       int ret = 0;
 
        if (parse_mtrk_header(track))
                return (-1);
@@ -795,10 +801,10 @@ parse_mtrk_chunk(smf_track_t *track)
                event = parse_next_event(track);
 
                /* Couldn't parse an event? */
-               if (event == NULL)
-                       return (-1);
-
-               assert(smf_event_is_valid(event));
+               if (event == NULL || !smf_event_is_valid(event)) {
+                       ret = -1;
+                       break;
+               }
 
                if (event_is_end_of_track(event))
                        break;
@@ -808,7 +814,7 @@ parse_mtrk_chunk(smf_track_t *track)
        track->file_buffer_length = 0;
        track->next_event_offset = -1;
 
-       return (0);
+       return (ret);
 }
 
 /**
@@ -870,6 +876,7 @@ smf_t *
 smf_load_from_memory(const void *buffer, const size_t buffer_length)
 {
        int i;
+       int ret;
 
        smf_t *smf = smf_new();
 
@@ -887,16 +894,16 @@ smf_load_from_memory(const void *buffer, const size_t buffer_length)
 
                smf_add_track(smf, track);
 
-               /* Skip unparseable chunks. */
-               if (parse_mtrk_chunk(track)) {
-                       g_warning("SMF warning: Cannot load track.");
-                       smf_track_delete(track);
-                       continue;
-               }
+               ret = parse_mtrk_chunk(track);
 
                track->file_buffer = NULL;
                track->file_buffer_length = 0;
                track->next_event_offset = -1;
+
+               if (ret) {
+                       g_warning("SMF warning: Error parsing track, continuing with data loaded so far.");
+                       break;
+               }
        }
 
        if (smf->expected_number_of_tracks != smf->number_of_tracks) {

diff --git a/libs/evoral/src/libsmf/smf_save.c b/libs/evoral/src/libsmf/smf_save.c
index 02cb2084bba0dde3fab40fc77486478a0bb2fa6d..05c6641b03c4cfaf7c495f5edbbfb8bcb45c8274 100644 (file)
--- a/libs/evoral/src/libsmf/smf_save.c
+++ b/libs/evoral/src/libsmf/smf_save.c
@@ -547,58 +547,68 @@ smf_validate(smf_t *smf)
 
 #ifndef NDEBUG
 
-static void
-assert_smf_event_is_identical(const smf_event_t *a, const smf_event_t *b)
+#define CHECK(cond) if (!(cond)) { return -1; }
+
+static int
+check_smf_event_is_identical(const smf_event_t *a, const smf_event_t *b)
 {
-       assert(a->event_number == b->event_number);
-       assert(a->delta_time_pulses == b->delta_time_pulses);
-       assert(abs((long)(a->time_pulses - b->time_pulses)) <= 2);
-       assert(fabs(a->time_seconds - b->time_seconds) <= 0.01);
-       assert(a->track_number == b->track_number);
-       assert(a->midi_buffer_length == b->midi_buffer_length);
-       assert(memcmp(a->midi_buffer, b->midi_buffer, a->midi_buffer_length) == 0);
+       CHECK(a->event_number == b->event_number);
+       CHECK(a->delta_time_pulses == b->delta_time_pulses);
+       CHECK(abs((long)(a->time_pulses - b->time_pulses)) <= 2);
+       CHECK(fabs(a->time_seconds - b->time_seconds) <= 0.01);
+       CHECK(a->track_number == b->track_number);
+       CHECK(a->midi_buffer_length == b->midi_buffer_length);
+       CHECK(memcmp(a->midi_buffer, b->midi_buffer, a->midi_buffer_length) == 0);
+       return 0;
 }
 
-static void
-assert_smf_track_is_identical(const smf_track_t *a, const smf_track_t *b)
+static int
+check_smf_track_is_identical(const smf_track_t *a, const smf_track_t *b)
 {
        size_t i;
 
-       assert(a->track_number == b->track_number);
-       assert(a->number_of_events == b->number_of_events);
+       CHECK(a->track_number == b->track_number);
+       CHECK(a->number_of_events == b->number_of_events);
 
        for (i = 1; i <= a->number_of_events; i++)
-               assert_smf_event_is_identical(smf_track_get_event_by_number(a, i), smf_track_get_event_by_number(b, i));
+               check_smf_event_is_identical(smf_track_get_event_by_number(a, i), smf_track_get_event_by_number(b, i));
+
+       return 0;
 }
 
-static void
-assert_smf_is_identical(const smf_t *a, const smf_t *b)
+static int
+check_smf_is_identical(const smf_t *a, const smf_t *b)
 {
        int i;
 
-       assert(a->format == b->format);
-       assert(a->ppqn == b->ppqn);
-       assert(a->frames_per_second == b->frames_per_second);
-       assert(a->resolution == b->resolution);
-       assert(a->number_of_tracks == b->number_of_tracks);
+       CHECK(a->format == b->format);
+       CHECK(a->ppqn == b->ppqn);
+       CHECK(a->frames_per_second == b->frames_per_second);
+       CHECK(a->resolution == b->resolution);
+       CHECK(a->number_of_tracks == b->number_of_tracks);
 
        for (i = 1; i <= a->number_of_tracks; i++)
-               assert_smf_track_is_identical(smf_get_track_by_number(a, i), smf_get_track_by_number(b, i));
+               check_smf_track_is_identical(smf_get_track_by_number(a, i), smf_get_track_by_number(b, i));
 
        /* We do not need to compare tempos explicitly, as tempo is always computed from track contents. */
+       return 0;
 }
 
-static void
-assert_smf_saved_correctly(const smf_t *smf, FILE* file)
+static int
+check_smf_saved_correctly(const smf_t *smf, FILE* file)
 {
        smf_t *saved;
+       int ret;
 
        saved = smf_load (file);
-       assert(saved != NULL);
-
-       assert_smf_is_identical(smf, saved);
+       if (!saved) {
+               ret = -1;
+       } else {
+               ret = check_smf_is_identical(smf, saved);
+       }
 
        smf_delete(saved);
+       return (ret);
 }
 
 #endif /* !NDEBUG */
@@ -645,7 +655,9 @@ smf_save(smf_t *smf, FILE* file)
                return (error);
 
 #ifndef NDEBUG
-       assert_smf_saved_correctly(smf, file);
+       if (check_smf_saved_correctly(smf, file)) {
+               g_warning("SMF warning: Did not save correctly, possible data loss.");
+       }
 #endif
 
        return (0);