fix(sarvam): map wav output codec to audio/pcm to prevent missing RIFF error (#5267) - #7248
fix(sarvam): map wav output codec to audio/pcm to prevent missing RIFF error (#5267)#7248rrfunde wants to merge 5 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17fc98b733
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _CODEC_TO_MIME: dict[str, str] = { | ||
| "mp3": "audio/mp3", | ||
| "wav": "audio/wav", | ||
| "wav": "audio/pcm", |
There was a problem hiding this comment.
When synthesize() requests output_audio_codec="wav", Sarvam's REST audios entries can be complete RIFF/WAVE files; this shared mapping is also used by ChunkedStream._run, so AudioEmitter now interprets the RIFF header and metadata as PCM samples instead of stripping them and honoring the container format. That corrupts the start of otherwise valid REST audio. Apply the raw-PCM handling only to the WebSocket path that returns headerless chunks, or detect and preserve RIFF responses.
Useful? React with 👍 / 👎.
…treaming WebSocket
|
Addressed feedback:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e83c3b6e74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._opts.output_audio_codec == "wav" | ||
| and audio_bytes.startswith(b"RIFF") | ||
| and len(audio_bytes) >= 44 | ||
| ): | ||
| audio_bytes = audio_bytes[44:] |
There was a problem hiding this comment.
When the WebSocket returns a valid RIFF/WAVE container whose data chunk is not at byte 44—for example, one with an extended fmt chunk or LIST/JUNK metadata—_handle_audio_message slices at the hard-coded offset and sends the remaining header bytes to the raw-PCM emitter, corrupting the start of the audio and its sample accounting. Locate the data chunk by parsing the WAV container, or pass headered responses through the WAV decoder, instead of assuming every valid WAV has a 44-byte header.
Useful? React with 👍 / 👎.
…of fixed 44-byte slice
|
Addressed review feedback: implemented |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d560dae4b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return audio_bytes[pos:end] if end <= len(audio_bytes) else audio_bytes[pos:] | ||
| pos += chunk_size + (chunk_size % 2) | ||
|
|
||
| return audio_bytes[44:] if len(audio_bytes) >= 44 else audio_bytes |
There was a problem hiding this comment.
When a RIFF/WAVE response is split so that the first WebSocket audio message ends before the data chunk header (for example, midway through an extended fmt or metadata chunk), this fallback returns either the entire partial header or an arbitrary slice as PCM; subsequent messages no longer start with RIFF and are also treated as raw PCM, corrupting the beginning of the audio. The fresh code replaces the fixed-offset assumption with a parser, but that parser remains stateless across _handle_audio_message calls; buffer the header until data is found or use an incremental WAV decoder before switching to PCM.
Useful? React with 👍 / 👎.
|
Addressed review feedback: preserved WAV parser state across WebSocket chunks in |
| if found: | ||
| self._wav_header_parsed = True | ||
| pcm = bytes(self._wav_header_buf[offset:]) | ||
| self._wav_header_buf.clear() | ||
| return pcm |
There was a problem hiding this comment.
🟡 Trailing metadata corrupts streamed audio
When trailing WAV metadata contains RIFF, _extract_streaming_wav_pcm treats it as another container. Metadata and later headers then enter PCM output.
Learn more
The WAV data chunk can be followed by arbitrary RIFF chunks such as LIST or JUNK. Their payloads are not constrained from containing the four bytes RIFF. Searching all trailing bytes therefore cannot distinguish metadata content from the next container boundary. A false match can switch parsing to metadata and eventually make _wav_is_riff false, after which every later byte is emitted unchanged.
Example: A LIST title containing RIFF Studio follows the PCM data. The parser starts a supposed container at that title, emits part of the metadata as PCM, and passes subsequent WAV headers through as audio.
Recommended fix: Track the outer RIFF chunk size and current container position. Skip all chunks through the exact container boundary, then recognize a consecutive container only when RIFF starts at that boundary. Apply the same boundary handling in both trailing-data branches.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6f49e2a88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self._wav_is_riff is None: | ||
| if not audio_bytes.startswith(b"RIFF"): | ||
| self._wav_is_riff = False | ||
| self._wav_header_parsed = True | ||
| return audio_bytes |
There was a problem hiding this comment.
When a RIFF response is split within its first four bytes, such as an initial WebSocket audio message containing only RI, this branch permanently sets _wav_is_riff to false and emits that prefix plus every subsequent header byte as PCM. The new stateful parser therefore still corrupts headers for this split case; buffer up to enough bytes to distinguish the RIFF signature before committing to the raw-PCM path.
Useful? React with 👍 / 👎.
| if found: | ||
| self._wav_header_parsed = True | ||
| pcm = bytes(self._wav_header_buf[offset:]) | ||
| self._wav_header_buf.clear() |
There was a problem hiding this comment.
When a valid RIFF/WAVE container has a chunk after data (for example LIST, cue , or other metadata), this copies everything after the data header rather than only the declared sample payload, so the trailing chunk header and contents are emitted as PCM and produce audible garbage and incorrect duration accounting. Preserve the parsed data chunk size across messages and stop emitting when that many bytes have been consumed.
Useful? React with 👍 / 👎.
|
Addressed review feedback:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50cc63ba64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| riff_idx = audio_bytes.find(b"RIFF") | ||
| if riff_idx != -1: | ||
| self._wav_data_remaining = None | ||
| return self._extract_streaming_wav_pcm(audio_bytes[riff_idx:]) |
There was a problem hiding this comment.
When a valid WAV has a trailing LIST/JUNK payload containing the byte sequence RIFF, this unrestricted search treats that metadata content as the start of another container. Once the following bytes fail the WAVE check, _extract_streaming_wav_pcm switches permanently to raw-PCM mode and emits the metadata from that false match, producing audible corruption; the same search is also used when trailing bytes share the message containing the end of data. Track the outer RIFF chunk length or parse trailing chunks instead of scanning arbitrary metadata for the signature.
Useful? React with 👍 / 👎.
1 participant
Summary
When using
output_audio_codec="wav", the Sarvam TTS API returns raw PCM bytes rather than a RIFF/WAVE container file.Mapping
"wav"to"audio/pcm"in_CODEC_TO_MIMEallowsAudioEmitterto ingest the raw PCM directly without failing withValueError: Invalid WAV file: missing RIFF/WAVE.Fixes #5267