Re: [fluid-dev] FluidSynth backend for SDL_mixer

[David Henningsson]

On 2011-01-25 05:59, Matt Giuca wrote:
> > Matt, how are things on your end? Were you waiting for some feedback?
>
> Hi James,
>
> Yes, I wasn't sure how to proceed, because I basically finished the
> patch. There was a discussion about the memory allocation strategy
> (since the memory needs to be allocated by the user) which was never
> decided on, so pending that decision (discussion in the other thread),
> the code might need to be changed, but other than that, I just needed
> someone to look over it.
>
> Also as I said in my last email on this thread, if SDL_mixer was going
> to use my new fluid_player_add_mem function, then it would have to
> depend upon some future release of FluidSynth. Even if that is soon,
> maybe the SDL guys don't want a dependency on such a recent version.
> Having said that, I understand that the "half the code" you are
> looking forward to cutting out of your mixer patch was doing its own
> MIDI file parsing and passing events to FluidSynth, so it's probably a
> very good thing to not duplicate all that code (and hence maybe it's
> well worth pushing my patch to FluidSynth ASAP then pushing your patch
> to SDL_mixer).
>
> So ... David, would you be able to take a look at my patch sometime
> soon, and we can wrap up our discussion of memory allocation on the
> other thread? This email summarises the patch:
> http://lists.nongnu.org/archive/html/fluid-dev/2010-12/msg00011.html

Oh, sorry, I didn't understand that you were awaiting feedback from me. 
Well, for the memory allocation issue it sounds like we both are leaning 
towards #5 as in copying the memory. I don't think the inefficiency is 
an issue and it gives FS the most options to change later.

As for the rest of the patch - well, haven't gone through it letter by 
letter but it looks good to me. Assuming that it works, I think you've 
done a good job with it!

A question about the fluid_midi_file struct:

typedef struct {
  const char* buffer;  /* Entire contents of MIDI file (borrowed) */
  const char* buf_end;    /* One-past-end of contents buffer */
  const char* buf_ptr;  /* Current read position in contents buffer */
  int eof;                      /* The "end of file" condition */
  int running_status;
  int c;
  int type;
  int dtime;
} fluid_midi_file;

First, is the "eof" variable necessary or is that the same as buf_ptr >= 
buf_end ?

Second, I would probably have done "int buf_length" and "int buf_pos" 
(as indices to the buffer array) instead of buf_end and buf_ptr, but I 
guess that's mostly a matter of taste. Nothing to reject a patch for.

> Also congratulations on the daughter.

Yeah, hopefully you'll get some sleep. If you're like me and sometimes 
having troubles falling asleep again after having woken up in the middle 
of the night, maybe going coding for a while can help to clear your mind 
a little :-)

// David