feat(audio): add channel-aware streaming output - #1259
ruiling-smartbear wants to merge 1 commit into
Conversation
Add an optional multichannel streaming capability without changing the existing mono callback or vtable. Route CLI streams through channel-aware validation and report channel metadata with raw PCM output. Test legacy dispatch, stereo chunk ordering, capability precedence, and malformed streams. Define callback lifetime and completion semantics; no model implementation or encoded transport is added. Refs: NVIDIA#1254 Signed-off-by: Ruilin Gao <ruiling@andrew.cmu.edu>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 SummarySummaryAdds channel-aware audio streaming through The CLI uses the multichannel capability when available. It falls back to existing mono generation when it is not available. Raw float32 JSON output now includes The CLI validates chunk shape, PCM format, channel count, sample rate, finite samples, sample-count overflow, producer errors, callback failures, and final sample counts. Tests cover stereo ordering, metadata, capability precedence, legacy mono behavior, and malformed streams. Architecture impact
PASS — The change preserves existing interfaces and vtables while adding an optional runtime capability. WalkthroughThe runtime adds a multichannel streaming capability with interleaved PCM chunks. The CLI selects this capability, validates streamed data, writes raw samples, reports channel metadata, and preserves mono fallback behavior. Tests and lifecycle documentation cover the new contract. ChangesMultichannel audio streaming
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Generator as IMultichannelStreamingAudioGeneration
participant Output as RawAudioFile
CLI->>Generator: Request streaming audio
Generator->>CLI: Deliver interleaved PCM chunks
CLI->>Output: Validate and write samples
Generator-->>CLI: Return total scalar sample count
CLI-->>Output: Close output and report num_channels
Merge Risk: ⚪ Minimal · up to The multichannel streaming path validates audio chunks and metadata while preserving mono fallback behavior. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 4 files. (1 skipped: 1 unsupported.) Comment |
|
Triggered the internal CI for you. But please work with @jkzhang7 on the minimax music PR to see what's the best way to get the stereo audio merge in |
|
Hi @yifeif-nv, the latest Community CPU checks have now passed. Could you please retrigger internal CI for #1259 when convenient? The previous attempt stopped at the CPU prerequisite gate. Thanks! |
Background
Refs #1254. The existing streaming callback carries samples and sample rate but cannot describe stereo. This adds the streaming counterpart to the complete-result/WAV proposal in #1256, as an independent PR based on main.
Exit Criteria
Implementation
Add
AudioChunkViewand the optionalIMultichannelStreamingAudioGenerationcapability. The CLI prefers that capability when available, otherwise uses the existing mono capability. Raw float32 output JSON now includesnum_channels.Document synchronous borrowed-buffer lifetime, completion by return, and exception propagation. Existing interfaces/vtables and
AudioResultare unchanged. No model-family, bundle-format, or dependency changes.Change categories
Validation
Commands and Results
On Linux, compiled and ran the actual CLI test with ASan/UBSan, using real loader/bundle sources and synthetic task fixtures:
g++ -std=c++17 -DTRTMC_VERSION_STRING='"0.1.0"' \ -fsanitize=address,undefined -fno-omit-frame-pointer -g \ -Wall -Wextra -Wpedantic -Wno-missing-field-initializers \ -Icore/runtime/include -Icore -Iapps -Ithird_party/stb \ -I/usr/local/lib/python3.11/site-packages/nlohmann_json/include \ -I/usr/local/lib/python3.11/site-packages/nvidia/cuda_runtime/include \ -I/usr/local/lib/python3.11/site-packages/nvidia/cuda_nvcc/include \ apps/cli/tests/test_cli.cpp apps/cli/cli.cpp apps/cli/io.cpp \ core/runtime/loader/family_loader.cpp core/runtime/bundle/bundle_format.cpp \ -ldl -o /tmp/test_cli /tmp/test_cliPassed (
ALL PASSED). Covers legacy mono, multichannel capability precedence, exact stereo chunk concatenation, metadata, and 12 failure modes: empty stream, null data, partial frame, empty/negative chunk count, invalid rate/channels, non-finite data, producer failure, rate/channel changes, and wrong final total.Also passed locally:
Hardware, Environment, and Revisions
Tested head
7fb82240, basea50cf5dc. Linux x86_64 Debian 12 CPU container, GCC 12, CUDA header packagesnvidia-cuda-runtime-cu12==12.4.127andnvidia-cuda-nvcc-cu12==12.4.131,nlohmann_json==3.12.0. Local API/format checks used Apple Silicon macOS and clang-format 22.1.8. No GPU or TensorRT inference; synthetic float32 audio only.Not Run / Remaining Gaps
No real model has been migrated to the new capability in this PR, so model streaming quality/latency is not qualified. Full repository CMake/CI was not run locally. HTTP, WAV streaming, and other encoded formats remain outside scope.
Contributor Self-Review
Notes For Future Readers
The new interface is opt-in and does not require #1256's
AudioResultchange.num_samplesmeans scalar samples, with whole frames in every chunk. A failed stream can leave a partial raw file, but must not emit success JSON. This is not a cancellation/preemption API; callback exceptions must propagate and the implementation must clean up its own generation state.Risk level
Existing ABI types are unchanged, but this is a new public capability and CLI validation now rejects malformed legacy chunks and inconsistent totals. Model-specific adoption needs its own validation.