Add support for balance control (CC 8) #317

mawe42 · 2018-01-04T00:01:33Z

This pull-request adds support for the balance control on CC 8, as discussed on the mailing list:
http://lists.nongnu.org/archive/html/fluid-dev/2018-01/msg00006.html

It also makes a slight change to the range of the panning control so that voices panned to the center are exactly the same value on both channels. It does this by reducing the range to 1 - 127, treating 0 and 1 as "hard left". This treatment is specified in the MIDI Recommended Practice (RP-036).

This special treatment is also used for the balance control, as it has the same problem with the non-representable center value on a 0 - 127 range.

As specified in MIDI Recommended Practice (RP-036)

derselbst · 2018-01-04T05:33:24Z

src/synth/fluid_synth.c

@@ -138,6 +138,7 @@ static fluid_mod_t default_expr_mod;           /* SF2.01 section 8.4.7  */
 static fluid_mod_t default_reverb_mod;         /* SF2.01 section 8.4.8  */
 static fluid_mod_t default_chorus_mod;         /* SF2.01 section 8.4.9  */
 static fluid_mod_t default_pitch_bend_mod;     /* SF2.01 section 8.4.10 */
+static fluid_mod_t default_balance_mod;        /* Non-standard modulator */


Should be called custom_balance_mod

Yes, good idea. Will change it.

derselbst · 2018-01-04T05:38:07Z

src/synth/fluid_synth.c

+		       );
+  fluid_mod_set_source2(&default_balance_mod, 0, 0);
+  fluid_mod_set_dest(&default_balance_mod, GEN_BALANCE);                  /* Destination: stereo balance */
+  fluid_mod_set_amount(&default_balance_mod, 1000.0);                     /* Amount: 1000 tens of a percent */


Why are you not using the same amount as GEN_PAN does?

Soundfont specifies that generator ranges should be in real-world units. -500;500 makes perfect sense for pan, because it is defined as the position on a 180 degree half circle. So -500 is 50% of 180 left.

Balance is not expressed as position, but as volume. So 100% left makes much more sense to say "right channel is silent" than 50%.

Hm... Thinking about it, maybe balance volume should be expressed in centibels, like volume and expression. That would make the range -960;960, the modulator would get a concave shape and we would use fluid_cb2amp for conversion. Actually, that would also make volume changes much smoother in the extreme regions. What do you think?

I just thought that giving it a pure linear scale wouldnt be "smooth" enough. I thought using a sinus would be still somewhat linear but in a smoother manner. This is where we would end up in fluid_pan() again.

fluid_cb2amp() could work as well. What I dont like about that [centi|deci]bel scale is that it never turns off. Yes, the volume gets very silent, probably unhearable, but if I set CC8=127 I would expect the left chan to be completely off.

But sure when looking at volume and expression, perhaps we should really use fluid_cb2amp() to be somewhat "consistent"?

The sinus in pan is actually there to linearise the perceived volume, using equal power distribution. In listening tests I checked the perceived volume changes on a mono sample with pan and balance. With the current implementation, they are very similar, if not the same.

Using centibels, balance would be much smoother. Oh, and we could simply force -960 and 960 to be 0 (or 1 on the opposite channel)

Alright then fluid_cb2amp().

Oh, and we could simply force -960 and 960 to be 0 (or 1 on the opposite channel)

No special casing pls. 10^(960/-200)=0.000016 and that's how things should be then :)

No special casing pls. 10^(960/-200)=0.000016 and that's how things should be then :)

In that case I would strongly prefer the current solution. A balance control that can't turn one channel off completely would be flawed, imho.

What's so bad about forcing the extreme values to 0?

A balance control that can't turn be channel off completely would be flawed, imho.

Same like a main volume control that cant turn off a channel completely, which currently seems to be the case (at least I experienced that).

What's so bad about forcing the extreme values to 0?

I dont like branches in math calculations. They cause hearable discontinuity and branch prediction failures. They just dont feel right. Where would you place these if clauses? Probably after modulator processing, a place where you would need to do a floating point comparison for x==960.0. How likely is that to evaluate to true?

In that case I would strongly prefer the current solution.

Or we go with fluid_pan(), i.e. sin().

mawe42 · 2018-01-04T10:52:57Z

I dont like branches in math calculations. They cause hearable discontinuity and branch prediction failures. They just dont feel right. Where would you place these if clauses? Probably after modulator processing, a place where you would need to do a floating point comparison for x==960.0. How likely is that to evaluate to true?

