API Sysex fixes

Fix memory leaks by using stack instead of malloc
Reduce memory usage by having less temporary bufffers
Remove warnings by adding includes
Decrease code size by 608 bytes (mostly due to not linking malloc)
More robust handling of buffer overflows
This commit is contained in:
Fred Sundvik 2016-12-29 12:13:30 +02:00
parent 273faa4d9c
commit dd685eceb2
4 changed files with 71 additions and 27 deletions

View File

@ -1,4 +1,6 @@
#include "api_sysex.h" #include "api_sysex.h"
#include "sysex_tools.h"
#include "print.h"
void send_bytes_sysex(uint8_t message_type, uint8_t data_type, uint8_t * bytes, uint16_t length) { void send_bytes_sysex(uint8_t message_type, uint8_t data_type, uint8_t * bytes, uint16_t length) {
// SEND_STRING("\nTX: "); // SEND_STRING("\nTX: ");
@ -6,24 +8,50 @@ void send_bytes_sysex(uint8_t message_type, uint8_t data_type, uint8_t * bytes,
// send_byte(bytes[i]); // send_byte(bytes[i]);
// SEND_STRING(" "); // SEND_STRING(" ");
// } // }
uint8_t * precode = malloc(sizeof(uint8_t) * (length + 2)); if (length > API_SYSEX_MAX_SIZE) {
precode[0] = message_type; xprintf("Sysex msg too big %d %d %d", message_type, data_type, length);
precode[1] = data_type; return;
memcpy(precode + 2, bytes, length); }
uint8_t * encoded = malloc(sizeof(uint8_t) * (sysex_encoded_length(length + 2)));
uint16_t encoded_length = sysex_encode(encoded, precode, length + 2);
uint8_t * array = malloc(sizeof(uint8_t) * (encoded_length + 5)); // The buffer size required is calculated as the following
array[0] = 0xF0; // API_SYSEX_MAX_SIZE is the maximum length
array[1] = 0x00; // In addition to that we have a two byte message header consisting of the message_type and data_type
array[2] = 0x00; // This has to be encoded with an additional overhead of one byte for every starting 7 bytes
array[3] = 0x00; // We just add one extra byte in case it's not divisible by 7
array[encoded_length + 4] = 0xF7; // Then we have an unencoded header consisting of 4 bytes
memcpy(array + 4, encoded, encoded_length); // Plus a one byte terminator
midi_send_array(&midi_device, encoded_length + 5, array); const unsigned message_header = 2;
const unsigned unencoded_message = API_SYSEX_MAX_SIZE + message_header;
const unsigned encoding_overhead = unencoded_message / 7 + 1;
const unsigned encoded_size = unencoded_message + encoding_overhead;
const unsigned unencoded_header = 4;
const unsigned terminator = 1;
const unsigned buffer_size = encoded_size + unencoded_header + terminator;
uint8_t buffer[encoded_size + unencoded_header + terminator];
// The unencoded header
buffer[0] = 0xF0;
buffer[1] = 0x00;
buffer[2] = 0x00;
buffer[3] = 0x00;
// We copy the message to the end of the array, this way we can do an inplace encoding, using the same
// buffer for both input and output
const unsigned message_size = length + message_header;
uint8_t* unencoded_start = buffer + buffer_size - message_size;
uint8_t* ptr = unencoded_start;
*(ptr++) = message_type;
*(ptr++) = data_type;
memcpy(ptr, bytes, length);
unsigned encoded_length = sysex_encode(buffer + unencoded_header, unencoded_start, message_size);
unsigned final_size = unencoded_header + encoded_length + terminator;
buffer[final_size - 1] = 0xF7;
midi_send_array(&midi_device, final_size, buffer);
// SEND_STRING("\nTD: "); // SEND_STRING("\nTD: ");
// for (uint8_t i = 0; i < encoded_length + 5; i++) { // for (uint8_t i = 0; i < encoded_length + 5; i++) {
// send_byte(array[i]); // send_byte(buffer[i]);
// SEND_STRING(" "); // SEND_STRING(" ");
// } // }
} }

View File

@ -80,4 +80,6 @@
# endif # endif
#endif #endif
#define API_SYSEX_MAX_SIZE 32
#endif #endif

View File

@ -1252,28 +1252,40 @@ void cc_callback(MidiDevice * device,
// midi_send_cc(device, (chan + 1) % 16, num, val); // midi_send_cc(device, (chan + 1) % 16, num, val);
} }
#ifdef API_SYSEX_ENABLE
uint8_t midi_buffer[MIDI_SYSEX_BUFFER] = {0}; uint8_t midi_buffer[MIDI_SYSEX_BUFFER] = {0};
#endif
void sysex_callback(MidiDevice * device, uint16_t start, uint8_t length, uint8_t * data) { void sysex_callback(MidiDevice * device, uint16_t start, uint8_t length, uint8_t * data) {
#ifdef API_SYSEX_ENABLE #ifdef API_SYSEX_ENABLE
// SEND_STRING("\n"); // SEND_STRING("\n");
// send_word(start); // send_word(start);
// SEND_STRING(": "); // SEND_STRING(": ");
// Don't store the header
int16_t pos = start - 4;
for (uint8_t place = 0; place < length; place++) { for (uint8_t place = 0; place < length; place++) {
// send_byte(*data); // send_byte(*data);
midi_buffer[start + place] = *data; if (pos >= 0) {
if (*data == 0xF7) { if (*data == 0xF7) {
// SEND_STRING("\nRD: "); // SEND_STRING("\nRD: ");
// for (uint8_t i = 0; i < start + place + 1; i++){ // for (uint8_t i = 0; i < start + place + 1; i++){
// send_byte(midi_buffer[i]); // send_byte(midi_buffer[i]);
// SEND_STRING(" "); // SEND_STRING(" ");
// } // }
uint8_t * decoded = malloc(sizeof(uint8_t) * (sysex_decoded_length(start + place - 4))); const unsigned decoded_length = sysex_decoded_length(pos);
uint16_t decode_length = sysex_decode(decoded, midi_buffer + 4, start + place - 4); uint8_t decoded[API_SYSEX_MAX_SIZE];
process_api(decode_length, decoded); sysex_decode(decoded, midi_buffer, pos);
process_api(decoded_length, decoded);
return;
}
else if (pos >= MIDI_SYSEX_BUFFER) {
return;
}
midi_buffer[pos] = *data;
} }
// SEND_STRING(" "); // SEND_STRING(" ");
data++; data++;
pos++;
} }
#endif #endif
} }

View File

@ -70,7 +70,6 @@ typedef struct {
#ifdef MIDI_ENABLE #ifdef MIDI_ENABLE
void MIDI_Task(void); void MIDI_Task(void);
MidiDevice midi_device; MidiDevice midi_device;
#define MIDI_SYSEX_BUFFER 32
#endif #endif
#ifdef API_ENABLE #ifdef API_ENABLE
@ -79,6 +78,9 @@ typedef struct {
#ifdef API_SYSEX_ENABLE #ifdef API_SYSEX_ENABLE
#include "api_sysex.h" #include "api_sysex.h"
// Allocate space for encoding overhead.
//The header and terminator are not stored to save a few bytes of precious ram
#define MIDI_SYSEX_BUFFER (API_SYSEX_MAX_SIZE + API_SYSEX_MAX_SIZE / 7 + (API_SYSEX_MAX_SIZE % 7 ? 1 : 0))
#endif #endif
// #if LUFA_VERSION_INTEGER < 0x120730 // #if LUFA_VERSION_INTEGER < 0x120730