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

[Matt Giuca]

> We could also copy the memory. While copying memory around is inoptimal, that might be a secondary concern. At least that would give us the most future flexibility.

Oh yeah, that was my other thought. I think the problem with that is that again, in almost all cases I can think of, you'll allocate the memory just to load and then pass to FluidSynth, which would then in turn allocate the memory again just to copy it and then you'll immediately free the original memory.

I don't see how it gives us future flexibility any more than the other approaches -- either way once the decision is made, clients will be locked in to one particular garbage collection strategy or another (in this case, clients will be freeing the memory as soon as it's passed to FluidSynth, which makes their code incompatible with the stealing or borrowing approaches).

I was about to suggest (and I see you suggested below as well) a fluid_alloc function which lets you allocate the memory in a fluid-approved way, which could then automatically be freed. This would be equivalent to the "copying" approach (in that Fluid owns the pointer), except that the client has control of the memory initialisation. If you had a buffer which is not fluid_alloc-allocated, you could do the copy yourself (which would be no more expensive than if fluid_player_add_mem did a copy).

So to summarize, the possible approaches are:
1. Stealing from client-malloc; fluid will call free(). Won't work with different allocators other than malloc.
2. Stealing from client-fluid_alloc; fluid will call fluid_free(). At least it lets fluid control the allocator.
3. Borrowing; fluid will not free memory. Requires complicated memory management in the client.
4. Borrowing, with a "destructor" callback. Lets the client control allocation and freeing, and can easily simulate #1 or #2.
5. Copying. Client must free, but can do it right away. Easiest method, but inefficient.

I think I prefer #2, because it gives maximum power to the client, is efficient and doesn't require that they do memory management. #4 is even more powerful, but a bit complicated an API call.

If so, we should probably also add fluid_malloc / fluid_realloc / fluid_free as API additions to make sure the same memory allocator is used. After all, in a future version of FS, we might want to switch memory allocator, write our own for some reason, etc.

Yep. OK so I'd probably advocate writing them. Maybe even a void* fluid_copy_alloc(void* buffer, size_t length), which is short-hand for fluid_malloc followed memcpy.

One hard part of FluidSynth is that it has so many use cases, and we can't fail a single one of them. What we can't do good enough today (IMHO), but what I would like to do, is good looping, which would be important for games, which would like to loop a background song seamlessly. It could also be that somebody wants to play along with a drum track (and the drum track is in the MIDI file), and so he adds the midi file 100 times. (Also player.reset-synth must be set to "no")

So to give you an overview - with the 1.1.2 architecture, while it improves some use cases, it doesn't do much to help out midi players.
The 1.1.2 architecture splits FS into two parts - one hard real-time safe part, which contains the rendering. So when you (or the audio drivers) call fluid_synth_write_s16/float, it should be guaranteed a quick return. This is to avoid underruns and the clicks that comes from it. This part is now in the rvoice directory.

To complicate matters, there are some MIDI synth events which we cannot do in a hard real-time safe way. So the MIDI engine - that transforms MIDI events into individual voices - is not real-time safe, and calls into that API are synchronized. Once the MIDI engine has completed the processing of a MIDI event, it stores commands to the rvoice engine in a queue.

Also important in this context is the system timer vs the sample timer. If we use the system timer, the problem with real-time safety is solved, because we have an additional thread checking the computer's system timer, calling into the API at predefined times. This however leads to another problem instead, which we called the "drunk drummer" problem a year or two ago. That is, we can't intercept a fluid_synth_write_s16 call in the middle to insert a MIDI event, so all MIDI events become quantizised to the buffer size, causing bad timing.

So therefore the sample timer was invented, which enables callbacks from the middle of the rendering, which could in turn can call the MIDI player or sequencer to insert MIDI events appropriately. This is good for timing and reliability, and a must for faster-than-realtime-rendering, but bad for real-time/underrun safety.

So the situation is not optimal either way (and has never been). I've had loose thoughts about how to attack this problem, but not thought them through enough to start discussing an implementation.

So to sum up, I'd like our MIDI player to be real-time safe one day. It's a long way there, so right now I just want to make sure we don't walk in the wrong direction.

OK thanks for explaining all that. So I think the proposed changes (especially if we let fluidsynth manage the memory) will get us closer towards that goal with respect to MIDI file loading. I say closer, because we'll be changing from "when you hit the next song, open the file then parse it reading bytes directly from the FILE* into the parser" to "when you hit the next song, open the file, read it entirely into memory, close the file, then parse the memory." That doesn't change the timing considerations at all, but it does open up the possibility of loading the file into memory ahead of time (at "add time", not when you hit the next song which we'll call "play time"), and potentially perhaps parsing the file into the track data structures at "add time" too.

And that was your other suggestion. We won't go there just yet, but I think the proposed architecture gets us closer to it.

You mean fully parse the MIDI file into track data in
fluid_player_add_mem. I see, well I don't think I'll tackle that just
yet, but I agree it could be an interesting trade-off (somewhat
increased memory consumption, but having no overhead -- disk or memory
-- in switching MIDI files).

This might be a future path, which means that is one more thing you shouldn't make harder or impossible.

Right. Well I think I'll probably make it easier if I make the above changes.Sounds good to me. My job is here to keep you informed of all the thoughts and use cases here, in order for you to have them in mind when you write code. Hope you're up for the challenge :-)

OK thanks David.

On 2010-10-19 00:02, Pedro Lopez-Cabanillas wrote:

If you are motivated to do this work, then go ahead. Nobody has any right to
stop you of adding an improvement that you want to implement, and you believe
that it is valuable.

...as long as it doesn't break anything else, which is the hard part.

I think he meant nobody has the right to stop you implementing it in your own private branch. It's convincing others to accept them that's the hard part :) Which I've learned from other open source projects. That's why it's good to have a discussion up-front to make sure people in the community are comfortable with the idea.

If you're talking about dropping backwards compatibility, then that is a big deal IMO. We might come to a point where we believe it is necessary, but I don't see it coming right now. It would mean all distros shipping with two FS library versions, then someone finds a bug in the old one, and we only support the new one, and so on...

I agree. For this specific change, I plan to keep fluid_player_add exactly the same (possibly changing some of the performance characteristics but not in any serious way) and add just one new function, so I don't think it will be backwards-incompatible.