Skip to content

fix(moq): fail connection on audio codec mismatch in pull node#461

Merged
streamer45 merged 1 commit into
mainfrom
devin/1778957949-fix-moq-codec-mismatch
May 17, 2026
Merged

fix(moq): fail connection on audio codec mismatch in pull node#461
streamer45 merged 1 commit into
mainfrom
devin/1778957949-fix-moq-codec-mismatch

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor

Summary

When catalog discovery during initialize() fails or times out, the MoQ pull node keeps the constructor's default output pin type (Opus). If run_connection() later discovers a different catalog audio codec, it previously only logged a warning but continued sending packets with the mismatched codec — a pin contract violation.

This PR makes run_connection() return Err(StreamKitError::Runtime(...)) on codec mismatch, which causes the reconnection loop (line 197) to retry, giving catalog discovery another chance.

Changes:

  • After the existing tracing::warn! in the mismatch block (~line 687), return an error to fail the connection
  • Replace the trivial test_codec_mismatch_detectable test (which only asserted Opus != Aac) with four focused tests:
    • test_default_node_has_opus_output_pin — default config produces Opus pin
    • test_codec_mismatch_detected_between_pin_and_catalog — mismatch scenario (Opus pin vs AAC catalog)
    • test_no_mismatch_when_pin_matches_catalog — matching codecs don't trigger failure
    • test_explicit_aac_config_avoids_mismatch_with_aac_catalog — explicit AAC config avoids mismatch

Closes #454

Review & Testing Checklist for Human

  • Verify that the error message in run_connection() is clear and actionable in logs
  • Confirm that the reconnection loop at line 197 handles this error the same as other StreamKitError::Runtime errors (1s retry)
  • Test with a real MoQ broadcast where catalog discovery initially times out but succeeds on retry — verify the node recovers

Notes

This is the safer of the two options from the issue — failing the connection rather than updating pin types at runtime. Runtime pin updates can be done as a follow-up if needed.

Link to Devin session: https://staging.itsdev.in/sessions/8ccfbaefee4d496ab9e2b402efabf989
Requested by: @streamer45

When catalog discovery during initialize() times out, the MoQ pull node
keeps the constructor's default output pin codec. If run_connection()
later discovers a different catalog audio codec, the node now returns an
error instead of silently continuing with a mismatched pin type. This
causes the reconnection loop to retry, giving catalog discovery another
chance.

Closes #454

Signed-off-by: Staging-Devin AI <166158716+staging-devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.93%. Comparing base (bddff06) to head (3a344b3).

Files with missing lines Patch % Lines
crates/nodes/src/transport/moq/pull.rs 77.77% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #461      +/-   ##
==========================================
+ Coverage   65.91%   65.93%   +0.02%     
==========================================
  Files         217      217              
  Lines       57496    57530      +34     
  Branches     1597     1597              
==========================================
+ Hits        37896    37931      +35     
+ Misses      19594    19593       -1     
  Partials        6        6              
Flag Coverage Δ
backend 65.01% <77.77%> (+0.02%) ⬆️

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

Components Coverage Δ
core 84.29% <ø> (ø)
engine 75.57% <ø> (ø)
api 84.73% <ø> (ø)
nodes 67.46% <77.77%> (+0.06%) ⬆️
server 57.16% <ø> (-0.02%) ⬇️
plugin-native 70.93% <ø> (ø)
plugin-wasm 6.37% <ø> (ø)
ui-services 74.73% <ø> (ø)
ui-components 60.49% <ø> (ø)
Files with missing lines Coverage Δ
crates/nodes/src/transport/moq/pull.rs 35.01% <77.77%> (+1.86%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@streamer45 streamer45 merged commit fad6274 into main May 17, 2026
45 of 46 checks passed
@streamer45 streamer45 deleted the devin/1778957949-fix-moq-codec-mismatch branch May 17, 2026 18:10
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.

MoQ pull: runtime codec mismatch should update output pin or fail connection

1 participant