From b109d20ef745415d8d39af292062aa89a3c94fd8 Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 5 Sep 2014 22:21:06 +0200 Subject: [PATCH] audio/out: make EOF handling properly event-based With --gapless-audio=no, changing from one file to the next apparently made it hang, until the player was woken up by unrelated events like input. The reason was that the AO doesn't notify the player of EOF properly. the played was querying ao_eof_reached(), and then just went to sleep, without anything waking it up. Make it event-based: the AO wakes up the playloop if the EOF state changes. We could have fixed this in a simpler way by synchronously draining the AO in these cases. But I think proper event handling is preferable. Fixes: #1069 CC: @mpv-player/stable (perhaps) --- audio/out/ao.c | 2 +- audio/out/internal.h | 2 ++ audio/out/pull.c | 9 +++++++++ audio/out/push.c | 45 +++++++++++++++++++++++++++++++++++++++----- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/audio/out/ao.c b/audio/out/ao.c index 8e1ceb4bf1..429028c10d 100644 --- a/audio/out/ao.c +++ b/audio/out/ao.c @@ -321,7 +321,7 @@ void ao_drain(struct ao *ao) bool ao_eof_reached(struct ao *ao) { - return ao_get_delay(ao) < AO_EOF_DELAY; + return ao->api->get_eof ? ao->api->get_eof(ao) : true; } bool ao_chmap_sel_adjust(struct ao *ao, const struct mp_chmap_sel *s, diff --git a/audio/out/internal.h b/audio/out/internal.h index 75f5798bff..743f2fbb4f 100644 --- a/audio/out/internal.h +++ b/audio/out/internal.h @@ -134,6 +134,8 @@ struct ao_driver { float (*get_delay)(struct ao *ao); // push based: block until all queued audio is played (optional) void (*drain)(struct ao *ao); + // Optional. Return true if audio has stopped in any way. + bool (*get_eof)(struct ao *ao); // Wait until the audio buffer needs to be refilled. The lock is the // internal mutex usually protecting the internal AO state (and used to // protect driver calls), and must be temporarily unlocked while waiting. diff --git a/audio/out/pull.c b/audio/out/pull.c index eb77be81a2..9376ea4232 100644 --- a/audio/out/pull.c +++ b/audio/out/pull.c @@ -201,6 +201,14 @@ static void drain(struct ao *ao) reset(ao); } +static bool get_eof(struct ao *ao) +{ + struct ao_pull_state *p = ao->api_priv; + // For simplicity, ignore the latency. Otherwise, we would have to run an + // extra thread to time it. + return mp_ring_buffered(p->buffers[0]) == 0; +} + static void uninit(struct ao *ao) { ao->driver->uninit(ao); @@ -225,6 +233,7 @@ const struct ao_driver ao_api_pull = { .get_space = get_space, .play = play, .get_delay = get_delay, + .get_eof = get_eof, .pause = pause, .resume = resume, .priv_size = sizeof(struct ao_pull_state), diff --git a/audio/out/push.c b/audio/out/push.c index 3f0d453153..0dae14ceb0 100644 --- a/audio/out/push.c +++ b/audio/out/push.c @@ -54,6 +54,7 @@ struct ao_push_state { bool drain; bool buffers_full; bool avoid_ao_wait; + bool still_playing; bool need_wakeup; bool requested_data; bool paused; @@ -87,15 +88,13 @@ static int control(struct ao *ao, enum aocontrol cmd, void *arg) return r; } -static float get_delay(struct ao *ao) +static double unlocked_get_delay(struct ao *ao) { struct ao_push_state *p = ao->api_priv; - pthread_mutex_lock(&p->lock); double driver_delay = 0; if (ao->driver->get_delay) driver_delay = ao->driver->get_delay(ao); double delay = driver_delay + mp_audio_buffer_seconds(p->buffer); - pthread_mutex_unlock(&p->lock); if (delay >= AO_EOF_DELAY && p->expected_end_time) { if (mp_time_sec() > p->expected_end_time) { MP_ERR(ao, "Audio device EOF reporting is broken!\n"); @@ -106,6 +105,15 @@ static float get_delay(struct ao *ao) return delay; } +static float get_delay(struct ao *ao) +{ + struct ao_push_state *p = ao->api_priv; + pthread_mutex_lock(&p->lock); + float delay = unlocked_get_delay(ao); + pthread_mutex_unlock(&p->lock); + return delay; +} + static void reset(struct ao *ao) { struct ao_push_state *p = ao->api_priv; @@ -114,6 +122,7 @@ static void reset(struct ao *ao) ao->driver->reset(ao); mp_audio_buffer_clear(p->buffer); p->paused = false; + p->still_playing = false; wakeup_playthread(ao); pthread_mutex_unlock(&p->lock); } @@ -190,6 +199,15 @@ static int get_space(struct ao *ao) return space; } +static bool get_eof(struct ao *ao) +{ + struct ao_push_state *p = ao->api_priv; + pthread_mutex_lock(&p->lock); + bool eof = !p->still_playing; + pthread_mutex_unlock(&p->lock); + return eof; +} + static int play(struct ao *ao, void **data, int samples, int flags) { struct ao_push_state *p = ao->api_priv; @@ -215,6 +233,7 @@ static int play(struct ao *ao, void **data, int samples, int flags) p->expected_end_time = 0; p->final_chunk = is_final; p->paused = false; + p->still_playing |= write_samples > 0; // If we don't have new data, the decoder thread basically promises it // will send new data as soon as it's available. @@ -266,6 +285,7 @@ static void ao_play_data(struct ao *ao) // any new data (due to rounding to period boundaries). p->buffers_full = max >= space && r <= 0; p->avoid_ao_wait = (max == 0 && space > 0) || p->paused || stuck; + p->still_playing |= r > 0; MP_TRACE(ao, "in=%d, space=%d r=%d flags=%d aw=%d full=%d f=%d\n", max, space, r, flags, p->avoid_ao_wait, p->buffers_full, p->final_chunk); } @@ -319,9 +339,23 @@ static void *playthread(void *arg) // The most important part is that the decoder is woken up, so // that the decoder will wake up us in turn. MP_TRACE(ao, "buffer inactive.\n"); - if (!p->requested_data) + + bool was_playing = p->still_playing; + double timeout = -1; + if (p->still_playing && !p->paused) { + timeout = unlocked_get_delay(ao); + if (timeout < AO_EOF_DELAY) + p->still_playing = false; + } + + if (!p->requested_data || (was_playing && !p->still_playing)) mp_input_wakeup(ao->input_ctx); - pthread_cond_wait(&p->wakeup, &p->lock); + + if (p->still_playing && timeout > 0) { + mpthread_cond_timedwait_rel(&p->wakeup, &p->lock, timeout); + } else { + pthread_cond_wait(&p->wakeup, &p->lock); + } } else { if (!ao->driver->wait || ao->driver->wait(ao, &p->lock) < 0) { // Fallback to guessing. @@ -390,6 +424,7 @@ const struct ao_driver ao_api_push = { .pause = audio_pause, .resume = resume, .drain = drain, + .get_eof = get_eof, .priv_size = sizeof(struct ao_push_state), };