fluid-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [fluid-dev] Propose changes for drum channels support


From: Matt Giuca
Subject: Re: [fluid-dev] Propose changes for drum channels support
Date: Thu, 27 Jan 2011 11:27:10 +1100


I'm not a FS dev either.  Just seeing something I may want to use isn't there, so I just take a crack at it.  Everyone on this [fluid-dev] list is one way or another interested in the coding/development side of FS one way or another.  That's why I ask for opinions instead of contacting just one of the FS developers and try to push through some new features.

I didn't mean to suggest you shouldn't be using fluid-dev. I was just disclaiming that when you read my reply, you shouldn't consider it to be representative of the opinion of the FS devs (for example, just because I like it does not mean it will be accepted). However, David is (one of?) the lead developers, so he has a good idea of what it will take to get a patch included, and he seems to like it too.

> A few points: Firstly, in the future could you attach your
> patches as attachments instead of pasting them at the end.
> It makes them much easier to read and apply.
>

There are some pro's and con's of this.  Attachments are cleaner so folks don't have to cut and paste them.  However, most mailing lists will not archive attachments, just the messages.  From time to time I have seen archived messages (sometimes from mirrored/relayed lists) referring to attachments that's just not there.  I don't think Yahoo lists would archive attachments either.  Such have been pointers to nowhere for me, and I don't like that.  Perhaps, I could attach and include them (2 copies in the message).

Ah, good point. Well FluidSynth mail archive seems to include attachments; for example:
http://lists.nongnu.org/archive/html/fluid-dev/2010-10/msg00007.html

Sometimes in printing debugging messages, enum/boolean make it extra work to print out info.

It shouldn't; if you use printf("%d") on a bool or enum, it will show up as an int (0 or 1) anyway.

For musical use, I don't know.  For MIDI lightings, stage control, video mixing, animation...  who knows, perhaps those things may become more common.

Yeah, but as FluidSynth is a synthesiser, maybe that isn't necessary. Anyway, David thought it was good to keep it general (enum).

That's right, the patch doesn't break any backward compatibility, I did put such consideration in the comment so later on, folks can go back and see why.

Yes, but that's the sort of thing that should be explained in a commit log, so if something did break, you could find the revision where it broke and read the rationale in the commit log. The code can't be littered with a discussion of the history of the project.

> I don't think this FIXME has been fully resolved. I
> don't quite understand it, but it says that the proper
> way to handle it would be to ignore bank changes, which
> isn't what your patch is doing. Can you explain what
> this comment means and why it's OK to remove it?

From what I understand, and strictly speaking, GM mode doesn't allow bank changes at all, 128 instruments and some drum kits, that's it.

Even later comments on GM specs, it's up in the air whether hardware and software synths should disallow bank changes at all.  It was noted that some implementations automatically switch out of GM-mode when a bank change message is received.

So isn't there still the potential need for FluidSynth to ignore bank changes when in GM mode or go out of GM mode?

So the first half of the comment is handled by this patch, the later part of that comment is a wash.

You mean "Shouldn't hard code bank selection for channel 10." How does your patch handle it? Does it hard code bank selection for all drum channels now? Or does it not hard-code it at all?

Matt

reply via email to

[Prev in Thread] Current Thread [Next in Thread]