Repository navigation
ao_avfoundation: stop requesting media while paused - #18390
aliceisjustplaying wants to merge 3 commits into
Conversation
Why should it do that?
Busylooping any time the player is paused would've resulted in far more noise about it, considering we haven't heard about this issue at all yet, is this a MacOS regression? |
| struct priv *p = ao->priv; | ||
|
|
||
| if (paused) { | ||
| [p->renderer stopRequestingMediaData]; |
There was a problem hiding this comment.
I know we use this property elsewhere in the AO too, but this is deprecated https://developer.apple.com/documentation/avfoundation/avsamplebufferaudiorenderer/stoprequestingmediadata()
It can't be the case that avoiding busylooping necessitates the use of a deprecated property
|
I also find it strange that there is no noise about it, but I tested this both on the macOS 27 beta but also on macOS 26 now and I can reproduce it, Updated the PR description. Looking into the deprecated property. |
|
@llyyr it's deprecated for swift but not objc, and as far as i can tell apple's website is just simply wrong about this Xcode 27 SDK, #if defined(__swift__)
API_DEPRECATED("Cancel the receiver's Task instead", macos(10.13, 27.0), ...)
#else
API_AVAILABLE(macos(10.13), ...)
#endif
|
The AVSampleBufferAudioRenderer callback remained registered when the synchronizer rate was set to zero. If its queue was empty when playback was paused, AVFoundation immediately invoked the callback again after ao_read_data returned no samples, consuming a full CPU core. Stop requesting media data before pausing and register the callback again when playback resumes. This balances the request/stop lifecycle and prevents the empty renderer from busy-looping.
4df03a9 to
4e616c3
Compare
|
I can reproduce this on macOS 27.0 arm64 with libmpv 0.41.0, and this PR fixes it. I hit it through Supersonic, a Subsonic client that bundles libmpv. It sat at 91-100% CPU while paused, and a My reproducer uses libmpv with
Pausing with audio still queued doesn't trigger it on either build. Neither does starting paused, seeking while paused, adding and removing an aggregate device while paused, or The harness is about 60 lines of C plus a small Python server. Happy to share it. Disclosure: I used an AI coding assistant for the diagnosis and the test harness. I ran the tests and checked the results myself. |
|
@GeiserX oh thank god it's not just my computer |
|
I have built mpv 0.41.0 patched with this PR and the problem still occurs for me. IINA has upgraded from mvp 0.38.0 to mpv 0.41.0 and what I believe is this problem has been reported in IINA issue iina/iina#6445. The problem is not reproducible with the older version of IINA that uses mpv 0.38.0, so this is a regression. To test the fix I'm using mpv-build to build mpv on my M1 running macOS 15.7.1 Sequoia with Xcode 26.3. I patched mpv 0.41.0 with this PR: git diff:I then did a full clean build of mpv: mpv --version:I started mpv playing a video with these options: I paused playback. Then in another terminal window I starting monitoring mpv's CPU usage by running this command: I then connected my AirPods Max. The monitoring of the CPU usage shows:
The terminal running mpv shows audio restarted due to a system notification: Terminal output:I ran Clip from Process Sample:Given the report from @GeiserX that the fix is working, I am straining my brain trying to figure out what I am doing wrong. Feels like somehow I'm not testing with the fix. Too tired to think at this point. If someone can approve the workflows for this PR to run I will test the mpv created by the CI build. |
|
@low-batt your test is right. The PR does not cover a device change while paused, and I reproduced your result. My earlier test paused with an empty renderer queue and measured CPU. It never changed the output device while paused, so it never went through Reproduced on macOS 27.0.1 arm64 with libmpv built from v0.41.0 plus this PR,
Same shape as your I also had it live. Supersonic bundles exactly this build (v0.41.0 plus this PR) and had been sitting paused at 100% CPU for 1480 CPU-minutes after a Bluetooth speaker reconnected. The change I tested skips the restart while paused. --- a/audio/out/ao_avfoundation.m
+++ b/audio/out/ao_avfoundation.m
@@ -15,6 +15,8 @@
* License along with mpv. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <stdatomic.h>
+
#include "ao.h"
#include "audio/format.h"
#include "audio/out/ao_coreaudio_chmap.h"
@@ -45,6 +47,7 @@ - (void)handleRestartNotification:(NSNotification*)notification;
CMAudioFormatDescriptionRef format_description;
AVObserver *observer;
int64_t end_time_av;
+ atomic_bool paused;
};
static int64_t CMTimeGetNanoseconds(CMTime time)
@@ -153,6 +156,7 @@ static void start(struct ao *ao)
{
struct priv *p = ao->priv;
+ atomic_store(&p->paused, false);
p->end_time_av = -1;
[p->synchronizer setRate:1];
request_media_data(ao);
@@ -173,6 +177,7 @@ static bool set_pause(struct ao *ao, bool paused)
{
struct priv *p = ao->priv;
+ atomic_store(&p->paused, paused);
if (paused) {
[p->renderer stopRequestingMediaData];
[p->synchronizer setRate:0];
@@ -219,8 +224,16 @@ - (void)handleRestartNotification:(NSNotification*)notification {
MP_WARN(ao, "restarting due to system notification; this will cause desync\n");
MP_VERBOSE(ao, "notification name: %s\n", name);
talloc_free(name);
+ struct priv *p = ao->priv;
stop(ao);
- start(ao);
+ if (atomic_load(&p->paused)) {
+ // Keep the renderer stopped; set_pause(false) requests media data
+ // again. The flushed buffers are gone, so enqueue from the current
+ // time on resume.
+ p->end_time_av = -1;
+ } else {
+ start(ao);
+ }
}
@endOne honest gap: a notification that lands in the middle of On why 0.38 did not have this: a3a9bc2 removed the @aliceisjustplaying fold this into the PR if you want, or I can open a follow-up PR, whichever is easier for you. The test program is about 250 lines of C that dlopens libmpv and keeps everything muted: Disclosure: I used an AI coding assistant for the diagnosis and the test program. I built both libraries and ran the tests myself. |
|
@GeiserX Excellent analysis! I built mpv 0.41.0 with the patch in your comment and repeated the test I have been running. When I connected my AirPods the CPU usage did not change. The revised fix works for me! |
|
@aliceisjustplaying I'm expecting the mpv developers would prefer one PR with a more comprehensive fix. Please do consider folding the patch in this comment from @GeiserX into this PR. |
Fold in the follow-up patch from GeiserX. A device-change notification previously called start() unconditionally, re-enabling media requests while the core was still paused. Keep the renderer stopped in that case and reset the enqueue timestamp for resume. This does not serialize notifications against set_pause(); the race noted with the posted patch remains. Patch-by: GeiserX Link: mpv-player#18390 (comment)
Run start, stop, pause and notification recovery on the renderer queue so a restart cannot overtake a pause. Track whether media requests are active, including reset and EOF, rather than only tracking pause. Enqueue notification recovery asynchronously to avoid a synchronous wait on the posting thread or a nested dispatch_sync on the renderer queue. Invalidate the observer AO pointer on the queue during teardown; queued recovery retains the observer and ignores an invalidated AO.
|
I have installed macOS 27.0.1 and Xcode 27.0 on my M1, built mpv from the current master: mpv --version:And confirmed the problem still reproduces with master. I then patched mpv with the latest commits from this PR. Rebuilt mpv and confirmed the test I've been running passes. The changes are working for me. @aliceisjustplaying @GeiserX Thank you very much for your excellent work on this problem. |
|
can you look over this @ruihe774 if you have some time, pls? |
|
not sure if a sync queue is the right approach here or we should rather use a mutex lock instead. |
Device changes while paused
Includes GeiserX's follow-up patch, which low-batt also verified with AirPods. The restart-notification handler now keeps the renderer stopped when playback is paused and resets the enqueue timestamp for resume, instead of unconditionally calling
start().The pause/notification race is addressed in 5582af753d by serializing start, stop, pause and notification recovery on the renderer queue. Recovery is asynchronous, and teardown invalidates the observer's AO pointer on that queue before freeing the AO. A queue-confined active flag prevents deferred recovery from restarting media requests after pause, reset or EOF.
Full macOS build and
git diff --checkpassed. An isolated harness using real AVFoundation reproduced the pause race on the previous PR commit; the updated implementation passed that case plus same-queue notifications, pause/resume, reset, EOF and teardown checks. Reset/EOF regressions also failed an intermediate implementation and passed the final fix. AddressSanitizer covered the changed AO and harness, not all dependencies. Notifications were synthetic; actual device switching, iOS/tvOS and seamless gapless transitions were not verified.Local verification for this update:
meson setup build -Dlibmpv=true -Dcplayer=true,meson compile -C buildandgit diff --checkpassed. The device-switch runtime test was not repeated locally; the runtime results linked above belong to GeiserX and low-batt. The original verification below predates this update.AI disclosure for these updates: GPT-6 Astra in Pi and GPT-6.1 Sol in Codex assisted with implementation, verification, review and this description.
Summary
Stop
AVSampleBufferAudioRendererfrom requesting media data while mpv is paused, then register the request callback again when playback resumes.Root cause
set_pauseset the render synchronizer rate to zero but left the media-data request active. If the renderer had no queued audio,ao_read_datareturned no samples while paused. AVFoundation still considered the renderer ready for more data and immediately invoked the callback again, creating a busy loop on theavfoundation eventqueue.The callback request now stops before the rate changes to zero. Resuming sets the rate back to one and registers a new callback, keeping the request and stop lifecycle balanced.
Observed failure
A paused mpv 0.41.0 process consistently consumed one full CPU core while its playback, decoder, demuxer, video-output, and Lua threads slept. A process sample showed the active thread in this stack:
The process-level signal stayed consistent across repeated measurements:
Verification
meson compile -C buildgit diff --check origin/master...HEADao=avfoundation, completed five pause/resume cycles, and left the player pausedAdditional macOS 26 runtime verification:
e7191f2a65) and PR commit (4df03a9b22) on macOS 26.6.2 (25G83), arm64paused_cpu=77.4; a three-second sample recorded 2,266 samples onDispatchQueue: avfoundation event, including 1,610 in-[AVMediaDataRequester _requestMediaDataIfReady]paused_cpu=0.1;requestMediaDataIfReadywas absent from the sample; resume and re-pause measured0.0%CPUThis build configuration defines no automated tests. The behavior depends on the macOS AVFoundation runtime, so the verification exercises the real audio output.
AI/LLM disclosure
OpenAI Codex with GPT-5.6 Sol (high) assisted with diagnosis, implementation, testing, and drafting this pull request. I understand the change and take full responsibility for it, including its licensing under the file's existing LGPLv2.1+ terms. I will participate in review with human-written responses.