Fix OBR resampler downsampling errors and rate-dependent binaural loudness - #14
Merged
Conversation
trsonic
requested review from
felicialim,
jingbo-marquis,
trevorknight,
yeroro and
yilun-zhangs
July 27, 2026 08:01
felicialim
reviewed
Jul 27, 2026
| // {source_rate, destination_rate, tone_frequency}; each tone sits at 1.5x | ||
| // the destination Nyquist frequency. | ||
| const int kCases[][3] = { | ||
| {96000, 48000, 36000}, {192000, 48000, 72000}, {88200, 44100, 33000}}; |
There was a problem hiding this comment.
is it worth adding a test where the downsampling factor is not an integer?
Collaborator
Author
There was a problem hiding this comment.
Good idea. Added in the 50846a162635a9688794a1983be607a56cb5df9c commit.
…pler GenerateInterpolatingFilter derived coeffs_per_phase_ from max(up_rate_, down_rate_), but the polyphase decomposition built by ArrangeFilterAsPolyphase has up_rate_ branches. Whenever down_rate_ > up_rate_ (downsampling), the tail of the interpolation filter was silently dropped: for integer ratios the kept segment ends before the sinc main lobe. Measured gain errors: +0.65 dB (88.2k->48k), -2.47 dB (96k->48k), -35.1 dB (192k->48k), -41.0 dB (176.4k->48k), with correspondingly degraded anti-aliasing (THD+N as poor as -5 dB at 176.4k->48k). The common 44.1k<->48k pair only loses near-zero window-tail taps, which is why the bug went unnoticed. Upsampling was unaffected (up_rate_ == max_rate there). Fix: partition the prototype filter across up_rate_ phases. Note this increases coeffs_per_phase_ (and thus per-sample cost and state size) for downsampling - the correct cost the truncated filter was not paying. Adds a regression test asserting unity passband gain (+-0.1 dB) for a 1 kHz tone across seven rate pairs; it fails on the previous code (amplitude 0.018 at 192k->48k) and passes with the fix. All 23 obr CMake tests pass.
The truncated polyphase filter tail did not only cause passband gain errors; it also degraded stopband (anti-aliasing) attenuation when downsampling. Verify that a tone at 1.5x the destination Nyquist frequency is attenuated below -34 dB for representative rate pairs. Before the partition fix, the 96k->48k and 88.2k->44.1k cases leaked the tone at -11 dB.
Process() carries the last coeffs_per_phase_ - 1 input samples between calls in state_. The branch handling input buffers shorter than that state addressed the buffer relative to state_channel.end(). That was correct in the original Resonance Audio code, where state_ was allocated at exactly coeffs_per_phase_ - 1 frames, but this port preallocates state_ at kMaxSupportedNumFrames, so end()-relative writes landed in a dead region of the buffer while the convolution reads the state from the front. Any block-based use with blocks shorter than the filter state therefore produced corrupted output (near-silence with glitches at block boundaries). Address the live region relative to begin(), matching the read path and the long-input branch: shift the old state down by input_length and append the entire input. Adds ChunkedProcessingMatchesOneShot, which verifies block-based processing is sample-exact against one-shot processing for both resampling directions, including blocks shorter than the filter state.
CreateShHrirsFromWav resamples the SH HRIRs to the target rate with a waveform-preserving signal resampler and uses the result directly as convolution filters. Those two operations have different invariants: the gain of the filter realized by discrete convolution is proportional to the rate at which the underlying continuous impulse response is sampled, so the uncompensated result scales the rendered binaural level by target_rate/native_rate at every frequency. With the 48 kHz-native bundled assets, rendering at 96 kHz came out +6.02 dB loud and 44.1 kHz -0.73 dB quiet (measured as exactly 2x and exactly 44100/48000 on the DTFT of the loaded filters). Scale the resampled HRIRs by wav_rate/target_rate so the realized frequency response - and thus the rendered loudness - is invariant to the output rate. Adds sh_hrir_creator_test covering: cross-channel response-norm invariance over 44.1/88.2/96 kHz at five frequencies, per-channel invariance across asset variants (orders, profiles, both ears), native-rate load determinism, and resampled length vs rate ratio. Fixes #2.
sh_hrir_creator.cc now includes obr/audio_buffer/simd_utils.h for ScalarMultiply, so add //obr/audio_buffer:simd_utils to the sh_hrir_creator library deps (required for the Bazel build to compile). Register the new sh_hrir_creator_test cc_test, mirroring resampler_test, so it is covered under the OBR Bazel build as it already is under CMake.
trsonic
force-pushed
the
fix/obr-resampler-shhrir-dsp
branch
from
July 27, 2026 18:08
a13fa13 to
d5f7d43
Compare
Extend the anti-aliasing test with an 88.2k->48k case (factor 1.8375) and the passband gain test with a 48k->32k pair (factor 1.5). The aliasing probe tone sits at 1.5x the destination Nyquist, which for a factor-1.5 pair coincides with the source Nyquist, so that pair is covered by the gain test instead.
felicialim
approved these changes
Jul 27, 2026
This file contains hidden or 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
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.Suggestion cannot be applied right now. Please check back later.
Three DSP fixes in the vendored OBR subtree (
src/renderer/obr/obr_capi/obr/obr/ambisonic_binaural_decoder/), each with regression tests. These surface whenever the output sample rate differs from the 48 kHz-native HRIR assets, or when audio is processed in blocks shorter than the resampler's filter state.1. Polyphase partition bug in the resampler (downsampling gain/anti-aliasing errors)
GenerateInterpolatingFilterderivedcoeffs_per_phase_frommax(up_rate_, down_rate_), but the polyphase decomposition built byArrangeFilterAsPolyphasehasup_rate_branches. Wheneverdown_rate_ > up_rate_(downsampling), the tail of the interpolation filter was silently dropped; for integer ratios the kept segment ends before the sinc main lobe. Measured gain errors: +0.65 dB (88.2k→48k), −2.47 dB (96k→48k), −35.1 dB (192k→48k), −41.0 dB (176.4k→48k), with correspondingly degraded anti-aliasing (THD+N as poor as −5 dB at 176.4k→48k). The common 44.1k↔48k pair only loses near-zero window-tail taps, which is why this went unnoticed. Upsampling was unaffected.Fix: partition the prototype filter across
up_rate_phases. Note this increasescoeffs_per_phase_(per-sample cost and state size) for downsampling — the correct cost the truncated filter was not paying.2. State-buffer addressing for input blocks shorter than the filter state
Process()carries the lastcoeffs_per_phase_ − 1input samples between calls instate_. The branch handling short input buffers addressed the buffer relative tostate_channel.end(). That was correct in the original Resonance Audio code, wherestate_was allocated at exactlycoeffs_per_phase_ − 1frames, but this port preallocatesstate_atkMaxSupportedNumFrames, so end()-relative writes landed in a dead region while the convolution reads the state from the front. Any block-based use with blocks shorter than the filter state produced corrupted output (near-silence with glitches at block boundaries).Fix: address the live region relative to
begin(), matching the read path and the long-input branch.3. HRIR filter gain not compensated for sample-rate conversion
CreateShHrirsFromWavresamples the SH HRIRs to the target rate with a waveform-preserving resampler and uses the result directly as convolution filters. Those two operations have different invariants: the gain of the filter realized by discrete convolution is proportional to the rate at which the underlying continuous impulse response is sampled, so the uncompensated result scales the rendered binaural level bytarget_rate / native_rateat every frequency. With the 48 kHz-native bundled assets, rendering at 96 kHz came out +6.02 dB loud and 44.1 kHz −0.73 dB quiet (measured as exactly 2× and exactly 44100/48000 on the DTFT of the loaded filters).Fix: scale the resampled HRIRs by
wav_rate / target_rateso rendered loudness is invariant to the output rate.Tests
resampler_test: unity passband gain (±0.1 dB) for a 1 kHz tone across seven rate pairs (fails on the old code at e.g. amplitude 0.018 for 192k→48k); anti-aliasing assertion for 96k→48k and 88.2k→44.1k (the tone previously leaked at −11 dB);ChunkedProcessingMatchesOneShotverifying block-based processing is sample-exact against one-shot for both directions, including blocks shorter than the filter state.sh_hrir_creator_test: cross-channel response-norm invariance over 44.1/88.2/96 kHz at five frequencies, per-channel invariance across asset variants (orders, profiles, both ears), native-rate load determinism, and resampled length vs. rate ratio.