quick fix for flickering sound by lerping gain #2369
Conversation
|
Thanks for your contribution. Related to https://discourse.urho3d.io/t/sound-issues-crash-and-flickering/903 |
|
The linter is not happy with the spacing. |
|
Thanks for that. Don’t worry about Appveyor CI also failed now for your PR. It seems Appveyor default VM image has been upgraded to use VS 2017 15.8 now, which has a known issue in compiling our project. Will review and merge at the soonest. |
| ++dest; | ||
| INC_POS_LOOPED(); | ||
| } | ||
| if (!~rampSamples) |
There was a problem hiding this comment.
This line (and also on other similar lines) looks weird to me. After the while loop immediately above it exits the block then the variable must become 0. Thus, that line always evaluates to if (false). Anyway, I believe our linter also complaints about this one although not directly.
There was a problem hiding this comment.
No, if the while loop exits "normally" then the variable become
(unsigned)-1 (because of the postfix decrement), then the condition evaluated to true.
In this case it is just a safeguard, but the INC_POS_ONESHOT() macro can break the loop and in that case the second loop will be skipped.
| @@ -96,6 +99,8 @@ namespace Urho3D | |||
|
|
|||
| static const int STREAM_SAFETY_SAMPLES = 4; | |||
|
|
|||
| static const int VOLUME_DENOM = 4096; | |||
There was a problem hiding this comment.
Just curious how did you decide on this magic number. It is 2^12 for 16bit and reduce to 2^4 for 8bit audio. The existing code for 16bit multiply the volume with 256.f and then divide it back later, but for 8bit leave the multiplied volume as it is. The proposed code changed that to 4096.f for 16bit. It does not change much for 16bit audio, but it looks quite different now for 8bit audio. It becomes mult and divide by 16.f.
There was a problem hiding this comment.
These mixer routines are using integer arithmetic. In the original code the [0.f..1.f] gain values scaled to [0..256] integer values, so there was an integer division by 256. I just changed this scale value to 4096 for better precision.
In the 8bit case there is another multiply by 256 so the volume level does not change,
and it is "built in" the denominator.
I tried to choose this "magic number" big enugh to lower the quantization effect, but not too big to avoid overflow and long lerping time.
weitjong
force-pushed
the
master
branch
2 times, most recently
from
6a26075
to
9c7ea24
Compare
|
Thanks for the fix. I will have another look at it later. |
|
could I recommend that this is prioritised? I had audio issues with crackling etc all over the place and this fixed it. I had no problems merging this pull request to master, and it worked perfectly for my project ( thank you, ssinai1 ) |
|
Marking this stale since there has been no activity for 30 days. |
Fixing discontinuities caused by volume change and gain quantization.
(There is a thread about it : "Sound Issues: Crash and Flickering".
)