Re: [fluid-dev] FluidSynth backend for SDL_mixer

[David Henningsson]

On 2010-10-07 01:23, James Le Cuirot wrote:
> Hi David,
>
> > Cool, thanks for the work!
>
> You're very welcome.

I guess this makes it easier for commercial game developers to use 
FluidSynth? I mean - they can design their own soundfonts and midi 
files, and use right away. And both SDL and FS is under LGPL, so that 
shouldn't pose a problem?

> > 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.

Good point. Perhaps we should add something that takes an in-memory 
image as well and adds that to the playlist? Midi files aren't usually 
that large, so I don't think it would be a problem that way.

> > 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.

Good, one problem less then :-)

> > 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, I see the need for changing the tempo at a pre-defined point in time.

> > 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.

Then I'm out of ideas at the moment. Note that in 1.1.2 the sample rate 
can be changed on-the-fly, whereas in 1.1.1 it must be set before 
creating the synth.

> > 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.

From what I know, there isn't support in either the soundfont 
specification, or Midi file specification, for something else than 
stereo. And there is no LFE mixing stuff either, so it makes little 
sense for FS to output surround. It would be rather cool though, and the 
new 1.1.2 architecture is somewhat prepared for that usage scenario.

> > (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.

Can you add a ticket for adding tempo changes to the sequencer? 
Currently I can't promise when it will be done, as I'm quite busy. But 
you're welcome to write a patch to FS trunk yourself, or perhaps one of 
the other people around here would like to do so? Then the person who 
has time to fix it can assign themselves to the ticket.

// David