[alsa-devel] [RFC][PATCH] ASoC: Move symmetric rate check for substreams to hw_params
Currently the symmetric rate check is present in the snd_pcm_open() call. However this causes snd_pcm_open() to fail for the 2nd substream is the 2nd call to snd_pcm_open() is made before the 1st substream has set a non-zero rate by calling snd_pcm_hw_params().
Fix this by moving the call to snd_pcm_apply_symmetry() to soc_pcm_hw_params().
Signed-off-by: Afzal Mohammed afzal@ti.com Signed-off-by: Vaibhav Bedia vaibhav.bedia@ti.com ---
This patch is one way of fixing the issues reported here http://comments.gmane.org/gmane.linux.alsa.devel/75489 and http://thread.gmane.org/gmane.linux.alsa.devel/81055/focus=21653
sound/soc/soc-core.c | 25 +++++++++++++++++-------- 1 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 64befac..edec9af 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -638,13 +638,6 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) goto config_err; }
- /* Symmetry only applies if we've already got an active stream. */ - if (cpu_dai->active || codec_dai->active) { - ret = soc_pcm_apply_symmetry(substream); - if (ret != 0) - goto config_err; - } - pr_debug("asoc: %s <-> %s info:\n", codec_dai->name, cpu_dai->name); pr_debug("asoc: rate mask 0x%x\n", runtime->hw.rates); @@ -742,6 +735,9 @@ static int soc_codec_close(struct snd_pcm_substream *substream) codec_dai->active--; codec->active--;
+ if(!codec_dai->active && !cpu_dai->active) + rtd->rate = 0; + /* Muting the DAC suppresses artifacts caused during digital * shutdown, for example from stopping clocks. */ @@ -898,12 +894,25 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } }
- rtd->rate = params_rate(params); + /* Symmetry only applies if we've already got an active stream, + a non zero rtd->rate implies that stream was already active and + rtd->rate has been set previously */ + if (rtd->rate) { + ret = soc_pcm_apply_symmetry(substream); + if (ret < 0) + goto symmetry_err; + } else { + rtd->rate = params_rate(params); + }
out: mutex_unlock(&pcm_mutex); return ret;
+symmetry_err: + if(platform->driver->ops->hw_free) + platform->driver->ops->hw_free(substream); + platform_err: if (cpu_dai->driver->ops->hw_free) cpu_dai->driver->ops->hw_free(substream, cpu_dai);
On Mon, Mar 07, 2011 at 05:26:03PM +0530, Vaibhav Bedia wrote:
Currently the symmetric rate check is present in the snd_pcm_open() call. However this causes snd_pcm_open() to fail for the 2nd substream is the 2nd call to snd_pcm_open() is made before the 1st substream has set a non-zero rate by calling snd_pcm_hw_params().
Fix this by moving the call to snd_pcm_apply_symmetry() to soc_pcm_hw_params().
This means that applications won't be constrained by the current settings so can't automatically choose the required rate. ALSA is just fundamentally racy here, if you want to reliably open bidirectional streams you need to serialise the startup of the two directions.
Hi Mark,
On Monday, March 07, 2011 5:39 PM, Mark Brown wrote:
On Mon, Mar 07, 2011 at 05:26:03PM +0530, Vaibhav Bedia wrote:
Currently the symmetric rate check is present in the snd_pcm_open() call. However this causes snd_pcm_open() to fail for the 2nd substream is the 2nd call to snd_pcm_open() is made before the 1st substream has set a non-zero rate by calling snd_pcm_hw_params().
Fix this by moving the call to snd_pcm_apply_symmetry() to soc_pcm_hw_params().
This means that applications won't be constrained by the current settings so can't automatically choose the required rate. ALSA is just fundamentally racy here, if you want to reliably open bidirectional streams you need to serialise the startup of the two directions.
When a custom application is written then the race condition can be avoided.
However right now a simple test like "arecord -f dat | aplay -f dat" fails. One way to make it work is to make sure that a call to arecord/aplay (or some dummy app which just sets some valid rate and returns) has been made before both are invoked simultaneously.
Regards, Vaibhav
On Mon, Mar 07, 2011 at 06:21:32PM +0530, Bedia, Vaibhav wrote:
On Monday, March 07, 2011 5:39 PM, Mark Brown wrote:
This means that applications won't be constrained by the current settings so can't automatically choose the required rate. ALSA is just fundamentally racy here, if you want to reliably open bidirectional streams you need to serialise the startup of the two directions.
When a custom application is written then the race condition can be avoided.
However right now a simple test like "arecord -f dat | aplay -f dat" fails. One way to make it work is to make sure that a call to arecord/aplay (or some dummy app which just sets some valid rate and returns) has been made before both are invoked simultaneously.
For command line testing you can do something like:
arecord -f dat | (sleep 1; aplay -f dat -)
to insert a delay if required. Obviously not all systems will hit the race.
participants (3)
-
Bedia, Vaibhav
-
Mark Brown
-
Vaibhav Bedia