Skip to content

Decode CELT frame side information#98

Merged
zshang-oai merged 2 commits intomainfrom
codex/celt-part4
Apr 15, 2026
Merged

Decode CELT frame side information#98
zshang-oai merged 2 commits intomainfrom
codex/celt-part4

Conversation

@zshang-oai
Copy link
Copy Markdown
Contributor

@zshang-oai zshang-oai commented Apr 8, 2026

Summary

This is the last piece before we get to "real meat" :) it's basically table 56 first eight columns.

Implements the first CELT frame side-info slice for RFC 6716 Section 4.3 work:

  • add CELT frame configuration validation and side-info parsing through the intra-energy flag
  • decode silence, pitch post-filter header, transient, and intra-energy flags in RFC Table 56 order
  • add rangecoding.Decoder.DecodeUniform for the RFC 6716 Section 4.1.5 ec_dec_uint() path needed by the post-filter octave
  • add a CELT-side range trace test harness that records Tell, TellFrac, remaining bits, and final range at named decode milestones

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 93.25843% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.56%. Comparing base (085f78c) to head (9d9c788).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/celt/frame.go 90.76% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
+ Coverage   80.98%   81.56%   +0.58%     
==========================================
  Files          15       16       +1     
  Lines        2135     2224      +89     
==========================================
+ Hits         1729     1814      +85     
- Misses        341      343       +2     
- Partials       65       67       +2     
Flag Coverage Δ
go 81.56% <93.25%> (+0.58%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@zshang-oai zshang-oai marked this pull request as ready for review April 8, 2026 17:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements the initial CELT frame “side information” decoding slice (per RFC 6716 §4.3 / Table 56 order) and adds supporting range-coder functionality plus trace-based tests to validate range decoder state at key milestones.

Changes:

  • Add rangecoding.Decoder.DecodeUniform() to support RFC 6716 ec_dec_uint() decoding (range-coded prefix + raw-bit suffix).
  • Introduce CELT frame config validation and side-info parsing through the intra-energy flag (silence, post-filter header, transient, intra-energy).
  • Add CELT range-trace test harness and targeted unit tests for the new decoding path.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/rangecoding/decoder.go Adds uniform integer decoding (DecodeUniform) needed for CELT post-filter parsing.
internal/rangecoding/decoder_test.go Adds unit tests covering DecodeUniform behavior (in-range, suffix combine, saturation, degenerate totals).
internal/celt/frame.go Implements CELT side-info parsing and frame config validation.
internal/celt/frame_test.go Adds tests for config validation, silence handling, post-filter decode, transient/intra flags, and range trace checkpoints.
internal/celt/trace_test.go Adds a reusable trace helper for asserting range-decoder state at named checkpoints.
internal/celt/errors.go Adds CELT-specific errors for invalid channel counts and range coder symbol issues.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/celt/frame.go
switch {
case tell >= info.totalBits:
info.silence = true
case tell == 1:
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decodeSilenceFlag only decodes the silence symbol when tell == 1. That’s currently true immediately after rangeDecoder.Init(), but if the call site changes (or the decoder is reused without re-init) the function will silently skip decoding even when tell < totalBits, leaving silence at its zero value. Consider removing the tell == 1 guard (decode whenever tell < totalBits) or documenting/enforcing the precondition that this function must be called right after Init().

Suggested change
case tell == 1:
default:

Copilot uses AI. Check for mistakes.
@zshang-oai zshang-oai merged commit d240917 into main Apr 15, 2026
17 checks passed
@zshang-oai zshang-oai deleted the codex/celt-part4 branch April 15, 2026 16:09
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.

3 participants