summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2020-06-02 20:43:32 +0200
committerwm4 <wm4@nowhere>2020-06-02 20:43:49 +0200
commit68ade4e5a5e760bb5a4b793ebfa98ca386786605 (patch)
tree4c3c8fd8aa6d7a05d83cb790a7d9fc38102b5512
parent08b198aab49f73829bb02a25bc46bd159b6f9c6e (diff)
downloadmpv-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.c19
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);
}