summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2020-08-29 16:27:56 +0200
committerwm4 <wm4@nowhere>2020-08-29 16:27:56 +0200
commit478d39c57492107ce8d5a33e80d5624db330ceab (patch)
tree71c4cd38145f5256fdbfd06403fbd8da7a988c14
parent3427aa4776b73ce0b9c8996bc4ef38da87fe1557 (diff)
downloadmpv-478d39c57492107ce8d5a33e80d5624db330ceab.tar.bz2
mpv-478d39c57492107ce8d5a33e80d5624db330ceab.tar.xz
audio: fix inefficient behavior with ao_alsa, remove period_size field
It is now the AO's responsibility to handle period size alignment. The ao->period_size alignment field is unused as of the recent audio refactor commit. Remove it. It turns out that ao_alsa shows extremely inefficient behavior as a consequence of the removal of period size aligned writes in the mentioned refactor commit. This is because it could get into a state where it repeatedly wrote single samples (as small as 1 sample), and starved the rest of the player as a consequence. Too bad. Explicitly align the size in ao_alsa. Other AOs, which need this, should do the same. One reason why it broke so badly with ao_alsa was that it retried the write() even if all reported space could be written. So stop doing that too. Retry the write only if we somehow wrote less. I'm not sure about ao_pulse.
-rw-r--r--audio/out/ao.c7
-rw-r--r--audio/out/ao_alsa.c3
-rw-r--r--audio/out/ao_lavc.c1
-rw-r--r--audio/out/ao_null.c2
-rw-r--r--audio/out/ao_openal.c1
-rw-r--r--audio/out/buffer.c10
-rw-r--r--audio/out/internal.h13
7 files changed, 13 insertions, 24 deletions
diff --git a/audio/out/ao.c b/audio/out/ao.c
index 32ffc3d82d..52a38b63be 100644
--- a/audio/out/ao.c
+++ b/audio/out/ao.c
@@ -204,8 +204,6 @@ static struct ao *ao_init(bool probing, struct mpv_global *global,
init_buffer_pre(ao);
- ao->period_size = 1;
-
int r = ao->driver->init(ao);
if (r < 0) {
// Silly exception for coreaudio spdif redirection
@@ -222,11 +220,6 @@ static struct ao *ao_init(bool probing, struct mpv_global *global,
}
ao->driver_initialized = true;
- if (ao->period_size < 1) {
- MP_ERR(ao, "Invalid period size set.\n");
- goto fail;
- }
-
ao->sstride = af_fmt_to_bytes(ao->format);
ao->num_planes = 1;
if (af_fmt_is_planar(ao->format)) {
diff --git a/audio/out/ao_alsa.c b/audio/out/ao_alsa.c
index 0380162eb4..49bdd32835 100644
--- a/audio/out/ao_alsa.c
+++ b/audio/out/ao_alsa.c
@@ -850,7 +850,6 @@ static int init_device(struct ao *ao, int mode)
MP_VERBOSE(ao, "period size: %d samples\n", (int)p->outburst);
ao->device_buffer = p->buffersize;
- ao->period_size = p->outburst;
p->convert.channels = ao->channels.num;
@@ -998,6 +997,8 @@ static bool recover_and_get_state(struct ao *ao, struct mp_pcm_state *state)
state->delay = MPMAX(del, 0) / (double)ao->samplerate;
state->free_samples = snd_pcm_status_get_avail(st);
state->free_samples = MPCLAMP(state->free_samples, 0, ao->device_buffer);
+ // Align to period size.
+ state->free_samples = state->free_samples / p->outburst * p->outburst;
state->queued_samples = ao->device_buffer - state->free_samples;
state->playing = pcmst == SND_PCM_STATE_RUNNING ||
pcmst == SND_PCM_STATE_PAUSED;
diff --git a/audio/out/ao_lavc.c b/audio/out/ao_lavc.c
index 049f8df2cf..8a98f0b67a 100644
--- a/audio/out/ao_lavc.c
+++ b/audio/out/ao_lavc.c
@@ -158,7 +158,6 @@ static int init(struct ao *ao)
ao->untimed = true;
ao->device_buffer = ac->aframesize * ac->framecount;
- ao->period_size = ao->device_buffer;
ac->filter_root = mp_filter_create_root(ao->global);
ac->fix_frame_size = mp_fixed_aframe_size_create(ac->filter_root,
diff --git a/audio/out/ao_null.c b/audio/out/ao_null.c
index 2f58ea22c3..c8c4117ec0 100644
--- a/audio/out/ao_null.c
+++ b/audio/out/ao_null.c
@@ -109,8 +109,6 @@ static int init(struct ao *ao)
priv->last_time = mp_time_sec();
- ao->period_size = priv->outburst;
-
return 0;
}
diff --git a/audio/out/ao_openal.c b/audio/out/ao_openal.c
index 64d957e893..0c5ec2d7f1 100644
--- a/audio/out/ao_openal.c
+++ b/audio/out/ao_openal.c
@@ -267,7 +267,6 @@ static int init(struct ao *ao)
goto err_out;
}
- ao->period_size = p->num_samples;
return 0;
err_out:
diff --git a/audio/out/buffer.c b/audio/out/buffer.c
index 4c08a26b0b..9cd7717000 100644
--- a/audio/out/buffer.c
+++ b/audio/out/buffer.c
@@ -619,7 +619,7 @@ static bool ao_play_data(struct ao *ao)
if (got_eof)
goto eof;
- return samples > 0;
+ return samples > 0 && (samples < space || ao->untimed);
eof:
MP_VERBOSE(ao, "audio end or underrun\n");
@@ -642,14 +642,14 @@ static void *playthread(void *arg)
while (1) {
pthread_mutex_lock(&p->lock);
- bool progress = false;
+ bool retry = false;
if (!ao->driver->initially_blocked || p->initial_unblocked)
- progress = ao_play_data(ao);
+ retry = ao_play_data(ao);
// Wait until the device wants us to write more data to it.
// Fallback to guessing.
double timeout = INFINITY;
- if (p->streaming && !p->paused && !progress) {
+ if (p->streaming && !p->paused && !retry) {
// Wake up again if half of the audio buffer has been played.
// Since audio could play at a faster or slower pace, wake up twice
// as often as ideally needed.
@@ -663,7 +663,7 @@ static void *playthread(void *arg)
pthread_mutex_unlock(&p->pt_lock);
break;
}
- if (!p->need_wakeup && !progress) {
+ if (!p->need_wakeup && !retry) {
MP_STATS(ao, "start audio wait");
struct timespec ts = mp_rel_time_to_timespec(timeout);
pthread_cond_timedwait(&p->pt_wakeup, &p->pt_lock, &ts);
diff --git a/audio/out/internal.h b/audio/out/internal.h
index 5b922657d0..6eb7016f2d 100644
--- a/audio/out/internal.h
+++ b/audio/out/internal.h
@@ -48,12 +48,6 @@ struct ao {
int init_flags; // AO_INIT_* flags
bool stream_silence; // if audio inactive, just play silence
- // Set by the driver on init.
- // This value is in complete samples (i.e. 1 for stereo means 1 sample
- // for both channels each).
- // Used for push based API only.
- int period_size;
-
// The device as selected by the user, usually using ao_device_desc.name
// from an entry from the list returned by driver->list_devices. If the
// default device should be used, this is set to NULL.
@@ -83,7 +77,12 @@ bool init_buffer_post(struct ao *ao);
struct mp_pcm_state {
// Note: free_samples+queued_samples <= ao->device_buffer; the sum may be
// less if the audio API can report partial periods played, while
- // free_samples should be period-size aligned.
+ // free_samples should be period-size aligned. If free_samples is not
+ // period-size aligned, the AO thread might get into a situation where
+ // it writes a very small number of samples in each iteration, leading
+ // to extremely inefficient behavior.
+ // Keep in mind that write() may write less than free_samples (or your
+ // period size alignment) anyway.
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)