Skip to content

support file-based ASR for nemotron streaming model#885

Open
rui-ren wants to merge 2 commits into
mainfrom
ruiren/add-file-based-support-nemotron-streaming
Open

support file-based ASR for nemotron streaming model#885
rui-ren wants to merge 2 commits into
mainfrom
ruiren/add-file-based-support-nemotron-streaming

Conversation

@rui-ren

@rui-ren rui-ren commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Add file-based Nemotron ASR transcription support in C++ SDK v2

Summary

This PR completes file-based audio transcription support for Nemotron streaming ASR models in  sdk_v2/cpp  by finishing the C++  AudioSession  implementation and aligning behavior with the existing OpenAI-JSON transcription flow.

For more details, please refer to github/copilot-cli#4024 (comment)

What changed

• Implemented/finished Nemotron file transcription path in:

 sdk_v2/cpp/src/inferencing/generative/audio/audio_session.cc 
 sdk_v2/cpp/src/inferencing/generative/audio/audio_session.h 

• Added WAV file parsing for file-based transcription input:
• RIFF/WAVE validation
•  fmt / data  chunk handling with bounds checks
• 16kHz enforcement
• PCM16 and float32 decoding
• Added Nemotron-specific language runtime option mapping ( lang_id ) and temperature handling.
• Fixed decode flow to keep a single generator across incremental audio processing (100ms chunks + flush), which resolves truncated transcription output for file-based runs.

Behavior

• OpenAI-JSON file transcription for Nemotron models now returns full transcript text from WAV input in SDK v2.
• Existing streaming and non-Nemotron paths remain unchanged.

End-to-end scenario covered

• Model alias:  nemotron-3.5-asr-streaming-0.6b 
• Input:  sample-speech-1m-16k.wav 
• Path exercised: JS SDK v2  AudioClient.transcribe(...)  → C++ SDK v2 audio transcription pipeline

Copilot AI review requested due to automatic review settings July 15, 2026 17:13
@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
foundry-local Ready Ready Preview, Comment Jul 15, 2026 5:13pm

Request Review

@rui-ren rui-ren changed the title Ruiren/add file based support nemotron streaming support file-based ASR for nemotron streaming model Jul 15, 2026
@rui-ren
rui-ren requested a review from skottmckay July 15, 2026 17:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds file-based WAV transcription for Nemotron streaming ASR models in the C++ SDK.

Changes:

  • Routes Nemotron file requests through the streaming processor.
  • Parses 16 kHz PCM16/float32 WAV files with channel downmixing.
  • Adds language mapping, temperature handling, and OpenAI JSON output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
audio_session.h Declares Nemotron transcription and WAV parsing helpers.
audio_session.cc Implements Nemotron inference, decoding, language mapping, and WAV parsing.

int completion_tokens = 0;

auto decode_all_tokens = [&]() {
while (!generator->IsDone() && !original_request.canceled) {
size_t count = std::min(kNemotronSamplesPerChunk, samples.size() - offset);
run_one_pass(processor->Process(samples.data() + offset, count));
}
run_one_pass(processor->Flush());
}
}

auto samples = LoadPcmWavAsFloatSamples(req.filename);
@nenad1002

nenad1002 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Why? Parakeet can execute faster and more accurate file-based transcription. I wouldn't want customers to rely on streaming models for non-streaming scenarios, and we're encouraging them with this.

@rui-ren

rui-ren commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Why? Parakeet can execute faster and more accurate file-based transcription. I wouldn't want customers to rely on streaming models for non-streaming scenarios, and we're encouraging them with this.

This is not a way to encourage them to do file-base transcription with RNNT streaming model, as you can see here,

github/copilot-cli#4024 (comment)

they are using streaming model, however they do file-based transcription and get error raised. IMO, we better to cover this edge case for them, instead of switch models?

What do you think? @sylvanc

@nenad1002

Copy link
Copy Markdown
Contributor

Why? Parakeet can execute faster and more accurate file-based transcription. I wouldn't want customers to rely on streaming models for non-streaming scenarios, and we're encouraging them with this.

This is not a way to encourage them to do file-base transcription with RNNT streaming model, as you can see here,

