silk_Encode: stereo-to-mono downmix clips at -6 dBFS since 1.6
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 76/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- c
- Domain
- audio-video-rtc
Research direction
Read silk/enc_API.c around line 317 and inspect the stereo API to mono internal-channel downmix. Compare the current sum and saturation with the described 1.5.2 arithmetic, then check the relevant encoder tests or add a focused regression case for loud stereo input. Done means the downmix no longer clips the summed channels at half scale and the regression case passes.
Written by the indexing model from the issue text.
Description
In silk/enc_API.c, when the API is stereo and SILK codes mono (nChannelsAPI == 2 && nChannelsInternal == 1), the downmix saturates the sum of the two channels to 16 bits before it halves it:
sum = RES2INT16(samplesIn[ 2 * n ] + samplesIn[ 2 * n + 1 ]);
buf[ n ] = (opus_int16)silk_RSHIFT_ROUND( sum, 1 );
RES2INT16 saturates (FLOAT2INT16 in float builds, SAT16(PSHR32(a, RES_SHIFT)) in fixed point). Wherever L+R goes past full scale the sum is held at 32767 and then halved to 16384, so a signal carried on both channels clips at half scale (-6 dBFS) before SILK sees it.
In 1.5.2 the input was opus_int16 and the sum was taken in 32 bits before halving (sum = samplesIn[ 2 * n ] + samplesIn[ 2 * n + 1 ];), so nothing clipped. The change came with the opus_res input in 1.6 and is still on main (line 317).
Where it shows: a voice app that feeds a mono microphone as 2-channel input at a bitrate where Opus codes SILK in mono. We hit it updating from 1.5.2 to 1.6.1: loud speech peaks clipped.
The change we build with, which keeps 1.5.2's arithmetic:
--- a/silk/enc_API.c
+++ b/silk/enc_API.c
@@ -314,7 +314,7 @@
} else if( encControl->nChannelsAPI == 2 && encControl->nChannelsInternal == 1 ) {
/* Combine left and right channels before resampling */
for( n = 0; n < nSamplesFromInput; n++ ) {
- sum = RES2INT16(samplesIn[ 2 * n ] + samplesIn[ 2 * n + 1 ]);
+ sum = RES2INT16(samplesIn[ 2 * n ]) + RES2INT16(samplesIn[ 2 * n + 1 ]);
buf[ n ] = (opus_int16)silk_RSHIFT_ROUND( sum, 1 );
}
- Dominant language
- C
- Stars
- 3.4k
- Forks
- 820
- PR merge metrics
- No merged PRs in 30d
Getting set up
This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from xiph/opus
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 5/5 Over a week Newbie friendliness 25/100
-
Difficulty 4/5 3-5 days Newbie friendliness 45/100
-
Windows CMake CI is brokenMay be free again A pull request for this issue was closed without being merged. Open
Difficulty 3/5 1-2 days Newbie friendliness 40/100
-
Signed integer overflow in frame_size_select's own validation lets a huge frame_size through to a stack allocationMay be free again A pull request for this issue was closed without being merged. Open
Difficulty 3/5 1-2 days Newbie friendliness 68/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
InukaAsith/TwinDesk#84 ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
CanadaHonk/porffor#426 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
FujiNetWIFI/fujinet-firmware#2221 ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 77/100
raysan5/raylib.com#132 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100