Hearable discontinuities are there in any case. Just listen to the steps in volume between pan 127 and pan 126 on a mono sample. And if we put the special casing into the setup of the conversion table (just like it's already been done for fluid_concave_tab and fluid_convex_tab where index 0 is forced to 0.0f and index 127 is forced to 1.0f), then we don't need any new branches. So just add the following after the fluid_cb2amp_tab setup:

fluid_cb2amp_tab[0] = 0.0f;
fluid_cb2amp_tab[FLUID_CB_AMP_SIZE - 1] = 1.0f;

That would have the added benefit that the volume and expression controls can also turn off (and on) completely.

But incidentally... I wonder why FLUID_CB_AMP_SIZE is defined as 961?!

mawe42 · 2018-01-04T11:27:16Z

But incidentally... I wonder why FLUID_CB_AMP_SIZE is defined as 961?!

Never mind... of course it's because the range is that long, from 0 - 960. Doh...

derselbst · 2018-01-04T12:18:02Z

Hearable discontinuities are there in any case. Just listen to the steps in volume between pan 127 and pan 126 on a mono sample.

Well ok, that's a necessary discontinuity due to the limited 7 bit resolution.

fluid_cb2amp_tab[0] = 0.0f;

I think it should be fluid_cb2amp_tab[FLUID_CB_AMP_SIZE-1] = 0.0f. But anyway, even if it's just a minor nit-picky change, it would violate the spec.

That would have the added benefit that the volume and expression controls can also turn off (and on) completely.

Yes I would be very happy with it. Still by the spec apparently volume and expression are not meant to turn channels off completely and I do not want to support such a change just because of my personal attitude.

So we have these three options. From my personal musical perspective/preference I would go with fluid_pan(). OTOH to be consistent with volume calc. I would use fluid_cb2amp(). Difficult. Keeping it linear is not my favourite. But I would leave the decision to you, as you have more musical experience than I.

mawe42 · 2018-01-04T12:26:57Z

Yes I would be very happy with it. Still by the spec apparently volume and expression are not meant to turn channels off completely and I do not want to support such a change just because of my personal attitude.

Actually, I think the current behaviour is against the spec. From the Sound Font 2.04 document:

The volume envelope operates in dB, with the attack peak providing a full scale output, appropriately scaled by the initial volume. The zero value, however, is actually zero gain. The implementation in the EMU8000 provides for 96 dB of amplitude control. When 96 dB of attenuation is reached in the final gain amplifier, an abrupt jump to zero gain (infinite dB of attenuation) occurs. In a 16-bit system, this jump is inaudible.

derselbst · 2018-01-04T12:34:54Z

The zero value, however, is actually zero gain.

Oh, good spot. Well ok then, let's make us both happy with fluid_cb2amp_tab[FLUID_CB_AMP_SIZE-1] = 0.0f and fluid_cb2amp().

mawe42 · 2018-01-04T12:38:35Z

Hm... on the other hand, that is only specified for the final gain amplifier and doesn't express anything about intermediate calculations. Oh dear... :-)

derselbst · 2018-01-04T12:48:36Z

Argh, I intentionally over-read that ^^

Well, we could ask mrbumpy409 (S. Christian Collins), he owns some EMU hardware, perhaps a third opinion might help?

mawe42 · 2018-01-04T13:25:31Z

we could ask mrbumpy409 (S. Christian Collins), he owns some EMU hardware, perhaps a third opinion might help?

That would be great, yes.

However, reading the spec again and again, I come to the conclusion that Fluidsynths current implementation is actually not correct. The output of the final gain amplifier (per sample/voice) is meant to be mixed into the output and sent to the effect units. The amount of attenuation that a voice receives should only be considered at this stage, and here we should also check if the attenuation is 96db or greater and force it to be infinite (so an amp of 0), according to spec. So the best place to check this is IMHO fluid_voice_calculate_gain_amplitude, maybe like this:

static FLUID_INLINE fluid_real_t
fluid_voice_calculate_gain_amplitude(const fluid_voice_t* voice, fluid_real_t gain)
{
    /* we use 24bit samples in fluid_rvoice_dsp. in order to normalize float
     * samples to [0.0;1.0] divide samples by the max. value of an int24 and
     * amplify them with the gain */
    const fluid_real_t INT24_MAX = (1 << (16+8-1)) * 1.0f;
    const fluid_real_t MIN_AMP = pow(10.0, (double) 960.0 / -200.0) / INT24_MAX;
    fluid_real_t amp = gain * voice->synth_gain / INT24_MAX;
    return (amp <= MIN_AMP) ? 0.0f : amp;
}

I've just tested this with s16 and s24 output and it does have the desired effect. And one more float comparison shouldn't affect performance, I think.

mawe42 · 2018-01-04T13:33:20Z

Ah, no. It shouldn't take the final output gain into consideration. So the check should be on gain, not on amp:

static FLUID_INLINE fluid_real_t
fluid_voice_calculate_gain_amplitude(const fluid_voice_t* voice, fluid_real_t gain)
{
    /* we use 24bit samples in fluid_rvoice_dsp. in order to normalize float
     * samples to [0.0;1.0] divide samples by the max. value of an int24 and
     * amplify them with the gain */
    const fluid_real_t INT24_MAX = (1 << (16+8-1)) * 1.0f;
    const fluid_real_t MIN_GAIN = pow(10.0, (double) 960.0 / -200.0);

    if (gain <= MIN_GAIN) {
        return 0.0f;
    }
    else {
        return gain * voice->synth_gain / INT24_MAX;
    }
}

mawe42 · 2018-01-04T13:40:12Z

Maybe I should think about this more before posting. Actually that only takes pan and balance into account, not the rest of the volume envelope. So the check should be in fluid_rvoice_calc_amp, testing if target_amp is less than pow(10.0, (double) 960.0 / -200.0).

mawe42 · 2018-01-04T14:07:40Z

Ok, I think this "96 dB should be zero" problem is outside of the scope of this feature addition, as it also affects other attenuation sources like expression and volume.

What about this: we change the balance to use centibels and live with not-completely-silent-channels problem for now. In addition, we open a ticket for the attenuation problem and solve that separately.

derselbst · 2018-01-04T14:25:57Z

we change the balance to use centibels and live with not-completely-silent-channels problem for now.

Ok.

In addition, we open a ticket for the attenuation problem and solve that separately.

Feel free to do so.

If you still need advice/review from mrbumpy409, he's on github, you could just ping him.

derselbst

Tested and seems to works as expected. I would leave this open for a few days to give the mailing list community some time. You can merge this whenever you want.

mawe42 · 2018-01-06T12:45:46Z

Ok, thanks a lot for testing! Will wait a few days and then merge.

jjceresa · 2018-01-29T15:24:39Z

@mawe42 I have done a quick look inside (not in depth). I only just wanted to get confirmation about the intended implementation.
Both, existing PAN modulator and new BALANCE modulator (this PR) now act simultaneously on the final voices amp (left and right) parameters. That is:

On CC pan change , both new CC pan value and current CC balance value update voice amp (left,right). ?
similarly, on CC balance change , both new CC balance value and current CC pan value update voice amp (left,right). ?

So actually the implementation seems to be done in the right place (i.e on the MIDI side task) without impacting performance of the audio rendering function (i.e on the audio task side).

I have not yet tested the CC balance response (i.e the function amp = f(CC balance)). I must admit that i haven't yet understood the intended response behaviour. Probably this could be easily resumed ?.
I guess default balance value is neutral (i.e centered) and behaves as before ?
Pan behaviour seems to be closer the same as before ?.

Thanks for your work.

mawe42 · 2018-02-04T10:59:58Z

@jjceresa Sorry for the late answer! Yes, pan and balance both affect the voices amp values.

I must admit that i haven't yet understood the intended response behaviour. Probably this could be easily resumed ? I guess default balance value is neutral (i.e centered) and behaves as before ?

Yes, the initial balance value is neutral, so if you don't set a balance value, the sound is unaffected. The intended response behaviour is just like the stereo balance knob on any home hifi system. So "turning the knob" to the left makes the right channel quieter but leaves the left channel untouched.

Pan behaviour seems to be closer the same as before ?

Yes, pan behaviour is unaffected by this change.

jjceresa · 2018-02-04T21:48:00Z

So "turning the knob" to the left makes the right channel quieter.

Yes , but please what is the response curve: left volume = f (from balance center to balance left) ?.
(i.e the function f is it linear, logarithmic, or other ?)

derselbst · 2018-02-05T07:55:39Z

The responsible function is fluid_balance(), which uses fluid_cb2amp(), i.e. pow(10.0, (double) i / -200.0) 2018-02-04 22:48 GMT+01:00 jjceresa <[email protected]>:

…

So "turning the knob" to the left makes the right channel quieter. Yes , but please what is the response curve: left volume = f (from balance center to balance left) ?. (i.e the function fis it linear, logarithmic, or other ?) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#317 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHxloCUFJu2toEDQQ_xf9Aph_dUjZXH7ks5tRiWRgaJpZM4RSaj1> .

mawe42 · 2018-02-05T08:00:17Z

(i.e the function fis it linear, logarithmic, or other ?)

As Tom said, it's using fluid_cb2amp at the moment. Which response curve to use has been subject to quite a bit of discussion already. But I would be very interested in your opinion on the subject!

jjceresa · 2018-02-05T20:51:45Z

it's using fluid_cb2amp at the moment, ..But I would be very interested in your opinion on the subject.

amp = fb(fluid_cb2amp(balance)) seems a good choice as it is also the response of amp = fv(volume) and both are what we have on an home hifi.

derselbst · 2018-02-11T14:20:59Z

Let's merge this as well.

mweseloh added 2 commits January 4, 2018 00:54

Add support for channel balance control (CC 8)

8f9b895

Use range with true center point for pan and balance

ff3e918

As specified in MIDI Recommended Practice (RP-036)

derselbst reviewed Jan 4, 2018

View reviewed changes

Rename default_balance_mod to custom_balance_mod

52cc645

Use absolute centibels for range of balance generator

2e4a517

derselbst approved these changes Jan 6, 2018

View reviewed changes

derselbst added the enhancement label Jan 6, 2018

derselbst added this to the 2.0 milestone Jan 6, 2018

derselbst assigned mawe42 Jan 6, 2018

mawe42 mentioned this pull request Jan 6, 2018

True zero gain for maximum attenuation #319

Closed

rename GEN_BALANCE to GEN_CUSTOM_BALANCE

297372d

Merge branch 'master' into balance

d3cfa28

derselbst merged commit 825216f into master Feb 11, 2018

derselbst deleted the balance branch February 11, 2018 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for balance control (CC 8) #317

Add support for balance control (CC 8) #317

mawe42 commented Jan 4, 2018

derselbst Jan 4, 2018

mawe42 Jan 4, 2018

derselbst Jan 4, 2018

mawe42 Jan 4, 2018

mawe42 Jan 4, 2018

derselbst Jan 4, 2018

mawe42 Jan 4, 2018

derselbst Jan 4, 2018

mawe42 Jan 4, 2018 •

edited

Loading

derselbst Jan 4, 2018

mawe42 commented Jan 4, 2018 •

edited

Loading

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

mawe42 commented Jan 4, 2018 •

edited

Loading

mawe42 commented Jan 4, 2018

mawe42 commented Jan 4, 2018

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

derselbst left a comment

mawe42 commented Jan 6, 2018

jjceresa commented Jan 29, 2018 •

edited

Loading

mawe42 commented Feb 4, 2018

jjceresa commented Feb 4, 2018

derselbst commented Feb 5, 2018 via email

mawe42 commented Feb 5, 2018

jjceresa commented Feb 5, 2018

derselbst commented Feb 11, 2018

Add support for balance control (CC 8) #317

Add support for balance control (CC 8) #317

Conversation

mawe42 commented Jan 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mawe42 Jan 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mawe42 commented Jan 4, 2018 • edited Loading

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

mawe42 commented Jan 4, 2018 • edited Loading

mawe42 commented Jan 4, 2018

mawe42 commented Jan 4, 2018

mawe42 commented Jan 4, 2018

derselbst commented Jan 4, 2018

derselbst left a comment

Choose a reason for hiding this comment

mawe42 commented Jan 6, 2018

jjceresa commented Jan 29, 2018 • edited Loading

mawe42 commented Feb 4, 2018

jjceresa commented Feb 4, 2018

derselbst commented Feb 5, 2018 via email

mawe42 commented Feb 5, 2018

jjceresa commented Feb 5, 2018

derselbst commented Feb 11, 2018

mawe42 Jan 4, 2018 •

edited

Loading

mawe42 commented Jan 4, 2018 •

edited

Loading

mawe42 commented Jan 4, 2018 •

edited

Loading

jjceresa commented Jan 29, 2018 •

edited

Loading