Skip to content

starboard: Fix integer overflow in AudioTrack sink#11549

Open
xiaomings wants to merge 1 commit into
youtube:mainfrom
xiaomings:audiosink_fix
Open

starboard: Fix integer overflow in AudioTrack sink#11549
xiaomings wants to merge 1 commit into
youtube:mainfrom
xiaomings:audiosink_fix

Conversation

@xiaomings

@xiaomings xiaomings commented Jul 25, 2026

Copy link
Copy Markdown
Member

Update accumulated_written_frames, last_playback_head_position, and
playback_head_position to use 64-bit integers in AudioTrackAudioSink.
At a 48kHz sampling rate, 32-bit signed integers overflow after
approximately 12.4 hours of continuous playback, which can cause
incorrect playback head reporting and audio sync issues.

Also update GetFramesDurationUs to accept int64_t to maintain type
consistency and prevent narrowing.

Note that this doesn't resolve all integer size issues across the audio
pipeline as the Java AudioTrackBridge is still tracking the playback
position using 32 bits integer, which will be resolve in b/539224216.

Bug: 539224216

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Suggested Commit Message


starboard: Fix integer overflow in AudioTrack sink

Update accumulated_written_frames, last_playback_head_position, and
playback_head_position to use 64-bit integers in AudioTrackAudioSink.
At a 48kHz sampling rate, 32-bit signed integers overflow after
approximately 12.4 hours of continuous playback, which can cause
incorrect playback head reporting and audio sync issues.

Also update GetFramesDurationUs to accept int64_t to maintain type
consistency and prevent narrowing.

Bug: 511356295

💡 Pro Tips for a Better Commit Message:

  1. Influence the Result: Want to change the output? You can write custom prompts or instructions directly in the Pull Request description. The model uses that text to generate the message.
  2. Re-run the Generator: Post a comment with: /generate-commit-message

Fix potential overflow and narrowing issues in AudioTrackAudioSink.
Change accumulated_written_frames, last_playback_head_position,
and playback_head_position to int64_t to prevent overflow after
long playback times (~12.4 hours at 48kHz).
Update GetFramesDurationUs to accept int64_t.

Bug: 511356295
TAG=agy
CONV=0d3a836a-08df-4860-a867-0f6affc85b25
@xiaomings xiaomings changed the title starboard: Fix unsafe integer in AudioTrack sink starboard: Fix integer overflow in AudioTrack sink Jul 25, 2026
@xiaomings

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates frame-related variables and function parameters in AudioTrackAudioSink from int to int64_t to prevent potential integer overflow issues when handling large frame counts. Specifically, accumulated_written_frames, last_playback_head_position, playback_head_position, and the parameter for GetFramesDurationUs in audio_track_audio_sink_type.cc and audio_track_audio_sink_type.h have been updated to 64-bit integers. There are no review comments, so I have no feedback to provide.

@xiaomings
xiaomings marked this pull request as ready for review July 26, 2026 20:38

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the audio frame tracking variables (accumulated_written_frames, last_playback_head_position, and playback_head_position) and the helper function GetFramesDurationUs in AudioTrackAudioSink to use 64-bit integers (int64_t) instead of 32-bit integers (int). This prevents potential integer overflow issues during long-running audio playback. There are no review comments to address, and the changes look correct.

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.

1 participant