[1/6] alsa-lib:pcm: allow users to configure different period sizes.

Commit Message

[e-mail] Dec. 30, 2016, 6:26 a.m. UTC

From: Joshua Frkuska [e-mail]

This patch allows the effective period size to be a multiple of the slave-pcm period size.
Allowing only exact multiple of original period size is achieved by borrowing code from the
kernel hwrules implemention.

This patch is intended to save cpu workload when for example, the slave operates with very
small periods but a user does not need that small periods.

This feature is enabled by default and can be disabled by adding config option 'var_periodsize 0'.

Signed-off-by: Alexander Jahn <[e-mail]>
Signed-off-by: Andreas Pape <[e-mail]>

Comments

Takashi Iwai Jan. 2, 2017, 1:51 p.m. UTC | #1

On Fri, 30 Dec 2016 07:26:15 +0100,
[e-mail] wrote:
> 
> From: Joshua Frkuska [e-mail]
> 
> This patch allows the effective period size to be a multiple of the slave-pcm period size.
> Allowing only exact multiple of original period size is achieved by borrowing code from the
> kernel hwrules implemention.
> 
> This patch is intended to save cpu workload when for example, the slave operates with very
> small periods but a user does not need that small periods.
> 
> This feature is enabled by default and can be disabled by adding config option 'var_periodsize 0'.
> 
> Signed-off-by: Alexander Jahn <[e-mail]>
> Signed-off-by: Andreas Pape <[e-mail]>

Looks like a good improvement.  I applied this patch with a slight
coding style fixes.


thanks,

Takashi

> diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
> index 6434983..8f42b19 100644
> --- a/src/pcm/pcm_direct.c
> +++ b/src/pcm/pcm_direct.c
> @@ -660,6 +660,28 @@ static int hw_param_interval_refine_minmax(snd_pcm_hw_params_t *params,
>  	return hw_param_interval_refine_one(params, var, &t);
>  }
>  
> +/* this code is used 'as-is' from the alsa kernel code */
> +static int snd_interval_step(struct snd_interval *i, unsigned int min, unsigned int step)
> +{
> +	unsigned int n;
> +	int changed = 0;
> +	n = (i->min - min) % step;
> +	if (n != 0 || i->openmin) {
> +		i->min += step - n;
> +		changed = 1;
> +	}
> +	n = (i->max - min) % step;
> +	if (n != 0 || i->openmax) {
> +		i->max -= n;
> +		changed = 1;
> +	}
> +	if (snd_interval_checkempty(i)) {
> +		i->empty = 1;
> +		return -EINVAL;
> +	}
> +	return changed;
> +}
> +
>  #undef REFINE_DEBUG
>  
>  int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
> @@ -710,15 +732,16 @@ int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
>  					   &dshare->shmptr->hw.rate);
>  	if (err < 0)
>  		return err;
> -	err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_SIZE,
> -					   &dshare->shmptr->hw.period_size);
> -	if (err < 0)
> -		return err;
> -	err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_TIME,
> -					   &dshare->shmptr->hw.period_time);
> -	if (err < 0)
> -		return err;
> +
>  	if (dshare->max_periods < 0) {
> +		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_SIZE,
> +						   &dshare->shmptr->hw.period_size);
> +		if (err < 0)
> +			return err;
> +		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_TIME,
> +						   &dshare->shmptr->hw.period_time);
> +		if (err < 0)
> +			return err;
>  		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_BUFFER_SIZE,
>  						   &dshare->shmptr->hw.buffer_size);
>  		if (err < 0)
> @@ -730,11 +753,38 @@ int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
>  	} else if (params->rmask & ((1<<SND_PCM_HW_PARAM_PERIODS)|
>  				    (1<<SND_PCM_HW_PARAM_BUFFER_BYTES)|
>  				    (1<<SND_PCM_HW_PARAM_BUFFER_SIZE)|
> -				    (1<<SND_PCM_HW_PARAM_BUFFER_TIME))) {
> +				    (1<<SND_PCM_HW_PARAM_BUFFER_TIME)|
> +				    (1<<SND_PCM_HW_PARAM_PERIOD_TIME)|
> +				    (1<<SND_PCM_HW_PARAM_PERIOD_SIZE)|
> +				    (1<<SND_PCM_HW_PARAM_PERIOD_BYTES))) {
> +		snd_interval_t period_size = dshare->shmptr->hw.period_size;
> +		snd_interval_t period_time = dshare->shmptr->hw.period_time;
>  		int changed;
>  		unsigned int max_periods = dshare->max_periods;
>  		if (max_periods < 2)
>  			max_periods = dshare->slave_buffer_size / dshare->slave_period_size;
> +
> +		/*make sure buffer size does not exceed slave buffer size*/
> +		err = hw_param_interval_refine_minmax(params, SND_PCM_HW_PARAM_BUFFER_SIZE,
> +					2 * dshare->slave_period_size, dshare->slave_buffer_size);
> +		if (err < 0)
> +			return err;
> +		if (dshare->var_periodsize) {
> +			/*more tolerant settings...*/
> +			if((dshare->shmptr->hw.buffer_size.max / 2) > period_size.max)
> +				period_size.max = dshare->shmptr->hw.buffer_size.max / 2;
> +			if((dshare->shmptr->hw.buffer_time.max / 2) > period_time.max)
> +				period_time.max = dshare->shmptr->hw.buffer_time.max / 2;
> +		}
> +
> +		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_SIZE,
> +						   &period_size);
> +		if (err < 0)
> +			return err;
> +		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_TIME,
> +						   &period_time);
> +		if (err < 0)
> +			return err;
>  		do {
>  			changed = 0;
>  			err = hw_param_interval_refine_minmax(params, SND_PCM_HW_PARAM_PERIODS,
> @@ -746,8 +796,16 @@ int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
>  			if (err < 0)
>  				return err;
>  			changed |= err;
> +			err = snd_interval_step(hw_param_interval(params, SND_PCM_HW_PARAM_PERIOD_SIZE),
> +								0, dshare->slave_period_size);
> +			if (err < 0)
> +				return err;
> +			changed |= err;
> +			if (err)
> +				params->rmask |= (1<<SND_PCM_HW_PARAM_PERIOD_SIZE);
>  		} while (changed);
>  	}
> +	dshare->timer_ticks = hw_param_interval(params, SND_PCM_HW_PARAM_PERIOD_SIZE)->max/dshare->slave_period_size;
>  	params->info = dshare->shmptr->s.info;
>  #ifdef REFINE_DEBUG
>  	snd_output_puts(log, "DMIX REFINE (end):\n");
> @@ -1183,6 +1241,7 @@ int snd_pcm_direct_initialize_poll_fd(snd_pcm_direct_t *dmix)
>  
>  	dmix->tread = 1;
>  	dmix->timer_need_poll = 0;
> +	dmix->timer_ticks = 1;
>  	ret = snd_pcm_info(dmix->spcm, &info);
>  	if (ret < 0) {
>  		SNDERR("unable to info for slave pcm");
> @@ -1366,7 +1425,7 @@ int snd_pcm_direct_set_timer_params(snd_pcm_direct_t *dmix)
>  	snd_timer_params_set_auto_start(&params, 1);
>  	if (dmix->type != SND_PCM_TYPE_DSNOOP)
>  		snd_timer_params_set_early_event(&params, 1);
> -	snd_timer_params_set_ticks(&params, 1);
> +	snd_timer_params_set_ticks(&params, dmix->timer_ticks);
>  	if (dmix->tread) {
>  		filter = (1<<SND_TIMER_EVENT_TICK) |
>  			 dmix->timer_events;
> @@ -1656,6 +1715,7 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
>  	rec->ipc_gid = -1;
>  	rec->slowptr = 1;
>  	rec->max_periods = 0;
> +	rec->var_periodsize = 1;
>  
>  	/* read defaults */
>  	if (snd_config_search(root, "defaults.pcm.dmix_max_periods", &n) >= 0) {
> @@ -1762,6 +1822,13 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
>  			rec->max_periods = val;
>  			continue;
>  		}
> +		if (strcmp(id, "var_periodsize") == 0) {
> +			err = snd_config_get_bool(n);
> +			if (err < 0)
> +				return err;
> +			rec->var_periodsize = err;
> +			continue;
> +		}
>  		SNDERR("Unknown field %s", id);
>  		return -EINVAL;
>  	}
> diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h
> index 611ad29..91e816c 100644
> --- a/src/pcm/pcm_direct.h
> +++ b/src/pcm/pcm_direct.h
> @@ -147,12 +147,14 @@ struct snd_pcm_direct {
>  	int tread: 1;
>  	int timer_need_poll: 1;
>  	unsigned int timer_events;
> +	unsigned int timer_ticks;
>  	int server_fd;
>  	pid_t server_pid;
>  	snd_timer_t *timer; 		/* timer used as poll_fd */
>  	int interleaved;	 	/* we have interleaved buffer */
>  	int slowptr;			/* use slow but more precise ptr updates */
>  	int max_periods;		/* max periods (-1 = fixed periods, 0 = max buffer size) */
> +	int var_periodsize;		/* allow variable period size if max_periods is != -1*/
>  	unsigned int channels;		/* client's channels */
>  	unsigned int *bindings;
>  	union {
> @@ -326,6 +328,7 @@ struct snd_pcm_direct_open_conf {
>  	int ipc_gid;
>  	int slowptr;
>  	int max_periods;
> +	int var_periodsize;
>  	snd_config_t *slave;
>  	snd_config_t *bindings;
>  };
> diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
> index 825677f..0ab7323 100644
> --- a/src/pcm/pcm_dmix.c
> +++ b/src/pcm/pcm_dmix.c
> @@ -1040,6 +1040,7 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
>  	dmix->state = SND_PCM_STATE_OPEN;
>  	dmix->slowptr = opts->slowptr;
>  	dmix->max_periods = opts->max_periods;
> +	dmix->var_periodsize = opts->var_periodsize;
>  	dmix->sync_ptr = snd_pcm_dmix_sync_ptr;
>  
>   retry:
> diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
> index 29cd6c6..a1fed5d 100644
> --- a/src/pcm/pcm_dshare.c
> +++ b/src/pcm/pcm_dshare.c
> @@ -725,6 +725,7 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
>  	dshare->state = SND_PCM_STATE_OPEN;
>  	dshare->slowptr = opts->slowptr;
>  	dshare->max_periods = opts->max_periods;
> +	dshare->var_periodsize = opts->var_periodsize;
>  	dshare->sync_ptr = snd_pcm_dshare_sync_ptr;
>  
>   retry:
> diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
> index 1aedf3c..85f0ff4 100644
> --- a/src/pcm/pcm_dsnoop.c
> +++ b/src/pcm/pcm_dsnoop.c
> @@ -606,6 +606,7 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
>  	dsnoop->state = SND_PCM_STATE_OPEN;
>  	dsnoop->slowptr = opts->slowptr;
>  	dsnoop->max_periods = opts->max_periods;
> +	dsnoop->var_periodsize = opts->var_periodsize;
>  	dsnoop->sync_ptr = snd_pcm_dsnoop_sync_ptr;
>  
>   retry:
> -- 
> 1.7.9.5
>

Patch

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 6434983..8f42b19 100644
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -660,6 +660,28 @@  static int hw_param_interval_refine_minmax(snd_pcm_hw_params_t *params,
 	return hw_param_interval_refine_one(params, var, &t);
 }
 
+/* this code is used 'as-is' from the alsa kernel code */
+static int snd_interval_step(struct snd_interval *i, unsigned int min, unsigned int step)
+{
+	unsigned int n;
+	int changed = 0;
+	n = (i->min - min) % step;
+	if (n != 0 || i->openmin) {
+		i->min += step - n;
+		changed = 1;
+	}
+	n = (i->max - min) % step;
+	if (n != 0 || i->openmax) {
+		i->max -= n;
+		changed = 1;
+	}
+	if (snd_interval_checkempty(i)) {
+		i->empty = 1;
+		return -EINVAL;
+	}
+	return changed;
+}
+
 #undef REFINE_DEBUG
 
 int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
@@ -710,15 +732,16 @@  int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
 					   &dshare->shmptr->hw.rate);
 	if (err < 0)
 		return err;
-	err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_SIZE,
-					   &dshare->shmptr->hw.period_size);
-	if (err < 0)
-		return err;
-	err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_TIME,
-					   &dshare->shmptr->hw.period_time);
-	if (err < 0)
-		return err;
+
 	if (dshare->max_periods < 0) {
+		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_SIZE,
+						   &dshare->shmptr->hw.period_size);
+		if (err < 0)
+			return err;
+		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_TIME,
+						   &dshare->shmptr->hw.period_time);
+		if (err < 0)
+			return err;
 		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_BUFFER_SIZE,
 						   &dshare->shmptr->hw.buffer_size);
 		if (err < 0)
