Re: [fluid-dev] FluidSynth backend for SDL_mixer

[James Le Cuirot]

Hi David,

> Cool, thanks for the work!

You're very welcome.

> I assume you had a look at the midi file player and decided it didn't 
> work for you?

I did. The main problem was that it only takes a file path, not a file
handle. I looked at whether it would be easy to add a function for this
but it didn't seem to fit into the playlist design.

> What version of FluidSynth are you testing with?

I was using 1.1.1. I've just upgraded to 1.1.2. I'm on Gentoo and 1.1.2
isn't available there yet but when I'm done with this, I'll update the
package to use CMake.

> Hmm, because they are private, that means that we are free to change
> the implementation (and even remove the field) between FS versions.
> I'm not sure I want to give up that freedom right away, what does the
> rest of the list think here?
> 
> Also, I'm not exactly seeing why wrapping this in another struct
> would be "a PITA", but you can also malloc another array next to the
> event array containing the times if that works better for you. That
> would also stop the overwriting problem you have.

I'm being very conservative with my memory management and backing out
of a failed malloc could get messy with nested structs. Having a
separate array works fine though. Should have thought of that! I've now
made that change.

> I think you'll have to look at fluid_player_set_midi_tempo and do 
> something similar, as you're doing your own midi file parser, that
> is, combine PPQN with the MIDI_SET_TEMPO meta event.

I read up some more on SYSEX and found out what these meta events are.
I now understand that the PPQN alone isn't enough even for the initial
tempo so I need to handle this event. The problem is, as I mentioned
before, there isn't a fluid_event function for tempo changes. I tried
to work around this by dealing with it in a client callback but when I
call fluid_sequencer_set_time_scale inside the callback, the game
crashes with this horrific error.

GThread-ERROR **: file gthread-posix.c: line 385
(g_thread_join_posix_impl): error 'Resource deadlock avoided' during
'pthread_join (*(pthread_t*)thread, &ignore)'
aborting..

Nasty stuff. I've checked and it definitely happens during that
function call. If I have to call this function myself, the callback is
really the only place I could do it. This is probably something you'll
have to fix on your end.

> Okay, no problem. I have changed it to 8 kHz. I'm feeling sorry for
> the Rise of the Triad users though, as they will lose all the
> treble ;-)

Thanks! I suppose the game doesn't have to mix at that rate but at
least we won't break existing games now.

> Sample rate mismatch between 44100 Hz and 48000 Hz, perhaps?

That's what I was thinking because it does sound like that kind of
difference. I've checked though and 44100Hz is definitely being
requested by the game, it is definitely what SDL_mixer is asking
FluidSynth to use, and it is what PulseAudio says the game is using.

> FluidSynth's native format is single- or double-precision floating 
> point, depending on configuration. There is also a convenience
> function for converting to 16-bit signed.
> 
> I'm quite certain that there are SDL functions for converting to the 
> other sample formats you need...?

You're right. I must admit I hadn't looked because I saw the functions
being used by the Timidity backend and figured they wouldn't have
written those if SDL could already do the job. I think they were needed
because Timidity's native format is 32-bit integer and support for that
was only introduced in 1.3. Support for float formats was also only
introduced in 1.3 so I'll stick to the s16 convenience function.

> For the multi-channel stuff, that is not 5.1 - at least not today. 
> That's mostly meant for professional audio (think studio), when you
> want separate output for separate MIDI channels.

So I don't want to request more than one stereo pair from FluidSynth?
I guess I'd need to copy that output several times to fill all the
requested channels. This is probably what Timidity does so I'll check
that out.

> (Btw, one thing I spotted: you probably want to free the settings 
> instance with delete_fluid_settings instead of just calling "free".)

Well spotted and now fixed!

Overall it's looking in better shape already. I just hope that tempo
error won't pose too much of a problem.

Cheers,
James