[alsa-devel] [PATCH] add number of periods constraint to snd-aoa [try 3]
From: Heikki Lindholm holindho@cs.helsinki.fi
The aoa driver is not specifying constraints on number of periods, and, it seems, it might end with a non-integer number, which it cannot deal with. Fix by adding a proper constraint.
Signed-off-by: Heikki Lindholm holindho@cs.helsinki.fi --- try 3: fixed coding style
diff -r 1b54a8725ded aoa/soundbus/i2sbus/i2sbus-pcm.c --- a/aoa/soundbus/i2sbus/i2sbus-pcm.c Wed Nov 14 17:07:17 2007 +0100 +++ b/aoa/soundbus/i2sbus/i2sbus-pcm.c Fri Nov 23 14:16:07 2007 +0200 @@ -194,6 +194,12 @@ static int i2sbus_pcm_open(struct i2sbus hw->period_bytes_max = 16384; hw->periods_min = 3; hw->periods_max = MAX_DBDMA_COMMANDS; + err = snd_pcm_hw_constraint_integer(pi->substream->runtime, + SNDRV_PCM_HW_PARAM_PERIODS); + if (err < 0) { + result = err; + goto out_unlock; + } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
On Fri, 2007-11-23 at 14:25 +0200, Heikki O Lindholm wrote:
From: Heikki Lindholm holindho@cs.helsinki.fi
The aoa driver is not specifying constraints on number of periods, and, it seems, it might end with a non-integer number, which it cannot deal with. Fix by adding a proper constraint.
Signed-off-by: Heikki Lindholm holindho@cs.helsinki.fi
try 3: fixed coding style
diff -r 1b54a8725ded aoa/soundbus/i2sbus/i2sbus-pcm.c --- a/aoa/soundbus/i2sbus/i2sbus-pcm.c Wed Nov 14 17:07:17 2007 +0100 +++ b/aoa/soundbus/i2sbus/i2sbus-pcm.c Fri Nov 23 14:16:07 2007 +0200 @@ -194,6 +194,12 @@ static int i2sbus_pcm_open(struct i2sbus hw->period_bytes_max = 16384; hw->periods_min = 3; hw->periods_max = MAX_DBDMA_COMMANDS;
- err = snd_pcm_hw_constraint_integer(pi->substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
Personally, I prefer that indented to just after the opening parenthesis.
;)
- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
johannes
Johannes Berg kirjoitti:
On Fri, 2007-11-23 at 14:25 +0200, Heikki O Lindholm wrote:
From: Heikki Lindholm holindho@cs.helsinki.fi
The aoa driver is not specifying constraints on number of periods, and, it seems, it might end with a non-integer number, which it cannot deal with. Fix by adding a proper constraint.
Signed-off-by: Heikki Lindholm holindho@cs.helsinki.fi
try 3: fixed coding style
diff -r 1b54a8725ded aoa/soundbus/i2sbus/i2sbus-pcm.c --- a/aoa/soundbus/i2sbus/i2sbus-pcm.c Wed Nov 14 17:07:17 2007 +0100 +++ b/aoa/soundbus/i2sbus/i2sbus-pcm.c Fri Nov 23 14:16:07 2007 +0200 @@ -194,6 +194,12 @@ static int i2sbus_pcm_open(struct i2sbus hw->period_bytes_max = 16384; hw->periods_min = 3; hw->periods_max = MAX_DBDMA_COMMANDS;
- err = snd_pcm_hw_constraint_integer(pi->substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);Personally, I prefer that indented to just after the opening parenthesis.
;)
- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
I'd guess so. As I said, I couldn't reproduce the bug on the x86 machines I tried. Anyways, the constraint system of ALSA being quite complex and the fact that buffer size and period size/number of periods are separated in ALSA do seem a like an open invitation to bugs like this.
-- Heikki Lindholm
At Fri, 23 Nov 2007 13:49:42 +0100, Johannes Berg wrote:
- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
Which driver? It should be fixed as well, then.
thanks,
Takashi
On Fri, 2007-11-23 at 14:43 +0100, Takashi Iwai wrote:
At Fri, 23 Nov 2007 13:49:42 +0100, Johannes Berg wrote:
- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
Which driver? It should be fixed as well, then.
snd-usb-audio I think. Was there any further debugging between you two that I missed? with the dummy driver?
johannes
Johannes Berg kirjoitti:
On Fri, 2007-11-23 at 14:43 +0100, Takashi Iwai wrote:
At Fri, 23 Nov 2007 13:49:42 +0100, Johannes Berg wrote:
- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
Which driver? It should be fixed as well, then.
snd-usb-audio I think. Was there any further debugging between you two that I missed? with the dummy driver?
I sent everything to everyone :) I tried the dummy driver, and it did produce all kinds of funny results, but nothing unexpected, being that its timing source is quite inaccurate.
-- hl
At Fri, 23 Nov 2007 21:23:11 +0100, Johannes Berg wrote:
On Fri, 2007-11-23 at 14:43 +0100, Takashi Iwai wrote:
At Fri, 23 Nov 2007 13:49:42 +0100, Johannes Berg wrote:
- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
Which driver? It should be fixed as well, then.
snd-usb-audio I think.
Oh that's I didn't know of. Do you have more detailed information?
usb-audio also has no integer periods constraint, so it might suffer from the same problem, too.
Takashi
snd-usb-audio I think.
Oh that's I didn't know of. Do you have more detailed information?
Not really. I just happened to have an usb audio dongle handy and plugged it in for a quick test.
usb-audio also has no integer periods constraint, so it might suffer from the same problem, too.
Interesting. What purpose serves not having the constraint globally?
johannes
Takashi Iwai wrote:
Johannes Berg wrote:
On Fri, 2007-11-23 at 14:43 +0100, Takashi Iwai wrote:
Johannes Berg wrote:
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
Which driver? It should be fixed as well, then.
snd-usb-audio I think.
Oh that's I didn't know of. Do you have more detailed information?
usb-audio also has no integer periods constraint, so it might suffer from the same problem, too.
snd-usb-audio uses double buffering and doesn't have any requirements regarding periods.
Regards, Clemens
So what I'm not sure on... Why do we see the same behaviour with other drivers? Same bug?
Which driver? It should be fixed as well, then.
snd-usb-audio I think.
Oh that's I didn't know of. Do you have more detailed information?
usb-audio also has no integer periods constraint, so it might suffer from the same problem, too.
snd-usb-audio uses double buffering and doesn't have any requirements regarding periods.
Hm. I'm pretty sure I did see the same problem there. I guess I'll have to test that again to convince myself (and then you) :)
johannes
At Fri, 23 Nov 2007 14:25:48 +0200 (EET), Heikki O Lindholm wrote:
From: Heikki Lindholm holindho@cs.helsinki.fi
The aoa driver is not specifying constraints on number of periods, and, it seems, it might end with a non-integer number, which it cannot deal with. Fix by adding a proper constraint.
Signed-off-by: Heikki Lindholm holindho@cs.helsinki.fi
try 3: fixed coding style
Applied to HG tree (with a slight change as Johannes's liking :)
Thanks!
Takashi
diff -r 1b54a8725ded aoa/soundbus/i2sbus/i2sbus-pcm.c --- a/aoa/soundbus/i2sbus/i2sbus-pcm.c Wed Nov 14 17:07:17 2007 +0100 +++ b/aoa/soundbus/i2sbus/i2sbus-pcm.c Fri Nov 23 14:16:07 2007 +0200 @@ -194,6 +194,12 @@ static int i2sbus_pcm_open(struct i2sbus hw->period_bytes_max = 16384; hw->periods_min = 3; hw->periods_max = MAX_DBDMA_COMMANDS;
- err = snd_pcm_hw_constraint_integer(pi->substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);- if (err < 0) {
result = err;goto out_unlock;- } list_for_each_entry(cii, &sdev->codec_list, list) { if (cii->codec->open) { err = cii->codec->open(cii, pi->substream);
participants (5)
-
Clemens Ladisch -
Heikki Lindholm -
Heikki O Lindholm
-
Johannes Berg -
Takashi Iwai