Optimize bDelayPan and divisions#3706
Conversation
| if ( ( i & 1 ) == 0 ) | ||
| { | ||
| // if even : left channel | ||
| vecfIntermProcBuf[i] += vecsData[i] * fGainL; | ||
| } | ||
| else | ||
| { | ||
| // if odd : right channel | ||
| vecfIntermProcBuf[i] += vecsData[i] * fGainR; | ||
| } |
There was a problem hiding this comment.
Got rid of modulo computation
ann0see
force-pushed
the
opt1/mixencodetx
branch
2 times, most recently
from
c6038cc to
ccd6f5e
Compare
ann0see
left a comment
ann0see
left a comment
There was a problem hiding this comment.
The function is still way too complex imo.
|
To avoid having the clipping to short twice, we may want someting like Maybe... |
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think there's a mono only code path?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
// 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")
There was a problem hiding this comment.
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.
2 participants
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