Skip to content

Audio stream synchronized refactoring and prevent stream sync from stopping when set #106608

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

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

adriano-sudario
Copy link

@adriano-sudario adriano-sudario commented May 19, 2025

I opened PR #100534 a while ago, but messed up after a rebase, so setted it as draft to be removed, but the description still applies to this one.

I think this PR still needs some discussion, but I believe it's important to take a look at that so AudioStreamSynchronized doesn't become a forgotten feature with no maintenance. Same applies to AudioStreamInteractive (to be discussed later), which I believe also needs some refactoring, but this PR doesn't cover nothing on that regard.

Besides the changes on the previous PR, this one also adds unit tests to make sure the refactoring didnt break anything.

Here's some points that needs to be addressed on the discussion:

  • Had to make some minor changes on audio_stream_wav after some behaviors caught up while writing the tests
    • has_loop method was not implemented
    • make sure offset is set to zero after begin_resample on start() if p_from_pos is zero, because it was starting with a few milliseconds ahead
  • Added get_sync_stream_playback method and exposed it for accessing each AudioStreamPlayback, so it can have more flexibility if someone needs to manipulate it manually. This one needs further discussion, but I believe it increases possibilities with AudioStreamSynchronized and gives more freedom to user. Maybe it can be left to another PR, but it'd have to refactor the tests if we decide to remove it.
  • The build pipeline it's breaking only on Linux, pointing some memory leak on test cases which checks playbacks behaviors. I still don't understand if it's a Linux only related problem or if windows and mac's pipeline build just doesn't detect the leaking. If detach_stream_sync is not called after test cases, the leaks are also caught as warning when running local. If some one can help with that, would be much appreciated.

That's it for this PR, please be welcome on joining discussion or just let me know if it doesn't make sense.

@adriano-sudario adriano-sudario requested review from a team as code owners May 19, 2025 20:08
@Calinou Calinou added the bug label May 20, 2025
@Calinou Calinou added topic:audio cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release labels May 20, 2025
@Calinou Calinou added this to the 4.5 milestone May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release topic:audio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AudioStreamSynchronized stops playback if 'set_sync_stream' is called while playing
2 participants