Skip to content

Add SILK encoder struct#165

Open
thomas-vilte wants to merge 1 commit into
pion:mainfrom
thomas-vilte:feat/silk-encoder-struct
Open

Add SILK encoder struct#165
thomas-vilte wants to merge 1 commit into
pion:mainfrom
thomas-vilte:feat/silk-encoder-struct

Conversation

@thomas-vilte

Copy link
Copy Markdown
Member

Description

adds the real Encoder struct (state fields mirroring Decoder, NewEncoder/resetPredictionState) and everything that was blocked on it existing:

  • find_pred_coefs.go / find_pitch_lags.go: LTP analysis filter, residual energy, NLSF interpolation search, and the pitch-lag whitening stage
  • pulses.go / pitch_ltp.go: the excitation shell-code and pitch/LTP bitstream wiring — these turned out to be almost entirely *Encoder methods with no separable pure logic, so pieces 12/13 from the original split plan land here instead of as their own PRs
  • Reintroduces what pieces 7/8/9/10 had to defer: encodeNLSF/emitNLSFIndices, quantLTPGains, encodeSubframeGains/emitGainIndices, and all of noise_shape_analysis.go (noiseShapeAnalysis/processGains). Also moves nlsfToLPCQ12 in from where it used to live (the not-yet-written orchestration file) so this piece doesn't depend on that one

ported from NSQ.c's caller-side struct, find_pred_coefs_FLP.c, find_pitch_lags_FLP.c, encode_pulses.c, and the pitch/LTP sections of encode_indices.c

found and fixed a real bug along the way: noiseShapeAnalysis was reading the sparseness measure that picks quantOffsetType from the raw windowed signal instead of the whitened LPC residual findPitchLags produces — libopus passes these as two separate buffers (pitch_res vs x) to silk_noise_shape_analysis_FLP, and the port had collapsed them into one. Doesn't desync the bitstream (either offset type is valid), but it was silently biasing that choice. Fixed by adding the missing pitchRes parameter, with a regression test that forces both branches from the correct buffer

one thing I found but didn't fix: findLPCNLSF runs the NLSF-interpolation search unconditionally, where libopus gates it on useInterpolatedNLSFs (tied to an encoder complexity setting SILK doesn't have here at all — no SetComplexity equivalent exists yet). Fixing it properly means adding that whole control surface, which felt like its own piece of work rather than something to bolt on here. Left as a known gap

Reference issue

part of the SILK encoder series (#164)

Co-authored-by: François Allais <francois.allais@hotmail.com>
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.20493% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.85%. Comparing base (9291bd6) to head (718d127).

Files with missing lines Patch % Lines
internal/silk/noise_shape_analysis.go 96.73% 3 Missing and 2 partials ⚠️
internal/silk/find_pitch_lags.go 88.57% 2 Missing and 2 partials ⚠️
internal/silk/find_pred_coefs.go 93.44% 2 Missing and 2 partials ⚠️
internal/silk/pulses.go 96.52% 2 Missing and 2 partials ⚠️
internal/silk/ltp_quant.go 93.47% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
+ Coverage   89.61%   90.85%   +1.24%     
==========================================
  Files          48       54       +6     
  Lines        9049     9576     +527     
==========================================
+ Hits         8109     8700     +591     
+ Misses        706      638      -68     
- Partials      234      238       +4     
Flag Coverage Δ
go 90.85% <96.20%> (+1.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thomas-vilte
thomas-vilte requested a review from FrantaBOT July 24, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant