From cb90a88d382971a146c4b7413bb6df6563cef7d1 Mon Sep 17 00:00:00 2001 From: Walter Lozano Date: Sun, 30 Aug 2020 00:04:14 -0300 Subject: meta-pipewire: additional improvements for iMX8MQ Include additional improvement related to iMX8MQ, which should help to mitigate audio issues. First, add a warning if not all the committed data is actually read/written. Secondly, if snd_pcm_hw_params_is_batch == 1 tweak the delay to make sure at least a period is available in the buffer to avoid xrun. Bug-AGL: SPEC-3410 Signed-off-by: Walter Lozano Change-Id: Ief2e69960b3e13c1d5085581d1a7c7b3e73570c0 Reviewed-on: https://gerrit.automotivelinux.org/gerrit/c/AGL/meta-agl/+/25164 Tested-by: Jenkins Job builder account ci-image-build: Jenkins Job builder account ci-image-boot-test: Jenkins Job builder account Reviewed-by: Jan-Simon Moeller --- ...add-warning-in-case-of-partial-read-write.patch | 80 ++++++++++++++++++++++ ...9-alsa-adjust-delay-depending-on-hardware.patch | 64 +++++++++++++++++ .../recipes-multimedia/pipewire/pipewire_git.bb | 2 + 3 files changed, 146 insertions(+) create mode 100644 meta-pipewire/recipes-multimedia/pipewire/pipewire/0008-alsa-add-warning-in-case-of-partial-read-write.patch create mode 100644 meta-pipewire/recipes-multimedia/pipewire/pipewire/0009-alsa-adjust-delay-depending-on-hardware.patch diff --git a/meta-pipewire/recipes-multimedia/pipewire/pipewire/0008-alsa-add-warning-in-case-of-partial-read-write.patch b/meta-pipewire/recipes-multimedia/pipewire/pipewire/0008-alsa-add-warning-in-case-of-partial-read-write.patch new file mode 100644 index 000000000..98a2c98fc --- /dev/null +++ b/meta-pipewire/recipes-multimedia/pipewire/pipewire/0008-alsa-add-warning-in-case-of-partial-read-write.patch @@ -0,0 +1,80 @@ +From 45658f75e61da47b274f2eba3a55e62d016f8b42 Mon Sep 17 00:00:00 2001 +From: Walter Lozano +Date: Mon, 24 Aug 2020 12:08:32 -0300 +Subject: [PATCH 8/9] alsa: add warning in case of partial read/write + +Currently alsa_read and alsa_write assumes that all the frames committed +using snd_pcm_mmap_commit are read or written, which is probably true. +However, as it could be some corner cases add a warning to notice this +fact. + +Signed-off-by: Walter Lozano +--- + spa/plugins/alsa/alsa-pcm.c | 28 ++++++++++++++++++++-------- + 1 file changed, 20 insertions(+), 8 deletions(-) + +diff --git a/spa/plugins/alsa/alsa-pcm.c b/spa/plugins/alsa/alsa-pcm.c +index ed9bf42b..92ef2151 100644 +--- a/spa/plugins/alsa/alsa-pcm.c ++++ b/spa/plugins/alsa/alsa-pcm.c +@@ -721,6 +721,7 @@ int spa_alsa_write(struct state *state, snd_pcm_uframes_t silence) + snd_pcm_t *hndl = state->hndl; + const snd_pcm_channel_area_t *my_areas; + snd_pcm_uframes_t written, frames, offset, off, to_write, total_written; ++ snd_pcm_sframes_t commitres; + int res; + + if (state->position && state->duration != state->position->clock.duration) { +@@ -834,11 +835,16 @@ again: + state, offset, written, state->sample_count); + total_written += written; + +- if ((res = snd_pcm_mmap_commit(hndl, offset, written)) < 0) { ++ if ((commitres = snd_pcm_mmap_commit(hndl, offset, written)) < 0) { + spa_log_error(state->log, NAME" %p: snd_pcm_mmap_commit error: %s", +- state, snd_strerror(res)); +- if (res != -EPIPE && res != -ESTRPIPE) +- return res; ++ state, snd_strerror(commitres)); ++ if (commitres != -EPIPE && commitres != -ESTRPIPE) ++ return commitres; ++ } ++ ++ if (commitres > 0 && written != (snd_pcm_uframes_t) commitres) { ++ spa_log_warn(state->log, NAME" %p: mmap_commit wrote %ld instead of %ld", ++ state, commitres, written); + } + + if (!spa_list_is_empty(&state->ready) && written > 0) +@@ -922,6 +928,7 @@ int spa_alsa_read(struct state *state, snd_pcm_uframes_t silence) + snd_pcm_uframes_t total_read = 0, to_read; + const snd_pcm_channel_area_t *my_areas; + snd_pcm_uframes_t read, frames, offset; ++ snd_pcm_sframes_t commitres; + int res; + + if (state->position) { +@@ -994,11 +1001,16 @@ int spa_alsa_read(struct state *state, snd_pcm_uframes_t silence) + offset, read, state->sample_count); + total_read += read; + +- if ((res = snd_pcm_mmap_commit(hndl, offset, read)) < 0) { ++ if ((commitres = snd_pcm_mmap_commit(hndl, offset, read)) < 0) { + spa_log_error(state->log, NAME" %p: snd_pcm_mmap_commit error: %s", +- state, snd_strerror(res)); +- if (res != -EPIPE && res != -ESTRPIPE) +- return res; ++ state, snd_strerror(commitres)); ++ if (commitres != -EPIPE && commitres != -ESTRPIPE) ++ return commitres; ++ } ++ ++ if (commitres > 0 && read != (snd_pcm_uframes_t) commitres) { ++ spa_log_warn(state->log, NAME" %p: mmap_commit read %ld instead of %ld", ++ state, commitres, read); + } + + state->sample_count += total_read; +-- +2.20.1 + diff --git a/meta-pipewire/recipes-multimedia/pipewire/pipewire/0009-alsa-adjust-delay-depending-on-hardware.patch b/meta-pipewire/recipes-multimedia/pipewire/pipewire/0009-alsa-adjust-delay-depending-on-hardware.patch new file mode 100644 index 000000000..a448063f1 --- /dev/null +++ b/meta-pipewire/recipes-multimedia/pipewire/pipewire/0009-alsa-adjust-delay-depending-on-hardware.patch @@ -0,0 +1,64 @@ +From 38cdfa4483de4c2e91bfccb9c22ec72d9c3720f4 Mon Sep 17 00:00:00 2001 +From: Walter Lozano +Date: Sat, 22 Aug 2020 11:51:30 -0300 +Subject: [PATCH 9/9] alsa: adjust delay depending on hardware + +Currently PipeWire is able to reproduce audio in systems where +DMA granularity is not burst but it could face an xrun. + +In order to mitigate this issue, adjust the delay PipeWire +calculates to make sure that a period is available in the buffer +when snd_pcm_hw_params_is_batch == 1. + +Signed-off-by: Walter Lozano +--- + spa/plugins/alsa/alsa-pcm.c | 12 +++++++++++- + spa/plugins/alsa/alsa-pcm.h | 1 + + 2 files changed, 12 insertions(+), 1 deletion(-) + +diff --git a/spa/plugins/alsa/alsa-pcm.c b/spa/plugins/alsa/alsa-pcm.c +index 92ef2151..1f15085f 100644 +--- a/spa/plugins/alsa/alsa-pcm.c ++++ b/spa/plugins/alsa/alsa-pcm.c +@@ -462,8 +462,9 @@ int spa_alsa_set_format(struct state *state, struct spa_audio_info *fmt, uint32_ + state->frame_size = info->channels * (snd_pcm_format_physical_width(format) / 8); + + dir = 0; ++ state->pcm_is_batch = snd_pcm_hw_params_is_batch(params); + period_size = 1024; +- if (snd_pcm_hw_params_is_batch(params)) { ++ if (state->pcm_is_batch) { + period_size = 512; + spa_log_warn(state->log, NAME" hardware does double buffering, changing period_size to %ld", period_size); + } +@@ -639,6 +640,15 @@ static int get_status(struct state *state, snd_pcm_uframes_t *delay, snd_pcm_ufr + + if (state->stream == SND_PCM_STREAM_PLAYBACK) { + *delay = state->buffer_frames - avail; ++ if (state->pcm_is_batch) { ++ /* In this case, as we don't have a good granularity in the ++ * avail report try to compensate this by tweaking the delay ++ * and make sure that a period is available in the buffer */ ++ if (*delay > state->period_frames) ++ *delay = *delay - state->period_frames; ++ else ++ *delay = 0; ++ } + } + else { + *delay = avail; +diff --git a/spa/plugins/alsa/alsa-pcm.h b/spa/plugins/alsa/alsa-pcm.h +index b7a2dd29..3b5c0d7b 100644 +--- a/spa/plugins/alsa/alsa-pcm.h ++++ b/spa/plugins/alsa/alsa-pcm.h +@@ -100,6 +100,7 @@ struct state { + + bool have_format; + struct spa_audio_info current_format; ++ bool pcm_is_batch; + + snd_pcm_uframes_t buffer_frames; + snd_pcm_uframes_t period_frames; +-- +2.20.1 + diff --git a/meta-pipewire/recipes-multimedia/pipewire/pipewire_git.bb b/meta-pipewire/recipes-multimedia/pipewire/pipewire_git.bb index 7ca4237ff..e2560ad9d 100644 --- a/meta-pipewire/recipes-multimedia/pipewire/pipewire_git.bb +++ b/meta-pipewire/recipes-multimedia/pipewire/pipewire_git.bb @@ -8,6 +8,8 @@ SRC_URI = "git://gitlab.freedesktop.org/pipewire/pipewire.git;protocol=https;bra file://0005-module-access-add-same-sec-label-mode.patch \ file://0006-alsa-pcm-call-reuse_buffers-when-resetting-the-state.patch \ file://0007-alsa-Set-period_size-depending-on-hardware.patch \ + file://0008-alsa-add-warning-in-case-of-partial-read-write.patch \ + file://0009-alsa-adjust-delay-depending-on-hardware.patch \ " SRCREV = "b0932e687fc47e0872ca291531f2291d99042d70" -- cgit 1.2.3-korg