Optimize bDelayPan and divisions by ann0see · Pull Request #3706 · jamulussoftware/jamulus

ann0see · 2026-05-20T20:38:00Z

Short description of changes

Moves delay pan checking slightly out of loop as this setting does not change often.
Moreover replaces some divisions with *0.5 and remove some modulo computations

CHANGELOG: Server: Optimize mixing

Context: Fixes an issue?

Related to: #3662

Does this change need documentation? What needs to be documented and how?

No

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?

Tested and it seems to work, I still would like an external test.

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want
My code follows the style guide
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

ann0see · 2026-05-20T20:38:29Z

-                        if ( ( i & 1 ) == 0 )
-                        {
-                            // if even : left channel
-                            vecfIntermProcBuf[i] += vecsData[i] * fGainL;
-                        }
-                        else
-                        {
-                            // if odd  : right channel
-                            vecfIntermProcBuf[i] += vecsData[i] * fGainR;
-                        }


Got rid of modulo computation

ann0see

The function is still way too complex imo.

ann0see · 2026-05-20T20:56:46Z

To avoid having the clipping to short twice, we may want someting like

    // convert from double to short with clipping
    for ( i = 0; i < ( ( bTargetIsMono ? 1 : 2 ) * iServerFrameSizeSamples ); i++ )
    {
        vecsSendData[i] = Float2Short ( vecfIntermProcBuf[i] );
    }

Maybe...

pljones · 2026-05-21T09:10:19Z

            const float fGain = vecvecfGains[iChanCnt][j];
            const float fPan  = bDelayPan ? 0.5f : vecvecfPannings[iChanCnt][j];

            // calculate combined gain/pan for each stereo channel where we define


This could do with expanding.

I thought the Server up-mixed all incoming channels to stereo (so it could treat them the same for all connected clients), did all the mixes in stereo - to allow a single path (either delay pan or not) - and then down-mixed to mono for clients with mono out.

The changes here seem to do something different but I don't quite follow it.

I think there's a mono only code path?

There is in this new flow. What I mean is I don't understand
a) exactly what was happening before this new mono flow was introduced
b) exactly what is happening now
c) whether it's therefore having the right effect

So it needs a bigger comment in the code to explain it.

Well... it needs a good answer 😄 and then a good enough comment to explain the new code.

New flow:

if delay pan: // code for delay pan in mono or stereo mode // not sure why there is even delay panning in mono mode but ok. else: // mono or stereo mixing

old flow:

if mono: if delay pan: // delay pan mixing else: // non delay pan mixing else: if delay pan: // parity based computation for delay pan else: // parity based mixing

Please check the code itself, not the diff as this is confusing

// not sure why there is even delay panning in mono mode but ok.

A mono client can pan any client in their mix and it will affect the output. ("I want more of their left channel in my mix down")

A stereo client can pan any client in their mix and it will affect the output. ("I want them moved left in my mix")

Please check the code itself, not the diff as this is confusing

I can see it moves the two ifs inside out. But I can't see where the panning is applied on the source channel for a mono target now.

Not sure I understand correctly. I didn't change the semantics I believe?
I believe that a mono target may not have panning?

A mono client can pan any client in their mix and it will affect the output.

If the source is stereo, it has two channels. The mono target can, therefore, adjust the mix - either with delaypan or not - that gets then mixed down to mono.

I think - but I'm not sure - the change here inverts the logic and prevents that.

Ok. Let's disect the logic.

vecNumAudioChannels[iChanCnt] == 1 => mono target channel. I only replaced /2 by *0.5f which is equivalent

vecNumAudioChannels[iChanCnt] != 1 => stereo target channel

Let j be arbitrary between 0 and iNumClients -1

case bDelayPan == false and vecNumAudioChannels[j] == 1 ( isMono == true):

Old version skips setting iPanDel, iPanL, iPanR statement

new version skips entire if (bDelayPan) section moving to the else block.

Since cNumAudioChannels[j] == 1 , old version enters mono: "copy same mono data" ... loop.

Since isMono == true, new version enters "copy same mono data" ... loop

since bDelayPan == false, old version skips // pan address shift and executes vecfIntermProcBuf[k] += vecsData[i] * fGainL [...]

since bDelayPan == false, and the entire outer conditional is already skipped before, new version checks (isMono) and executes vecfIntermProcBuf[k] += vecsData[i] * fGainL

=> looks valid

case bDelayPan == false and vecNumAudioChannels[j] != 1 ( isMono == false):

old version enters //stereo.