@@ -730,11 +753,38 @@  int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
 	} else if (params->rmask & ((1<<SND_PCM_HW_PARAM_PERIODS)|
 				    (1<<SND_PCM_HW_PARAM_BUFFER_BYTES)|
 				    (1<<SND_PCM_HW_PARAM_BUFFER_SIZE)|
-				    (1<<SND_PCM_HW_PARAM_BUFFER_TIME))) {
+				    (1<<SND_PCM_HW_PARAM_BUFFER_TIME)|
+				    (1<<SND_PCM_HW_PARAM_PERIOD_TIME)|
+				    (1<<SND_PCM_HW_PARAM_PERIOD_SIZE)|
+				    (1<<SND_PCM_HW_PARAM_PERIOD_BYTES))) {
+		snd_interval_t period_size = dshare->shmptr->hw.period_size;
+		snd_interval_t period_time = dshare->shmptr->hw.period_time;
 		int changed;
 		unsigned int max_periods = dshare->max_periods;
 		if (max_periods < 2)
 			max_periods = dshare->slave_buffer_size / dshare->slave_period_size;
+
+		/*make sure buffer size does not exceed slave buffer size*/
+		err = hw_param_interval_refine_minmax(params, SND_PCM_HW_PARAM_BUFFER_SIZE,
+					2 * dshare->slave_period_size, dshare->slave_buffer_size);
+		if (err < 0)
+			return err;
+		if (dshare->var_periodsize) {
+			/*more tolerant settings...*/
+			if((dshare->shmptr->hw.buffer_size.max / 2) > period_size.max)
+				period_size.max = dshare->shmptr->hw.buffer_size.max / 2;
+			if((dshare->shmptr->hw.buffer_time.max / 2) > period_time.max)
+				period_time.max = dshare->shmptr->hw.buffer_time.max / 2;
+		}
+
+		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_SIZE,
+						   &period_size);
+		if (err < 0)
+			return err;
+		err = hw_param_interval_refine_one(params, SND_PCM_HW_PARAM_PERIOD_TIME,
+						   &period_time);
+		if (err < 0)
+			return err;
 		do {
 			changed = 0;
 			err = hw_param_interval_refine_minmax(params, SND_PCM_HW_PARAM_PERIODS,
@@ -746,8 +796,16 @@  int snd_pcm_direct_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
 			if (err < 0)
 				return err;
 			changed |= err;
+			err = snd_interval_step(hw_param_interval(params, SND_PCM_HW_PARAM_PERIOD_SIZE),
+								0, dshare->slave_period_size);
+			if (err < 0)
+				return err;
+			changed |= err;
+			if (err)
+				params->rmask |= (1<<SND_PCM_HW_PARAM_PERIOD_SIZE);
 		} while (changed);
 	}
