[Denemo-devel] The recursive jack_output_midi()

[Richard Shann]

Jeremiah,

The code you checked in to cope with the buffer not being ready looks
like it could cause trouble:
void jack_output_midi_event(unsigned char *buffer){
 if (BufferEmpty==TRUE){ 
   /*needs replacing with something like this:*/
   //memcpy(&global_midi_buffer.buffer, buffer, sizeof(buffer));
   global_midi_buffer.buffer[0] = buffer[0];
   global_midi_buffer.buffer[1] = buffer[1];
   global_midi_buffer.buffer[2] = buffer[2];
   BufferEmpty=FALSE;
 } 
 else 
  jack_output_midi_event(buffer); 
}

you are recursively calling the function until the buffer has been
consumed. I would expect some systems to crash with the stack getting
exhausted because of getting the function call parameter pushed on to it
repeatedly. (There is something called tail recursion something or other
which the optimizer could do to save you, but we shouldn't count on it).
If you really want the function to hang until the buffer is ready then a
while(BufferEmpty==TRUE) sleep(for a while) would do this more safely.

However, I think it is perhaps a bad way to go since if for some reason
the jack stops releasing the buffer you will hang Denemo. Instead create
a static array of a few thousand of these global_midi_buffer things and
define

static volatile gint BufferIndex;

static gint BufferFillIndex;

Where BufferFillIndex is the next place to put a midi message and the
BufferIndex is the place jack will take from. The above code will look
like this:

 if (BufferEmpty==TRUE){ 
   /*needs replacing with something like this:*/
   //memcpy(&global_midi_buffer.buffer, buffer, sizeof(buffer));
   global_midi_buffer[BufferFillIndex].buffer[0] = buffer[0];
   global_midi_buffer[BufferFillIndex].buffer[1] = buffer[1];
   global_midi_buffer[BufferFillIndex].buffer[2] = buffer[2];
BufferFillIndex++;
        check if BufferFillIndex > maxindex if so set BufferFillIndex = 0
        check if BufferFillIndex == BufferIndex if so you have a buffer overrun
and you should stop/warn or WHY.
    BufferEmpty=FALSE;//always do this last
}


The BufferIndex is incremented in the interrupt, after taking the data
from global_midi_buffer[BufferIndex] and again you need to wrap round at
maxindex - you don't need to take any other action, just set
BufferIndex=0.

A few thousand is plenty and will be much easier to program than some
dynamic allocation thing because of the difficulties of communicating
with the interrupt routine. It could get upgraded if someone came up
with some application needing hundreds of messages buffered.

Richard