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

ao_coreaudio: stop audio unit after idle timeout #13663

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

low-batt
Copy link
Contributor

@low-batt low-batt commented Mar 8, 2024

Commit 39f7f83 changed ao_driver.reset to use AudioUnitReset instead of AudioOutputUnitStop. The problem with calling AudioOutputUnitStop was that AudioOutputUnitStart takes a significant amount of time after a stop when a wireless audio device is being used. This resulted in lagging that was noticeable to users during seeking and short pause/resume cycles. Switching to AudioUnitReset eliminated this lagging.

However with the switch to AudioUnitReset the macOS daemon coreaudiod continued to consume CPU time and did not release a powerd assertion that it created on behalf of mpv, preventing macOS from sleeping.

This commit will change ao_coreaudio.reset to call AudioOutputUnitStop after a delay if playback has not resumed. This preserves the faster restart of playback for seeking and short pause/resume cycles and avoids preventing sleep and needless CPU consumption.

Fixes #11617

The code changes were authored by @orion1vi and @lhc70000.

@low-batt
Copy link
Contributor Author

low-batt commented Mar 8, 2024

This is the replacement pull request for PR #11667.

@Akemi If there is anything you don't like and want changed do not hesitate to ask. I am happy to update as needed.

I did not change the code at all other than to add a couple of comments. I am worried about the switch from synchronous to asynchronous in the reset and stop methods as I mentioned in the other PR. If these operations need to be synchronous, let me know and I will correct them.

audio/out/ao_coreaudio.c Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Mar 8, 2024

Download the artifacts for this pull request:

Windows
macOS

audio/out/ao_coreaudio.c Outdated Show resolved Hide resolved
@Akemi
Copy link
Member

Akemi commented Mar 10, 2024

besides my comment about keeping it in sync, i think this is good to go.

Commit 39f7f83 changed ao_driver.reset to use AudioUnitReset instead of
AudioOutputUnitStop. The problem with calling AudioOutputUnitStop was
that AudioOutputUnitStart takes a significant amount of time after a
stop when a wireless audio device is being used. This resulted in
lagging that was noticeable to users during seeking and short
pause/resume cycles. Switching to AudioUnitReset eliminated this
lagging.

However with the switch to AudioUnitReset the macOS daemon coreaudiod
continued to consume CPU time and did not release a powerd assertion
that it created on behalf of mpv, preventing macOS from sleeping.

This commit will change ao_coreaudio.reset to call AudioOutputUnitStop
after a delay if playback has not resumed. This preserves the faster
restart of playback for seeking and short pause/resume cycles and avoids
preventing sleep and needless CPU consumption.

Fixes mpv-player#11617

The code changes were authored by @orion1vi and @lhc70000.

Co-authored-by: Collider LI <lhc199652@gmail.com>
@low-batt
Copy link
Contributor Author

I have updated the commit to use dispatch_sync_f in reset and start.

I was very tempted to make that change when preparing the PR, but as I do not know the macOS audio APIs I was concerned this might be some sort of event driven interface and I would be undoing some sort of improvement.

@Akemi
Copy link
Member

Akemi commented Mar 16, 2024

thank you all @low-batt, @lhc70000 and @orion1vi, for the work on those changes and all the analysing you did on the related issues/PRs.

and sorry again i am taking so long with reviewing those changes and going through the accumulated issues and PRs.

@Akemi Akemi merged commit ab419a6 into mpv-player:master Mar 16, 2024
14 checks passed
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.

While paused mpv prevents sleep on Mac, consumes CPU
3 participants