Conversation
Originally off by 1000
![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.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| def calculate_audio_length_ms(format: RealtimeAudioFormat | None, audio_bytes: bytes) -> float: | ||
| if format and isinstance(format, str) and format.startswith("g711"): | ||
| return (len(audio_bytes) / 8000) * 1000 | ||
| return (len(audio_bytes) / 24 / 2) * 1000 | ||
| return (len(audio_bytes) / 24000 / 2) * 1000 |
There was a problem hiding this comment.
Switching the PCM branch to divide by 24000 instead of 24 fixes the sample‑rate bug but also changes every computed duration by 1/1000. The existing tests still assert values based on the old 24 constant (e.g., tests/realtime/test_openai_realtime.py::test_calculate_audio_length_ms_pure_function expects 48 bytes to equal 1000 ms, and tests/realtime/test_playback_tracker.py uses the same assumption), so the suite will now fail and callers relying on those expectations may still treat 48 bytes as one second. The tests and any hard‑coded comments need to be updated to use 24 kHz math (len(bytes) / (24000 * 2) * 1000) to keep the build green and ensure downstream logic matches the corrected behavior.
Useful? React with 👍 / 👎.
|
Thanks for sending this patch! However, #2059 already suggested this change! |
Thanks, that's good. This is so small I didn't check for duplication. ;-) |
2 participants
Originally off by 1000