Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error when p_start_time is negative when starting an audio playback stream #89499

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

garrettgu10
Copy link
Contributor

Fixes #89390.

I'm not sure if this is idiomatic/desired behavior, feel free to correct me :-)

@garrettgu10 garrettgu10 requested a review from a team as a code owner March 15, 2024 01:30
@garrettgu10 garrettgu10 changed the title Throw error when p_start_time is negative in start_playback_stream Throw error when p_start_time is negative when starting an audio playback stream Mar 15, 2024
@akien-mga akien-mga changed the title Throw error when p_start_time is negative when starting an audio playback stream Throw error when p_start_time is negative when starting an audio playback stream Mar 15, 2024
@akien-mga akien-mga added this to the 4.x milestone Mar 15, 2024
@akien-mga akien-mga added bug and removed enhancement labels Mar 15, 2024
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Mar 15, 2024
@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 15, 2024
@Malcolmnixon
Copy link
Contributor

All existing streams silently clamp any positions greater than the stream end/length; however they are not consistent as to how they handle positions less than the stream start (e.g. negative positions):

Type Results
MP3 Negative start or seek positions are passed to AudioStreamPlaybackMP3::seek which fails to clamp negative values resulting in extremely large positive values being passed to the minimp3 mp3dec_ex_seek function.
Ogg Vorbis Negative start or seek positions are passed to AudioStreamPlaybackOggVorbis::seek which fails to clamp negative values resulting in large positive frame values.
WAV Negative start or seek positions are passed to AudioStreamPlaybackWAV::seek which clamps them to 0.

If clamping occurs at the end, I feel it would be more consistent to apply silent clamping at the start as well. This may be beneficial for code performing relative seeking in a stream.

@akien-mga
Copy link
Member

@Malcolmnixon Clamping negative start/seek positions consistently as you suggest seems good to me. Would you like to make an alternative PR with that approach?

@KoBeWi KoBeWi modified the milestones: 4.3, 4.4 Aug 5, 2024
@KoBeWi KoBeWi added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:audio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AudioStreamPlayer can play sound from a negative position
4 participants