Re: [fluid-dev] Making MIDI player read from a buffer

[David Henningsson]

On 2010-10-18 16:14, Matt Giuca wrote:
> There was a discussion in the thread "FluidSynth backend for
> SDL_mixer" about the difficulty with the current implementation of the
> FluidSynth API's MIDI file player, in that it will only load a MIDI
> file via a filename (fluid_player_add --
> http://fluidsynth.sourceforge.net/api/midi_8h.html#a1ac5b59e4ab9d0f48e917dda0a9a4403
> -- seems to be the only way to load anything into the MIDI player).
> This means if you need to load a MIDI file from any source that is not
> a regular disk file (e.g., from an already-open FILE* such as stdin, a
> network socket, an SDL RWop, or another high-level "file-like object"
> which is not an ordinary file on disk), you are presently out of luck.
>
> This apparently was so bad that James Le Cuirot, working on SDL_mixer
> backend for FluidSynth, wrote a whole MIDI parser ...
>
> In that thread, I volunteered to help fix FluidSynth around this
> (thanks to James for reminding me this morning). It looks like quite a
> job since the "loading of files via a filename" aspect goes quite deep
> into the internals of FluidSynth. I thought I'd start this thread to
> a) see if there is interest in this change, and b) have a discussion
> about whether I'm going about it right. I think it would be quite
> useful to fix. What I proposed was not the most general solution to
> this problem (the "ideal" solution would be to allow any "file-like
> object" to be plugged into FluidSynth and have it request bytes from
> that), but a good compromise: Allow FluidSynth to read MIDI file data
> from a void* memory buffer. Since MIDI files are small, any other
> "file-like object" could simply load all of the bytes into memory and
> then pass the void* buffer in. Like I said, it's not ideal, but at
> least it allows these solutions to function.

Thanks for the analysis and willingness to help out with FluidSynth!

> I can hopefully do this without breaking the existing FluidSynth API,
> but it would require a lot of changes to the internals. Here we go.

...although this would only be in fluid_midi.c, right? It wouldn't 
affect other files.

> 1. INTERFACE CHANGES
>
> There is simply one new function, declared in
> include/fluidsynth/midi.h and defined in src/midi/fluid_midi.c:
>
> /**
>   * Add a MIDI file to a player queue, from a buffer in memory.
>   * @param player MIDI player instance
>   * @param buffer Pointer to memory containing the bytes of a complete MIDI
>   *   file. The pointer is "stolen" and will be freed by FluidSynth once it is
>   *   no longer needed.

I'd personally prefer the client handling memory entirely. This has two 
advantages:
 * We won't have to do "free" in a context that might be real-time 
sensitive.
 * The client does not have to use the same memory allocator as 
fluidsynth.

>   * @param len Length of the buffer, in bytes.
>   * @return #FLUID_OK
>   */
> FLUIDSYNTH_API int fluid_player_add_mem(fluid_player_t* player, void
> *buffer, size_t len);
>
> The idea is that it is possible to add filenames (using
> fluid_player_add) and memory buffers (using fluid_player_add_mem)
> interleaved in the queue, so each queue item can either be a filename
> entry or a memory buffer entry.

That's consistent with my thoughts.

> 2. INTERNAL CHANGES
>
> Internally, this will be difficult, as the following data structures
> are currently hard-wired to deal with filenames/FILE*s (from
> src/midi/fluid_midi.h):
>
> /*
>   * fluid_player
>   */
> struct _fluid_player_t {
>    ...
>    fluid_list_t* playlist; /* List of file names */
>    fluid_list_t* currentfile; /* points to an item in files, or NULL if
> not playing */
>    ...
> }
>
> /*
>   * fluid_midi_file
>   */
> typedef struct {
>    fluid_file fp;
>    ...
> } fluid_midi_file;
>
> (where fluid_file is a typedef for FILE*).
>
> First, fluid_player_add adds the filename to fluid_player::playlist
> (without loading from disk). This stores the filename as a char* in
> playlist->data.

So playlist->data has to point to a struct instead of a char*.

> Then, when it gets up to that particular playlist
> item, it calls fluid_player_load (fluid_midi.c) with that filename,
> which opens the file into a FILE*. This then temporarily creates a
> fluid_midi_file object, storing the FILE*, and then calls a bunch of
> other files to read the bytes of the MIDI file from disk and assemble
> them into an in-memory data structure inside the fluid_player_t. It
> then deletes the fluid_midi_file object, and from that point onwards,
> everything is in memory (our job is done).
>
> All of the above architecture would have to be modified to handle
> reading from a memory buffer.

In addition, here are things that might or might not be real-time 
sensitive. This conversion sounds like time intensive.

> (Note: Short of using mmap and doing
> file descriptor tricks, which I'm sure would be Linux-specific or
> something, there's not really any way to make use of the FILE*
> infrastructure.) Possibly the easiest way to handle this would be to
> *completely replace* the existing FILE* stuff with a void* instead,
> and replace such functions as fluid_midi_file_getc with new
> implementations which read bytes from the buffer. That would be
> relatively straightforward. Then fluid_player_add_mem would work
> naturally with the new infrastructure. The old fluid_player_add
> function would then wrap around that -- it would load the file from
> disk into a buffer in memory, then pass that buffer to
> fluid_player_add_mem. However, I expect this will not be what people
> want to do, because it fundamentally changes the way fluid_player_add
> works -- it would load the entire file into memory up-front, rather
> than the current approach which reads in a few bytes at a time, never
> loading the whole file into memory. Maybe this is an acceptable
> trade-off though (since come on, MIDI files are easily going to fit
> into memory these days, even on embedded systems, and load very
> quickly also). One important caveat is that it will mean file access
> errors come up during fluid_player_add, rather than when the player
> reaches the particular playlist item.

That is probably a good thing though.

Here's a compromise:

struct loadable_file
{
        char* filename;
        void* data;
        size_t data_length;
}

(You don't need an enum because either one will be NULL.)

But then, at file load time, if filename != NULL, load the file into the 
data and data_length fields, then do midi parsing, followed by freeing 
the data buffer again. Then we at least have only one midi file in 
memory at a time.

The other end of it would be to run through all of the midi conversion 
functions inside fluid_player_add_mem. With all time intensive tasks 
done, we might be able to switch from one midi file to another very 
quickly (without risking underruns). That'd be nice, but I don't know 
about the memory consumption.

What do you think?

// David