From 08b198aab49f73829bb02a25bc46bd159b6f9c6e Mon Sep 17 00:00:00 2001 From: wm4 Date: Tue, 2 Jun 2020 20:30:59 +0200 Subject: audio: further simplify internal audio API somewhat Instead of the relatively subtle underflow handling, simply signal whether the stream is in a playing state. Should make it more robust. Should affect ao_alsa and ao_pulse only (and ao_openal, but it's broken). For ao_pulse, I'm just guessing. How the hell do you query whether a stream is playing? Who knows. Seems to work, judging from very superficial testing. --- audio/out/ao_alsa.c | 16 +++++++--------- audio/out/ao_null.c | 9 ++------- audio/out/ao_pulse.c | 19 ++++++++++++------- audio/out/buffer.c | 32 ++++++++++++-------------------- audio/out/internal.h | 12 ++++++++---- 5 files changed, 41 insertions(+), 47 deletions(-) diff --git a/audio/out/ao_alsa.c b/audio/out/ao_alsa.c index 0f375133e1..0380162eb4 100644 --- a/audio/out/ao_alsa.c +++ b/audio/out/ao_alsa.c @@ -91,7 +91,6 @@ static const struct m_sub_options ao_alsa_conf = { struct priv { snd_pcm_t *alsa; bool device_lost; - bool underrun; snd_pcm_format_t alsa_fmt; bool can_pause; snd_pcm_uframes_t buffersize; @@ -916,8 +915,8 @@ static int init(struct ao *ao) } // Function for dealing with playback state. This attempts to recover the ALSA -// state (bring it into SND_PCM_STATE_{PREPARED,RUNNING,PAUSED}). If state!=NULL, -// fill it after recovery. +// state (bring it into SND_PCM_STATE_{PREPARED,RUNNING,PAUSED,UNDERRUN}). If +// state!=NULL, fill it after recovery. // Returns true if PCM is in one the expected states. static bool recover_and_get_state(struct ao *ao, struct mp_pcm_state *state) { @@ -928,6 +927,7 @@ static bool recover_and_get_state(struct ao *ao, struct mp_pcm_state *state) snd_pcm_status_alloca(&st); bool state_ok = false; + snd_pcm_state_t pcmst = SND_PCM_STATE_DISCONNECTED; // Give it a number of chances to recover. This tries to deal with the fact // that the API is asynchronous, and to account for some past cargo-cult @@ -936,7 +936,7 @@ static bool recover_and_get_state(struct ao *ao, struct mp_pcm_state *state) err = snd_pcm_status(p->alsa, st); CHECK_ALSA_ERROR("snd_pcm_status"); - snd_pcm_state_t pcmst = snd_pcm_status_get_state(st); + pcmst = snd_pcm_status_get_state(st); if (pcmst == SND_PCM_STATE_PREPARED || pcmst == SND_PCM_STATE_RUNNING || pcmst == SND_PCM_STATE_PAUSED) @@ -949,10 +949,9 @@ static bool recover_and_get_state(struct ao *ao, struct mp_pcm_state *state) n + 1, snd_pcm_state_name(pcmst)); switch (pcmst) { - // Underrun; note and recover. We never use draining, + // Underrun; recover. (We never use draining.) case SND_PCM_STATE_XRUN: case SND_PCM_STATE_DRAINING: - p->underrun = true; err = snd_pcm_prepare(p->alsa); CHECK_ALSA_ERROR("pcm prepare error"); continue; @@ -1000,7 +999,8 @@ static bool recover_and_get_state(struct ao *ao, struct mp_pcm_state *state) state->free_samples = snd_pcm_status_get_avail(st); state->free_samples = MPCLAMP(state->free_samples, 0, ao->device_buffer); state->queued_samples = ao->device_buffer - state->free_samples; - state->underrun = p->underrun; + state->playing = pcmst == SND_PCM_STATE_RUNNING || + pcmst == SND_PCM_STATE_PAUSED; } return true; @@ -1011,9 +1011,7 @@ alsa_error: static void audio_get_state(struct ao *ao, struct mp_pcm_state *state) { - struct priv *p = ao->priv; recover_and_get_state(ao, state); - p->underrun = false; } static void audio_start(struct ao *ao) diff --git a/audio/out/ao_null.c b/audio/out/ao_null.c index 107dc7e35a..ac113ca186 100644 --- a/audio/out/ao_null.c +++ b/audio/out/ao_null.c @@ -43,7 +43,6 @@ struct priv { float buffered; // samples int buffersize; // samples bool playing; - bool underrun; int untimed; float bufferlen; // seconds @@ -77,10 +76,8 @@ static void drain(struct ao *ao) double now = mp_time_sec(); if (priv->buffered > 0) { priv->buffered -= (now - priv->last_time) * ao->samplerate * priv->speed; - if (priv->buffered < 0) { - priv->underrun = true; + if (priv->buffered < 0) priv->buffered = 0; - } } priv->last_time = now; } @@ -127,7 +124,6 @@ static void reset(struct ao *ao) { struct priv *priv = ao->priv; priv->buffered = 0; - priv->underrun = false; priv->playing = false; } @@ -200,8 +196,7 @@ static void get_state(struct ao *ao, struct mp_pcm_state *state) state->delay = (int)(state->delay / q) * q; } - state->underrun = priv->underrun; - priv->underrun = false; + state->playing = priv->playing && priv->buffered > 0; } #define OPT_BASE_STRUCT struct priv diff --git a/audio/out/ao_pulse.c b/audio/out/ao_pulse.c index 563848b523..c826a6c993 100644 --- a/audio/out/ao_pulse.c +++ b/audio/out/ao_pulse.c @@ -52,7 +52,7 @@ struct priv { struct pa_sink_input_info pi; int retval; - bool underrun; + bool playing; char *cfg_host; int cfg_buffer; @@ -134,7 +134,7 @@ static void underflow_cb(pa_stream *s, void *userdata) { struct ao *ao = userdata; struct priv *priv = ao->priv; - priv->underrun = true; + priv->playing = false; ao_wakeup_playthread(ao); pa_threaded_mainloop_signal(priv->mainloop, 0); } @@ -486,9 +486,15 @@ static void cork(struct ao *ao, bool pause) struct priv *priv = ao->priv; pa_threaded_mainloop_lock(priv->mainloop); priv->retval = 0; - if (!waitop(priv, pa_stream_cork(priv->stream, pause, success_cb, ao)) || - !priv->retval) + if (waitop_no_unlock(priv, pa_stream_cork(priv->stream, pause, success_cb, ao)) + && priv->retval) + { + priv->playing = true; + } else { GENERIC_ERR_MSG("pa_stream_cork() failed"); + priv->playing = false; + } + pa_threaded_mainloop_unlock(priv->mainloop); } // Play the specified data to the pulseaudio server @@ -614,14 +620,13 @@ static void audio_get_state(struct ao *ao, struct mp_pcm_state *state) state->delay = get_delay_pulse(ao); } - state->underrun = priv->underrun; - priv->underrun = false; + state->playing = priv->playing; pa_threaded_mainloop_unlock(priv->mainloop); // Otherwise, PA will keep hammering us for underruns (which it does instead // of stopping the stream automatically). - if (state->underrun) + if (!state->playing) cork(ao, true); } diff --git a/audio/out/buffer.c b/audio/out/buffer.c index 0a13089de9..da0d69f0de 100644 --- a/audio/out/buffer.c +++ b/audio/out/buffer.c @@ -71,7 +71,6 @@ struct buffer_state { bool hw_paused; // driver->set_pause() was used successfully bool recover_pause; // non-hw_paused: needs to recover delay bool draining; - bool had_underrun; bool ao_wait_low_buffer; struct mp_pcm_state prepause_state; pthread_t thread; // thread shoveling data to AO @@ -108,20 +107,8 @@ static void get_dev_state(struct ao *ao, struct mp_pcm_state *state) .free_samples = -1, .queued_samples = -1, .delay = -1, - .underrun = false, }; ao->driver->get_state(ao, state); - - if (state->underrun) { - p->had_underrun = true; - if (p->draining) { - MP_VERBOSE(ao, "underrun signaled for audio end\n"); - p->still_playing = false; - pthread_cond_broadcast(&p->wakeup); - } else { - ao_add_events(ao, AO_EVENT_UNDERRUN); - } - } } static int unlocked_get_space(struct ao *ao) @@ -455,7 +442,7 @@ void ao_drain(struct ao *ao) pthread_mutex_lock(&p->lock); p->final_chunk = true; - while (!p->paused && p->still_playing && !p->had_underrun) { + while (!p->paused && p->still_playing && p->streaming) { if (ao->driver->write) { if (p->draining) { // Wait for EOF signal from AO. @@ -586,16 +573,21 @@ static bool realloc_buf(struct ao *ao, int samples) static void ao_play_data(struct ao *ao) { struct buffer_state *p = ao->buffer_state; + struct mp_pcm_state state; + get_dev_state(ao, &state); + + if (p->streaming && !state.playing && !ao->untimed) { + if (p->draining) { + MP_VERBOSE(ao, "underrun signaled for audio end\n"); + p->still_playing = false; + pthread_cond_broadcast(&p->wakeup); + } else { + ao_add_events(ao, AO_EVENT_UNDERRUN); + } - if (p->had_underrun) { - MP_VERBOSE(ao, "recover underrun\n"); - ao->driver->reset(ao); p->streaming = false; - p->had_underrun = false; } - struct mp_pcm_state state; - get_dev_state(ao, &state); // Round free space to period sizes to reduce number of write() calls. int space = state.free_samples / ao->period_size * ao->period_size; bool play_silence = p->paused || (ao->stream_silence && !p->still_playing); diff --git a/audio/out/internal.h b/audio/out/internal.h index 15ffd82e57..5cf99a02b7 100644 --- a/audio/out/internal.h +++ b/audio/out/internal.h @@ -87,9 +87,11 @@ struct mp_pcm_state { int free_samples; // number of free space in ring buffer int queued_samples; // number of samples to play in ring buffer double delay; // total latency in seconds (includes queued_samples) - bool underrun; // if in underrun state (signals both accidental - // underruns and normal playback end); cleared by AO - // driver on reset() calls + bool playing; // set if underlying API is actually playing audio; + // the AO must unset it on underrun (accidental + // underrun and EOF are indistinguishable; the upper + // layers decide what it was) + // real pausing may assume playing=true }; /* Note: @@ -149,13 +151,15 @@ struct ao_driver { void (*reset)(struct ao *ao); // push based: set pause state. Only called after start() and before reset(). // returns success (this is intended for paused=true; if it - // returns false, playback continues; unpausing always works) + // returns false, playback continues, and the core emulates via + // reset(); unpausing always works) bool (*set_pause)(struct ao *ao, bool paused); // pull based: start the audio callback // push based: start playing queued data // AO should call ao_wakeup_playthread() if a period boundary // is crossed, or playback stops due to external reasons // (including underruns or device removal) + // must set mp_pcm_state.playing; unset on error/underrun/end void (*start)(struct ao *ao); // push based: queue new data. This won't try to write more data than the // reported free space (samples <= mp_pcm_state.free_samples). -- cgit v1.2.3