+	dshare->timer_ticks = hw_param_interval(params, SND_PCM_HW_PARAM_PERIOD_SIZE)->max/dshare->slave_period_size;
 	params->info = dshare->shmptr->s.info;
 #ifdef REFINE_DEBUG
 	snd_output_puts(log, "DMIX REFINE (end):\n");
@@ -1183,6 +1241,7 @@  int snd_pcm_direct_initialize_poll_fd(snd_pcm_direct_t *dmix)
 
 	dmix->tread = 1;
 	dmix->timer_need_poll = 0;
+	dmix->timer_ticks = 1;
 	ret = snd_pcm_info(dmix->spcm, &info);
 	if (ret < 0) {
 		SNDERR("unable to info for slave pcm");
@@ -1366,7 +1425,7 @@  int snd_pcm_direct_set_timer_params(snd_pcm_direct_t *dmix)
 	snd_timer_params_set_auto_start(&params, 1);
 	if (dmix->type != SND_PCM_TYPE_DSNOOP)
 		snd_timer_params_set_early_event(&params, 1);
-	snd_timer_params_set_ticks(&params, 1);
+	snd_timer_params_set_ticks(&params, dmix->timer_ticks);
 	if (dmix->tread) {
 		filter = (1<<SND_TIMER_EVENT_TICK) |
 			 dmix->timer_events;
@@ -1656,6 +1715,7 @@  int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
 	rec->ipc_gid = -1;
 	rec->slowptr = 1;
 	rec->max_periods = 0;
+	rec->var_periodsize = 1;
 
 	/* read defaults */
 	if (snd_config_search(root, "defaults.pcm.dmix_max_periods", &n) >= 0) {
@@ -1762,6 +1822,13 @@  int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
 			rec->max_periods = val;
 			continue;
 		}
+		if (strcmp(id, "var_periodsize") == 0) {
+			err = snd_config_get_bool(n);
+			if (err < 0)
+				return err;
+			rec->var_periodsize = err;
+			continue;
+		}
 		SNDERR("Unknown field %s", id);
 		return -EINVAL;
 	}
diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h
index 611ad29..91e816c 100644
--- a/src/pcm/pcm_direct.h
+++ b/src/pcm/pcm_direct.h
@@ -147,12 +147,14 @@  struct snd_pcm_direct {
 	int tread: 1;
 	int timer_need_poll: 1;
 	unsigned int timer_events;
+	unsigned int timer_ticks;
 	int server_fd;
 	pid_t server_pid;
 	snd_timer_t *timer; 		/* timer used as poll_fd */
 	int interleaved;	 	/* we have interleaved buffer */
 	int slowptr;			/* use slow but more precise ptr updates */
 	int max_periods;		/* max periods (-1 = fixed periods, 0 = max buffer size) */
+	int var_periodsize;		/* allow variable period size if max_periods is != -1*/
 	unsigned int channels;		/* client's channels */
 	unsigned int *bindings;
 	union {
@@ -326,6 +328,7 @@  struct snd_pcm_direct_open_conf {
 	int ipc_gid;
 	int slowptr;
 	int max_periods;
+	int var_periodsize;
 	snd_config_t *slave;
 	snd_config_t *bindings;
 };
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index 825677f..0ab7323 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -1040,6 +1040,7 @@  int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
 	dmix->state = SND_PCM_STATE_OPEN;
 	dmix->slowptr = opts->slowptr;
 	dmix->max_periods = opts->max_periods;
+	dmix->var_periodsize = opts->var_periodsize;
 	dmix->sync_ptr = snd_pcm_dmix_sync_ptr;
 
  retry:
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
index 29cd6c6..a1fed5d 100644
--- a/src/pcm/pcm_dshare.c
+++ b/src/pcm/pcm_dshare.c
@@ -725,6 +725,7 @@  int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
 	dshare->state = SND_PCM_STATE_OPEN;
 	dshare->slowptr = opts->slowptr;
 	dshare->max_periods = opts->max_periods;
+	dshare->var_periodsize = opts->var_periodsize;
 	dshare->sync_ptr = snd_pcm_dshare_sync_ptr;
 
  retry:
diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
index 1aedf3c..85f0ff4 100644
--- a/src/pcm/pcm_dsnoop.c
+++ b/src/pcm/pcm_dsnoop.c
@@ -606,6 +606,7 @@  int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
 	dsnoop->state = SND_PCM_STATE_OPEN;
 	dsnoop->slowptr = opts->slowptr;
 	dsnoop->max_periods = opts->max_periods;
+	dsnoop->var_periodsize = opts->var_periodsize;
 	dsnoop->sync_ptr = snd_pcm_dsnoop_sync_ptr;
 
  retry: