Library for playing MIDI files from the Arduino

Update - See last posting.

I have been working on a class to read MIDI files (Standard MIDI Format) from an SD card and play them through a MIDI serial interface on the Arduino. My aim is to be able to play any MIDI file as long as it is properly constructed.

This code is now very stable but at a point where (a) I have been looking at it too long, and (b) I need input from people who know more about MIDI than I do.

The class generally works ok, but there are a few files that do not play well (or at all) and I am stumped as to what makes these files 'special'. All the MIDI files are playable on my Windows PC and I can't see why is in the data that makes them not play when I print out what is in the file (using the dump() method in the class).

All the library code, some example program files that call the library, and a collection of MIDI files are attached. There is a dependency on other libraries - MIDI, SdFat and Flash. Locations to get these is documented in MIDIFile.h. You should also have a working MIDI serial interface - OUT is the only channel used form the Arduino for this software.

The problems I am experiencing are probably best explained in the MIDIFile_Play example - some of the files play really well and other sound strange or don't even start. My feeling is that I am not getting some of the time signals correct as the Time Signature META event is only partially processed, but I can't see how what I have ignore affects anything (ignorance here, probabaly, as I am not a musician).

Happy to get feedback and or help on sorting these issues out from anyone with some free time and interest to test or review the code.

Thanks in advance.

but there are a few files that do not play well (or at all) and I am stumped as to what makes these files 'special'.

Make a midi2text sketch and you might see unsupported codes...

I have no midi board (yet) but will have a look at the code.

starnges that an destructor calls initialize

MFTrack::~MFTrack()
{
	close();
}

void MFTrack::close(void)
{
	initialise();
}

bool MFTrack::getNextEvent(MIDIFile *mf)
is not handling all values between 0xf1 ... 0xff:

midi2txt - saw you have a dump mode (but it is a cooked dump not a raw dump)

WARNING:

void MIDIFile::setMicrosecondPerQuarterNote(uint32_t m)
// This is the value given in the META message setting tempo
{
	uint32_t	x;

	_microsecondDelta = m / _ticksPerQuarterNote;

	// work out the tempo from the delta by reversing the calcs in calcMicrosecondsDelta
	// m is alreay per quarter note
	_tempo = (60 * 1000000L) / m;
}

x not used

BUG:

bool MIDIFile::isEOF(void)
{
	bool eof = true;

	for (int i=0; i<_trackCount; i++)
		eof &= _track[i].getEndOfTrack();	// all must be true for eof to be true

	return(eof);
}

is syntactically wrong as you should use the boolean && not the bitwise & (it just works because all trues seem to be 1)

and it can be faster

bool MIDIFile::isEOF(void)
{
	for (int i=0; i<_trackCount; i++)
	{
		if (! _track[i].getEndOfTrack() ) return false;  // breaks at first false
	}
	return true;
}

Generic:
many loops use int i; you could check if uint8_t is enough ?

STYLE:

#if EVENT_TRACK_PRIORITY
	// process all viable events from each track first - TRACK PRIORITY
#if DUMP_DATA
	Serial.print(F("\n-- TRK ")); 
#endif // DUMP_DATA
	for (uint8_t i=0; i<_trackCount; i++)
	{
#if DUMP_DATA
		int n = 0;

		Serial.print(i, HEX);
#endif // DUMP_DATA
		while (_track[i].getNextEvent(this))
		{
#if DUMP_DATA
			n++; // do all events from this track
#endif
		}
#if DUMP_DATA
		if (n>0)
			Serial.print(F("\n-- TRK ")); 
#endif
	}
#else
	bool doneEvents;

#if DUMP_DATA
		Serial.print(F("\n-- TRK ")); 
#endif // DUMP_DATA
	do
	{
		doneEvents = false;

		for (int i = 0; i<_trackCount; i++)
		{
			bool	b;
#if DUMP_DATA
			Serial.print(i, HEX);
#endif // DUMP_DATA
			b = _track[i].getNextEvent(this);
#if DUMP_DATA
			if (b)
				Serial.print(F("\n-- TRK ")); 
#endif // DUMP_DATA
			doneEvents |= b;
		}
	} while (doneEvents);
#endif // else EVENT_TRACK_PRIORITY

	// save for next round of checks
	_lastEventCheckTime = micros();

	return;
}

So many #if 's makes me wonder if you could not better do something like

#if DUMP_DATA
#define dump.print(x) Serial.print(x)
#define dump.println(x) Serial.print(x)
#else
#define dump.print(x)
#define dump.println(x)
#endif

in the header and just call dump.print() everywhere => would give less conditionals in the code

Thanks robtillart for taking the time to look and comment.

starnges that an destructor calls initialize

Initially I had some memory allocation going on and I had to free memory (that was being done in close()), so the destructor needed to call close(). I have renamed initialise() to resetTrack() which may give us all less of an uncomfortable feeling.

saw you have a dump mode (but it is a cooked dump not a raw dump)

There is a sketch that does a raw dump of the data in the chunks after the header, without using the library. I also use a utility called "MIDI File Analyzer" (picked up somewhere off a Google link) that basically agrees with my own dump(s) of the files, so I am have a high confidence the dumps are correct.

not handling all values between 0xf1 ... 0xff:

Yes, correct. These values are real time valuies that appear in a MIDI live stream as control messages but are not valid in a file - 0xf1 is undefined, 0xf2 is Start a sequence, 0xf3 is Continue sequence. 0xf4 is Stop sequence, 0xf5 is undefined, 0xf6 is Active Sense (like a watchdog timer), 0xff is Reset System. All others in this range should be processed.

x not used

A leftover from when I had an intermediate result being calculated.

syntactically wrong as you should use the boolean && not the bitwise &
and it can be faster

Agree. Suggested version also saves 1 byte of stack space

many loops use int i; you could check if uint8_t is enough ?

Good point, will check.

So many #if 's makes me wonder if you could not better

Great point here. I have done what you suggested before and I don't know why I didn't do it here as well. This will also make the code more maintanable, as an alternative output stream is easier to implement if needed.

I'll post an updated version of the library once implemented and tested.

not handling all values between 0xf1 ... 0xff:
Yes, correct. These values are real time valuies that appear in a MIDI live stream as control messages but are not valid in a file - 0xf1 is undefined, 0xf2 is Start a sequence, 0xf3 is Continue sequence. 0xf4 is Stop sequence, 0xf5 is undefined, 0xf6 is Active Sense (like a watchdog timer), 0xff is Reset System. All others in this range should be processed.

but that still means that when you encounter them in a stream you should handle them appropriately (skip/resync stream etc).

I take your point, but they are actually not allowed to exist in the stream according to the Standard MIDI File documentation. If they are in there then either the file should be considered invalid (ie, abandon it) or the sync has to be to the next byte with a top bit not set, which should be the next deltaT.

The problem with MIDI files is that they are sequential - each note or event is defined in a time offset (deltaT) from the previous one and if you sync forward you get out of time sequence with respect to the other tracks. I actually had this happening early in the piece and it sounds awful.

I'll need to think about this one.

Edit - on reflection, the most straightforward way is to make the track invalid (ie, mark it as end of track) and stop playing it. This allows the other tracks to play on and the file is not totally lost.

Updated version of the library is attached. This implements points discussed above plus some additional tidy up of the debug messages.

I take your point, but they are actually not allowed to exist in the stream according to the Standard MIDI File documentation.

There are many things in the world not allowed that still happen ...

if you encounter forbidden codes you should close the file immediately. In the big switch this should be the default: option to catch all non supported

This version (0.4) now works with everything I have been able to throw at it.

I have reworked the timing section so that it is tighter and more accurate in keeping the music to time. There were also serious logical errors in the processing of MIDI run on messages that caused the some files to play badly or not at all (see my first post).

I think there is a bit of tidy up to do and then it can be released as v1.

Good to hear it is working now!
I wait until after the cleanup is done before reviewing again

This work is now completed and can be found at Google Code Archive - Long-term storage for Google Code Project Hosting.

Thanks to robtillaart for his great comments and feedback during development.

The library is used to process MIDI files from the SD card. MIDI and SYSEX events are passed to the calling program through callback functions for processing. This allows programmers the flexibility to implement a message to a MIDI synthesiser through serial interface or other output devices,
like a MIDI shield or custom hardware.

Processing of files may may be paused and restarted using methods in the library.

Download the library and see MIDIFile.h for more information if you are interested.

Hi,
minor issue, the midihelper.h has no construct like

#ifndef _MIDIFILE_H
#define _MIDIFILE_H

in midifile.cpp ~line 270

	for (n = 0; (n < 100) && (!doneEvents); n++)  //<<<<<<<<<<<<<<<<<<<<
	{
		doneEvents = false;

		for (uint8_t i = 0; i < _trackCount; i++)	// cycle through all
		{
			bool	b;

			if (_format != 0) DUMPX(i);
			// Other than the first event, the other have an elapsed time of 0 (ie occur simultaneously)
			b = _track[i].getNextEvent(this, (n==0 ? elapsedTime : 0));
			if (b && (_format != 0))
				DUMPS("\n-- TRK "); 
			doneEvents |= b;
		}
		n++;  //<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< extra n++ in the loop? why ? 

		// When there are no more events, just break out
		if (!doneEvents)
			break;
	}

in int MIDIFile::load()
if there is an error should you not close the file ?

• some var names are still cryptic e.g. c (which is a string containing part of header)

shorter?

uint32_t readVarLen(SdFile *f)
// read a variable length parameter from the input stream
{
  uint32_t  value = 0;
  char      c;
  
  do
  {
    c = f->read();
    value = (value << 7) + (c & 0x7f);
  }  while (c & 0x80);
  
  return(value);
}

buffer with "position info"

void dumpBuffer(uint8_t *p, int len)
{
  for (int i=0; i<len; i++, p++)
  {
    if (i% 8 ==0)
	{
	   DUMP('\n');    // << #DEFINE DUMPLN(T) Serial.println(T) ??
	   DUMP(i);
	   DUMP('\t');
	}
    DUMP(' ');
    if (*p<=0xf) 
		DUMP('0');
    DUMPX(*p);
  }
}

finally there is a style diff between the #ifdef used for void MFTrack::dump(void) and void MIDIFile::dump(void)

(end of nitpicking

Thanks again for the review (nitpick?)

midihelper.h has no construct like

OK. It has now, but it is only ever included in one file which is protected from re-inclusion.

n++; //<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< extra n++ in the loop? why ?

Left over from when it was a while loop. Should not affect the functionality though, as the for loop is just a way to limit too many events. Effectively halves the (arbitrary) limit.

if there is an error should you not close the file ?

Yes absolutely. Interesting is that SDFat did not fail over repeated testing with a file that was not correct. It must handle the case of the open file (or maybe it just doesn't care?).

some var names are still cryptic e.g. c

This is picky. The char array is local to a block of 5 lines of code! To keep you happy I have renamed it 'h'

shorter?

Clearly so. I'll do some testing as the original code was proposed as the right code to process the varlen parameters by the MIDI organisation.

#DEFINE DUMPLN(T) Serial.println(T) ??

The philosophy adopted in the DUMP output is that the \n is at the start of the new line, not at the end of the previous one. This makes it cleaner as I can't always tell where the end may be but am sure when I start a new line. So I need to explicitely use '\n' when I want it. It also makes it easier if I want to add debug as I don't need to worry about the line endings sprinkled through the code.

style diff between the #ifdef used for void MFTrack::dump(void) and void MIDIFile::dump(void)

I assume you mean in the header file for the class definition?
I wanted to keep the dump function prototype available at all times so that I don't need #if in the sketch code around a call to the dump() method, even if it is an empty function. However, in the classes, the #if switch will cut out all the code for the MFTRack class, as this is only called by MIDIFile.dump().
I suppose an alternative is an empty inline definition in the class dependent on the #if, but the result isthe same.

Welcome!

(nitpick?)

some one who keeps nagging about details - I don't know for sure if nitpicking is the right term here (English is not my mother tongue)

Think there will be a point release soon

I have renamed it 'h'

much better

I do appreciate your time in reviewing. When I used to write software for a living it was amazing how much better code could be when a fresh set of eyes were applied and constructive comments passed.

Yes, there will be a pint release once I can check a few things.

Yes, there will be a pint release once I can check a few things.

I like that

Thanks for your work on this -- I can see a lot of applications and think it will be helpful for a lot of people.

I have a related newbie question: For those of us without an SD-enabled Ethernet shield, what adjustments should be made? I have an adafruit waveshield and will be getting an SD breakout shield, but I'm not sure what modifications in SD reading pins to make. For instance, your MIDIFile_Play sample uses "#define SD_SELECT 4"; it might be helpful to explain what an "SD chip select pin" is and what other common values might be for other arduino versions or shields with SD readers.

Also, I imagine it would be possible to store very small MIDI files in the arduino's memory itself. That would open up some options for people who want to play back simple music or other actions (lights, servos, etc.) with an open, easy-to-modify standard like MIDI, but without needing to include an SD card in their setup. Would that be hard to do? What would be the best way to approach that?

Thanks again. Looking forward to learning enough to use this.

What would be the best way to approach that?

Start building it.