new version enters else and checks isMono -> ` // left/right channel vecfIntermProcBuf[i] += vecsData[i] * fGainL;```

old version checks bDelayPan == false, and thus checks if ( ( i & 1 ) == 0 ) [...] -> optimised in the new version

This also looks valid?

case bDelayPan == true and vecNumAudioChannels[j] == 1 ( isMono == true):

old version enters // mono: copy same[]...

new version enters iPanDel[...]

old version enters // pan address shift [...] ilpan = I- iPanDelL...

new version enters same

==> also looks valid

case bDelayPan == true and vecNumAudioChannels[j] != 1 ( isMono == false)

New:

// stereo for ( i = 0; i < ( 2 * iServerFrameSizeSamples ); i += 2 ) { // pan address shift iLpan = i - 2 * iPanDelL;

Old (unoptimised)

for ( i = 0; i < ( 2 * iServerFrameSizeSamples ); i++ ) { // left/right channel if ( bDelayPan ) { // pan address shift if ( ( i & 1 ) == 0 ) { iPan = i - 2 * iPanDelL; // if even : left channel }

Also looks valid.

Could you please specify the variable state which you think is invalid?

Since cNumAudioChannels[j] == 1 , old version enters mono: "copy same mono data" ... loop.

Uh? How did you get that? vecNumAudioChannels[j] is not equivalent to cNumAudioChannels[j] so I don't see where that came from.

This is what I mean -- I think cNumAudioChannels[j] is where the sound comes from and vecNumAudioChannels[j] is where the sound goes to. You look at vecNumAudioChannels[j] to determine isMono and then assume that means every connected client is mono for that output.

dingodoppelt · 2026-05-25T11:45:57Z

                    for ( i = 0, k = 0; i < iServerFrameSizeSamples; i++, k += 2 )
                    {
-                        vecfIntermProcBuf[i] += ( static_cast<float> ( vecsData[k] ) + vecsData[k + 1] ) / 2;
+                        vecfIntermProcBuf[i] += ( static_cast<float> ( vecsData[k] ) + vecsData[k + 1] ) * 0.5f;


Couldn't we just right shift the short to halve it?

dingodoppelt · 2026-05-25T11:50:07Z

                    for ( i = 0, k = 0; i < iServerFrameSizeSamples; i++, k += 2 )
                    {
-                        vecfIntermProcBuf[i] += fGain * ( static_cast<float> ( vecsData[k] ) + vecsData[k + 1] ) / 2;
+                        vecfIntermProcBuf[i] += fGain * ( static_cast<float> ( vecsData[k] ) + vecsData[k + 1] ) * 0.5f;


I don't know if we do this right. Shouldn't we normalize the floats to -1 .. 1 to get most out of the float precision? And even if we did we wouldn't mitigate clipping because adding many channels might still clip while we don't scale back or check, if clipping occurred. When converting back to short we don't care for the actual range, we just hard clip everything back into the short range. That doesn't seem right at all to me.

ann0see commented May 20, 2026

View reviewed changes

ann0see requested review from pljones and softins May 20, 2026 20:38

ann0see added this to Tracking May 20, 2026

github-project-automation Bot moved this to Triage in Tracking May 20, 2026

ann0see moved this from Triage to Waiting on Team in Tracking May 20, 2026

ann0see force-pushed the opt1/mixencodetx branch 2 times, most recently from c6038cc to ccd6f5e Compare May 20, 2026 20:45

ann0see commented May 20, 2026

View reviewed changes

ann0see force-pushed the opt1/mixencodetx branch from ccd6f5e to 62a6218 Compare May 20, 2026 20:47

Optimize MixEncodeTransmitData()

d6372fb

ann0see force-pushed the opt1/mixencodetx branch from 62a6218 to d6372fb Compare May 20, 2026 20:57

pljones reviewed May 21, 2026

View reviewed changes

dingodoppelt reviewed May 25, 2026

View reviewed changes

Conversation

ann0see commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ann0see left a comment

Choose a reason for hiding this comment

Uh oh!

ann0see commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pljones May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pljones May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ann0see May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pljones May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dingodoppelt May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ann0see commented May 20, 2026 •

edited

Loading

ann0see commented May 20, 2026 •

edited

Loading

pljones May 21, 2026 •

edited

Loading

pljones May 22, 2026 •

edited

Loading

ann0see May 24, 2026 •

edited

Loading

pljones May 25, 2026 •

edited

Loading

dingodoppelt May 25, 2026 •

edited

Loading