Skip to content

Conversation

@Daniel-Chin
Copy link

Originally off by 1000

Originally off by 1000
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment on lines 6 to +9
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

Choose a reason for hiding this comment

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

P1 Badge Update tests for corrected PCM duration

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 👍 / 👎.

@seratch
Copy link
Member

seratch commented Nov 15, 2025

Thanks for sending this patch! However, #2059 already suggested this change!

@seratch seratch closed this Nov 15, 2025
@Daniel-Chin
Copy link
Author

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. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants