diff options
author | wm4 <wm4@nowhere> | 2020-06-02 20:43:32 +0200 |
---|---|---|
committer | wm4 <wm4@nowhere> | 2020-06-02 20:43:49 +0200 |
commit | 68ade4e5a5e760bb5a4b793ebfa98ca386786605 (patch) | |
tree | 4c3c8fd8aa6d7a05d83cb790a7d9fc38102b5512 | |
parent | 08b198aab49f73829bb02a25bc46bd159b6f9c6e (diff) | |
download | mpv-68ade4e5a5e760bb5a4b793ebfa98ca386786605.tar.bz2 mpv-68ade4e5a5e760bb5a4b793ebfa98ca386786605.tar.xz |
audio: avoid possible deadlock regression for some AOs
It's conceivable that ao->driver->reset() will make the audio API wait
for ao_read_data() (i.e. its audio callback) to return. Since we
recently moved the reset() call inside the same lock that ao_read_data()
acquires, this could deadlock. Whether this really happens depends on
how exactly the AO behaves. For example, ao_wasapi does not have this
problem. "Push" AOs are not affected either.
Fix by moving it outside of the lock. Assume ao->driver->start() will
not have this problem.
Could affect ao_sdl, ao_coreaudio (and similar rotten fruit AOs). I'm
unsure whether anyone experienced the problem in practice.
-rw-r--r-- | audio/out/buffer.c | 19 |
1 files changed, 17 insertions, 2 deletions
diff --git a/audio/out/buffer.c b/audio/out/buffer.c index da0d69f0de..a274fc9f7c 100644 --- a/audio/out/buffer.c +++ b/audio/out/buffer.c @@ -327,6 +327,7 @@ void ao_reset(struct ao *ao) { struct buffer_state *p = ao->buffer_state; bool wakeup = false; + bool do_reset = false; pthread_mutex_lock(&p->lock); @@ -334,7 +335,13 @@ void ao_reset(struct ao *ao) mp_ring_reset(p->buffers[n]); if (!ao->stream_silence && ao->driver->reset) { - ao->driver->reset(ao); // assumes the audio callback thread is stopped + if (ao->driver->write) { + ao->driver->reset(ao); + } else { + // Pull AOs may wait for ao_read_data() to return. + // That would deadlock if called from within the lock. + do_reset = true; + } p->streaming = false; } p->paused = false; @@ -350,6 +357,9 @@ void ao_reset(struct ao *ao) pthread_mutex_unlock(&p->lock); + if (do_reset) + ao->driver->reset(ao); + if (wakeup) ao_wakeup_playthread(ao); } @@ -358,6 +368,7 @@ void ao_pause(struct ao *ao) { struct buffer_state *p = ao->buffer_state; bool wakeup = false; + bool do_reset = false; pthread_mutex_lock(&p->lock); @@ -373,7 +384,8 @@ void ao_pause(struct ao *ao) p->streaming = false; } } else if (ao->driver->reset) { - ao->driver->reset(ao); + // See ao_reset() why this is done outside of the lock. + do_reset = true; p->streaming = false; } } @@ -383,6 +395,9 @@ void ao_pause(struct ao *ao) pthread_mutex_unlock(&p->lock); + if (do_reset) + ao->driver->reset(ao); + if (wakeup) ao_wakeup_playthread(ao); } |