AX: fix missing ramp of main surround channel #10643
Merged
+73
−52
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tilka
force-pushed
the
ax_cleanup
branch
3 times, most recently
from
662d8ea
to
6967603
Compare
BhaaLseN
reviewed
May 7, 2022
| { | ||
| ret |= MIX_L_RAMP | MIX_R_RAMP; | ||
| if (ret & MIX_AUXA_L) | ||
| ret |= MIX_AUXA_L_RAMP | MIX_AUXA_R_RAMP; | ||
| if (ret & MIX_AUXB_L) | ||
| ret |= MIX_AUXB_L_RAMP | MIX_AUXB_R_RAMP; | ||
| if (ret & MIX_AUXA_S) | ||
| ret |= MIX_AUXA_S_RAMP; | ||
| if (ret & MIX_AUXB_S) | ||
| ret |= MIX_AUXB_S_RAMP; | ||
| } | ||
| ret |= MIX_ALL_RAMPS; |
There was a problem hiding this comment.
Am I correct in guessing that the intended change here is equivalent to adding | MIX_MAIN_S_RAMP to the first line, like it's done below if m_crc is not 0x4e8a8b21?
If so this consolidation seems wrong to me, but I'm not familiar with the DSP so I could be missing context. For example, if the 0x8 bit of mixer_control is set and the lowest three bits are not then in the old code none of these ifs are true leaving ret == 0xf, while in this version it becomes 0xAAAAAF.
There was a problem hiding this comment.
- Make sure to look at the two commits individually.
- Yes, setting all ramp flags changes the value of
ret. However, a ramp flag has no effect if its corresponding channel is not active. See theMIX_ONchecks in AXVoice.h that precede each use ofRAMP_ON.
Dentomologist
approved these changes
May 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Also more renaming. Not sure if fixing the ramp makes a noticeable difference anywhere.