From 168d1879943859653e197237e9baf57f3feb909e Mon Sep 17 00:00:00 2001 From: David Robillard Date: Tue, 30 Dec 2014 23:07:19 -0500 Subject: 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. --- libs/evoral/evoral/midi_util.h | 9 +++++- libs/evoral/src/SMF.cpp | 6 +++- libs/evoral/src/libsmf/smf_load.c | 35 ++++++++++++-------- libs/evoral/src/libsmf/smf_save.c | 68 +++++++++++++++++++++++---------------- 4 files changed, 74 insertions(+), 44 deletions(-) diff --git a/libs/evoral/evoral/midi_util.h b/libs/evoral/evoral/midi_util.h index 5c72fb86c9..f32e816321 100644 --- 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 a989d01353..218e2851ff 100644 --- 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 26ab0e6a1f..1db09a7a45 100644 --- 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 02cb2084bb..05c6641b03 100644 --- 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); -- cgit v1.2.3