Skip to content

fix(voice): honor sample width in AudioInput.to_base64 - #4364

Closed
abhay-codes07 wants to merge 1 commit into
openai:mainfrom
abhay-codes07:fix/voice-base64-sample-width
Closed

fix(voice): honor sample width in AudioInput.to_base64#4364
abhay-codes07 wants to merge 1 commit into
openai:mainfrom
abhay-codes07:fix/voice-base64-sample-width

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

Summary

#4361 made to_audio_file respect AudioInput.sample_width, but to_base64 on the same class still hardcodes a PCM16 conversion. One object with one declared width now produces two different encodings:

sample_width to_audio_file() frames to_base64() bytes
1 8 16
2 16 16
3 24 16
4 32 16

(5-sample buffer.)

Why it matters

to_base64 is what feeds the transcription span input when trace_include_sensitive_audio_data is enabled:

input=input.to_base64() if trace_include_sensitive_audio_data else "",

That span also records input_format = "pcm". So for an input declared as 1, 3 or 4 bytes per sample, the traced audio is 16-bit data labelled as the session's PCM format, and cannot be played back correctly from the trace. #4361 validated sample_width into {1, 2, 3, 4} and made those widths a supported, exercised configuration, which is what turns this from a latent inconsistency into a reachable one.

Fix

The width conversion is extracted from _buffer_to_audio_file into _buffer_to_pcm_bytes and shared by both paths, so they cannot drift again. No conversion logic changed; it only moved.

Preserved exactly:

  • the 16-bit default is byte for byte what it was before, asserted against the previous expression
  • the caller's buffer is still never mutated (the existing regression test for that still passes)
  • an unsupported dtype or sample width still raises UserError, now from the shared encoder instead of a duplicated check

Test plan

New TestAudioInputBase64SampleWidth in tests/voice/test_input.py:

  • test_base64_matches_the_wav_frames_for_every_width, parametrized over widths 1-4 and both int16 and float32, asserting the base64 bytes are byte-identical to the WAV frames for the same object. 7 of these fail on main.
  • test_default_width_encoding_is_unchanged, pinning the 16-bit output against the old expression; passes both with and without the change.
  • test_invalid_sample_width_is_rejected, which also fails on main because to_base64 never validated the width at all.
Command Result
make format / make lint clean
make mypy / make pyright 0 issues in the touched files
uv run pytest tests/voice/ 131 passed
make tests 7314 passed

Run on Windows against a main baseline captured at the same commit. One test differed, tests/test_trace_processor.py::test_backend_span_exporter_deadline_stops_during_5xx_retry_backoff; it is a wall-clock deadline test that passes 4/4 in isolation on this branch and 6/6 on unmodified main, and only fails under -n 6 load.

Issue number

None. Found while auditing #4361.

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

The verification script shells out to make; I ran the underlying steps individually, with the results above.

openai#4361 made `to_audio_file` respect `AudioInput.sample_width`, but
`to_base64` still hardcoded a PCM16 conversion. The same object with the
same declared width therefore produced two different encodings:

    AudioInput(buffer=..., sample_width=1)
      to_audio_file() -> 8 bytes  (8-bit frames, header says 8-bit)
      to_base64()     -> 16 bytes (16-bit samples)

`to_base64` feeds the transcription span input when
`trace_include_sensitive_audio_data` is enabled, and that span records
`input_format = "pcm"`, so a 1, 3 or 4 byte input was traced as 16-bit
PCM and could not be played back correctly from the trace.

The width conversion is extracted from `_buffer_to_audio_file` into
`_buffer_to_pcm_bytes` and shared, so the two paths cannot drift again.
The 16-bit default is byte for byte what it was before, the caller's
buffer is still never mutated, and an unsupported dtype or sample width
still raises `UserError`, now from the shared encoder rather than a
duplicated check.
Copilot AI lite review requested due to automatic review settings August 11, 2026 16:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@seratch

seratch commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. I traced AudioInput.to_base64() through both transcription paths. Equality with WAV frames is not the current trace contract: the released method normalizes samples to PCM16, and the streamed path does the same while both spans export only format: "pcm". Changing static input to 8-, 24-, or 32-bit bytes would make the paths diverge, and the span carries no sample-width, sample-rate, or channel metadata that could make those bytes self-describing. The actual transcription request already uses to_audio_file(), so #4361's WAV fix remains intact.

I am going to close this PR. If there is a reproducible Traces dashboard failure for a supported non-default AudioInput, please open an issue with the exact settings and observed playback behavior so we can address the trace payload contract directly.

@seratch seratch closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants