All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaroslav Kysela <perex@perex.cz>
To: ALSA development <alsa-devel@alsa-project.org>
Cc: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Subject: [PATCH alsa-lib] pcm: hw: setup explicit silencing for snd_pcm_drain by default
Date: Fri, 21 Apr 2023 16:36:06 +0200	[thread overview]
Message-ID: <20230421143606.475100-1-perex@perex.cz> (raw)

RFC: Patch for alsa-lib. Not tested. For a discussion.

Some applications may not follow the period_size transfer blocks and
also the driver developers may not follow the consequeces of the
access beyond valid samples in the playback DMA buffer.

To avoid clicks, fill a little silence at the end of the playback
ring buffer when the snd_pcm_drain() is called.

TODO:
  - add SND_PCM_NO_DRAIN_SILENCE as a new flag for snd_pcm_open()
  - make this behaviour configurable (plugin config)

Cc: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 src/pcm/pcm.c       | 33 ++++++++++++++++++++-------------
 src/pcm/pcm_hw.c    | 45 +++++++++++++++++++++++++++++++++++++++++++++
 src/pcm/pcm_local.h |  4 ++++
 3 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 2b966d44..88b13ed4 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -6167,6 +6167,25 @@ int snd_pcm_hw_params_get_min_align(const snd_pcm_hw_params_t *params, snd_pcm_u
 	return 0;
 }
 
+#ifndef DOXYGEN
+void snd_pcm_sw_params_current_no_lock(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
+{
+	params->proto = SNDRV_PCM_VERSION;
+	params->tstamp_mode = pcm->tstamp_mode;
+	params->tstamp_type = pcm->tstamp_type;
+	params->period_step = pcm->period_step;
+	params->sleep_min = 0;
+	params->avail_min = pcm->avail_min;
+	sw_set_period_event(params, pcm->period_event);
+	params->xfer_align = 1;
+	params->start_threshold = pcm->start_threshold;
+	params->stop_threshold = pcm->stop_threshold;
+	params->silence_threshold = pcm->silence_threshold;
+	params->silence_size = pcm->silence_size;
+	params->boundary = pcm->boundary;
+}
+#endif
+
 /**
  * \brief Return current software configuration for a PCM
  * \param pcm PCM handle
@@ -6183,19 +6202,7 @@ int snd_pcm_sw_params_current(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
 		return -EIO;
 	}
 	__snd_pcm_lock(pcm); /* forced lock due to pcm field changes */
-	params->proto = SNDRV_PCM_VERSION;
-	params->tstamp_mode = pcm->tstamp_mode;
-	params->tstamp_type = pcm->tstamp_type;
-	params->period_step = pcm->period_step;
-	params->sleep_min = 0;
-	params->avail_min = pcm->avail_min;
-	sw_set_period_event(params, pcm->period_event);
-	params->xfer_align = 1;
-	params->start_threshold = pcm->start_threshold;
-	params->stop_threshold = pcm->stop_threshold;
-	params->silence_threshold = pcm->silence_threshold;
-	params->silence_size = pcm->silence_size;
-	params->boundary = pcm->boundary;
+	snd_pcm_sw_params_current_no_lock(pcm, params);
 	__snd_pcm_unlock(pcm);
 	return 0;
 }
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
index daa3e1ff..4995b2cd 100644
--- a/src/pcm/pcm_hw.c
+++ b/src/pcm/pcm_hw.c
@@ -98,6 +98,8 @@ typedef struct {
 	bool mmap_control_fallbacked;
 	struct snd_pcm_sync_ptr *sync_ptr;
 
+	bool prepare_reset_sw_params;
+
 	int period_event;
 	snd_timer_t *period_timer;
 	struct pollfd period_timer_pfd;
@@ -534,6 +536,7 @@ static int snd_pcm_hw_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t * params)
 		SYSMSG("SNDRV_PCM_IOCTL_SW_PARAMS failed (%i)", err);
 		goto out;
 	}
+	hw->prepare_reset_sw_params = false;
 	if ((snd_pcm_tstamp_type_t) params->tstamp_type != pcm->tstamp_type) {
 		if (hw->version < SNDRV_PROTOCOL_VERSION(2, 0, 12)) {
 			int on = (snd_pcm_tstamp_type_t) params->tstamp_type ==
@@ -660,7 +663,18 @@ static int snd_pcm_hw_hwsync(snd_pcm_t *pcm)
 static int snd_pcm_hw_prepare(snd_pcm_t *pcm)
 {
 	snd_pcm_hw_t *hw = pcm->private_data;
+	snd_pcm_sw_params_t sw_params;
 	int fd = hw->fd, err;
+
+	if (hw->prepare_reset_sw_params) {
+		snd_pcm_sw_params_current_no_lock(pcm, &sw_params);
+		if (ioctl(hw->fd, SNDRV_PCM_IOCTL_SW_PARAMS, sw_params) < 0) {
+			err = -errno;
+			SYSMSG("SNDRV_PCM_IOCTL_SW_PARAMS failed (%i)", err);
+			return err;
+		}
+		hw->prepare_reset_sw_params = false;
+	}
 	if (ioctl(fd, SNDRV_PCM_IOCTL_PREPARE) < 0) {
 		err = -errno;
 		SYSMSG("SNDRV_PCM_IOCTL_PREPARE failed (%i)", err);
@@ -718,7 +732,38 @@ static int snd_pcm_hw_drop(snd_pcm_t *pcm)
 static int snd_pcm_hw_drain(snd_pcm_t *pcm)
 {
 	snd_pcm_hw_t *hw = pcm->private_data;
+	snd_pcm_sw_params_t sw_params;
+	snd_pcm_uframes_t silence_size;
 	int err;
+
+	if (pcm->stream != SND_PCM_STREAM_PLAYBACK)
+		goto __skip_silence;
+	/* compute end silence size, align to period size + extra time */
+	snd_pcm_sw_params_current_no_lock(pcm, &sw_params);
+	if ((pcm->buffer_size % pcm->period_size) == 0) {
+		silence_size = pcm->period_size - (pcm->period_size % *pcm->appl.ptr);
+		if (silence_size == pcm->period_size)
+			silence_size = 0;
+	} else {
+		/* it not not easy to compute the period cross point
+		 * in this case because boundary is aligned only
+		 * to the buffer_size - use the full range (period)
+		 */
+		silence_size = pcm->period_size;
+	}
+	silence_size += pcm->rate / 10;	/* 1/10th of second */
+	if (sw_params.silence_size < silence_size) {
+		/* fill silence soon as possible (in the bellow ioctl) */
+		sw_params.silence_threshold = pcm->buffer_size;
+		sw_params.silence_size = silence_size;
+		if (ioctl(hw->fd, SNDRV_PCM_IOCTL_SW_PARAMS, sw_params) < 0) {
+			err = -errno;
+			SYSMSG("SNDRV_PCM_IOCTL_SW_PARAMS failed (%i)", err);
+			return err;
+		}
+		hw->prepare_reset_sw_params = true;
+	}
+__skip_silence:
 	if (ioctl(hw->fd, SNDRV_PCM_IOCTL_DRAIN) < 0) {
 		err = -errno;
 		SYSMSG("SNDRV_PCM_IOCTL_DRAIN failed (%i)", err);
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index ae0c44bf..4a859cd1 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -366,6 +366,8 @@ struct _snd_pcm {
 	snd1_pcm_hw_param_get_max
 #define snd_pcm_hw_param_name		\
 	snd1_pcm_hw_param_name
+#define snd_pcm_sw_params_current_no_lock \
+	snd1_pcm_sw_params_current_no_lock
 
 int snd_pcm_new(snd_pcm_t **pcmp, snd_pcm_type_t type, const char *name,
 		snd_pcm_stream_t stream, int mode);
@@ -390,6 +392,8 @@ void snd_pcm_mmap_appl_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
 void snd_pcm_mmap_hw_backward(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
 void snd_pcm_mmap_hw_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
 
+void snd_pcm_sw_params_current_no_lock(snd_pcm_t *pcm, snd_pcm_sw_params_t *params);
+
 snd_pcm_sframes_t snd_pcm_mmap_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size);
 snd_pcm_sframes_t snd_pcm_mmap_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size);
 snd_pcm_sframes_t snd_pcm_mmap_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size);
-- 
2.39.2


                 reply	other threads:[~2023-04-21 14:37 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230421143606.475100-1-perex@perex.cz \
    --to=perex@perex.cz \
    --cc=alsa-devel@alsa-project.org \
    --cc=oswald.buddenhagen@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.