[v2] pcm: null: Do not allow a period size of 0

Commit Message

Timo Wischer April 8, 2019, 8:55 a.m. UTC

From: Timo Wischer <[e-mail]>

Some applications do not expect that get_period_size_min() could
return 0. Therefore these applications cannot use the null plugin without
this patch.
Due to there is no use case for having a period size of 0 this patch
disallows a period size of 0 when using the null plugin.

Signed-off-by: Timo Wischer <[e-mail]>
---

Hi Takashi,

> Why 32?
I thought there is also no use case for a period of 1 frame. Therefore I
increased it to a 2^n value to also lower the CPU usage in case of null
plugin.

> Or does it cause a problem?
I do not see any issues here. But for example portaudio will automatically
choose a period size of 1 frame which will result in a higher CPU load.

But anyway I would also be fine with this higher CPU load if you prefer
this patch.

Best regards

Timo

 src/pcm/pcm_null.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Takashi Iwai April 8, 2019, 12:28 p.m. UTC | #1

On Mon, 08 Apr 2019 10:55:54 +0200,
<[e-mail]> wrote:
> 
> From: Timo Wischer <[e-mail]>
> 
> Some applications do not expect that get_period_size_min() could
> return 0. Therefore these applications cannot use the null plugin without
> this patch.
> Due to there is no use case for having a period size of 0 this patch
> disallows a period size of 0 when using the null plugin.
> 
> Signed-off-by: Timo Wischer <[e-mail]>
> ---
> 
> Hi Takashi,
> 
> > Why 32?
> I thought there is also no use case for a period of 1 frame. Therefore I
> increased it to a 2^n value to also lower the CPU usage in case of null
> plugin.
> 
> > Or does it cause a problem?
> I do not see any issues here. But for example portaudio will automatically
> choose a period size of 1 frame which will result in a higher CPU load.
> 
> But anyway I would also be fine with this higher CPU load if you prefer
> this patch.

Well, this is rather a bug in portaudio, then.  Clearly it's nonsense
to return zero for the minimal size, but any positive number can be
seen as a theoretically valid number.  This doesn't mean that any
value is usable for the practical use case, because such parameters
strongly depend on the other components like the CPU speed, memory
speed, whatever.  From the same reason, this can't be fixed from the
driver or the plugin itself.

So, I took now your v2 patch as is.  The rest should be a tuning
issue.


thanks,

Takashi

Patch

diff --git a/src/pcm/pcm_null.c b/src/pcm/pcm_null.c
index ff61624..1d81548 100644
--- a/src/pcm/pcm_null.c
+++ b/src/pcm/pcm_null.c
@@ -261,7 +261,17 @@  static snd_pcm_sframes_t snd_pcm_null_mmap_commit(snd_pcm_t *pcm,
 
 static int snd_pcm_null_hw_refine(snd_pcm_t *pcm ATTRIBUTE_UNUSED, snd_pcm_hw_params_t *params)
 {
-	int err = snd_pcm_hw_refine_soft(pcm, params);
+	int err;
+
+	/* Do not return a period size of 0 because for example portaudio cannot
+	 * handle it.
+	 */
+	err = _snd_pcm_hw_param_set_min(params, SND_PCM_HW_PARAM_PERIOD_SIZE, 1,
+					0);
+	if (err < 0)
+		return err;
+
+	err = snd_pcm_hw_refine_soft(pcm, params);
 	params->info = SND_PCM_INFO_MMAP | SND_PCM_INFO_MMAP_VALID |
 		       SND_PCM_INFO_RESUME | SND_PCM_INFO_PAUSE;
 	params->fifo_size = 0;