github/copilot-cli#4024 (comment)

they are using streaming model, however they do file-based transcription and get error raised. IMO, we better to cover this edge case for them, instead of switch models?

What do you think? @sylvanc

Why? Parakeet can execute faster and more accurate file-based transcription. I wouldn't want customers to rely on streaming models for non-streaming scenarios, and we're encouraging them with this.

This is not a way to encourage them to do file-base transcription with RNNT streaming model, as you can see here,

github/copilot-cli#4024 (comment)

they are using streaming model, however they do file-based transcription and get error raised. IMO, we better to cover this edge case for them, instead of switch models?

What do you think? @sylvanc

Hmm, I haven't realized they're using file-based transcription. Ok, but there are better models for this going forward.

@rui-ren

rui-ren commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@skottmckay can you please review this PR ? Thanks.

text.reserve(512);
int completion_tokens = 0;

auto decode_all_tokens = [&]() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we put this in a named helper function rather than use a lambda function?

}
};

auto run_one_pass = [&](std::unique_ptr<OgaNamedTensors> tensors) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment here


// If the model is a Nemotron speech model, process the transcription using the Nemotron-specific method.
// e.g. RNNT models require Nemotron-specific transcription handling.
if (IsNemotronSpeechModel()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I presume that the v2 SDK already supported Nemotron when it was merged into main. Why do we now need an early return to support it?

auto tokenizer_stream = OgaTokenizerStream::Create(*tokenizer);
auto generator_params = OgaGeneratorParams::Create(oga_model);
if (temperature.has_value()) {
generator_params->SetSearchOption("temperature", *temperature);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we simplify this if-else branching to the following?

generator_params->SetSearchOption("temperature", temperature.value_or(0.0f));

FOUNDRY_LOCAL_TEXT_ITEM_TYPE_OPENAI_JSON));
}

std::vector<float> AudioSession::LoadPcmWavAsFloatSamples(const std::string& audio_file_path) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From Copilot:

Good addition overall — the Nemotron transcription path is clean, the WAV parser is thorough, and keeping a single generator across chunks is the right fix for truncated output. A few things worth addressing before merging:


🔴 Blocking issues

  1. WAV chunk bounds check is subtly wrong (LoadPcmWavAsFloatSamples, chunk-walking loop)
    The bounds guard chunk_start + chunk_size > file_size - (chunk_size % 2 == 1 ? 1 : 0) mixes the padding-byte adjustment into the main bounds check, which can undercount available space and let a crafted file slip through. The padding byte lives after the chunk data, so it shouldn't reduce the window used to validate the chunk itself. Suggested fix:

    // Before: chunk_start + chunk_size > file_size - (chunk_size % 2 == 1 ? 1 : 0)
    // After:
    if (chunk_start < 0 ||
        chunk_start + static_cast<std::streamoff>(chunk_size) > file_size) {
      FL_THROW(FOUNDRY_LOCAL_ERROR_INVALID_USAGE, "Corrupted WAV chunk.");
    }

    The odd-byte padding seek that follows handles alignment correctly as-is.

  2. Format validation happens after full data load (LoadPcmWavAsFloatSamples, data chunk branch)
    data.resize(chunk_size) allocates the full audio buffer before sample rate and format have been validated. For a large or malformed file, this could allocate hundreds of MB only to throw immediately afterward. Consider moving the sample_rate, audio_format, and bits_per_sample checks to right after the fmt chunk is parsed, before the data chunk is even encountered.


🟡 Suggestions

  1. Gap at language ID 5 in NemotronLanguageIdMap — IDs jump from 4 (zh-cn) to 6 (hi). If this is intentional (reserved/unsupported language), a brief comment would prevent future contributors from assuming it's a typo.

  2. prompt_tokens hardcoded to 0 in ProcessNemotronFileTranscription — the existing path computes this from the processor. If it's not meaningful for Nemotron, a // TODO explaining why would help maintain parity expectations with the OpenAI usage spec.

  3. decode_all_tokens lambda and thread-safety — the lambda captures generator by reference and is safe as used today (same-thread, same scope). A short comment noting it must not be moved off the creation thread would protect against future refactors.

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.

4 participants