Re: [PATCH 6/9] rate: handle negative values from snd_pcm_mmap_playback_hw_avail

["Alexander E. Patrakov" <patrakov@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]

15.09.2014 16:14, Takashi Iwai wrote:
> At Mon, 15 Sep 2014 16:03:57 +0600,
> Alexander E. Patrakov wrote:
>>
>> 15.09.2014 14:49, Takashi Iwai пишет:
>>> At Sun, 14 Sep 2014 00:30:18 +0600,
>>> Alexander E. Patrakov wrote:
>>>>
>>>> Such negative returns are possible during an underrun if xrun detection
>>>> is disabled.
>>>>
>>>> So, don't store the result in an unsigned variable (where it will
>>>> overflow), and postpone the trigger in such case, too.
>>>>
>>>> Signed-off-by: Alexander E. Patrakov <patrakov@gmail.com>
>>>> ---
>>>>
>>>> The patch is only compile-tested and the second hunk may well be wrong.
>>>>
>>>> There are also similar issues in pcm_share.c, but, as I don't completely
>>>> understand the code there and cannot test that plugin at all due to
>>>> unrelated crashes, there will be no patch from me.
>>>
>>> In general, hw_avail must not be negative before starting the stream.
>>> If it is, then it means most likely the driver problem, so we should
>>> return error immediately instead.
>>
>> Thanks for the review. Would -EBADFD be a correct error code, or do you
>> want something different? or maybe even an assert?
>
> I'd take either EPIPE or EBADFD.
> An assert would be an overkill, IMO.

I have sent the fix to the list, but nobody reacted. Resending as an 
attachment to this message.

-- 
Alexander E. Patrakov

[-- Attachment #2: 0001-pcm-rate-hw_avail-must-not-be-negative-before-starti.patch --]
[-- Type: text/x-patch, Size: 1079 bytes --]

>From 81d3b1b107e0f55f041eb9df8f6c3edd3e6450f4 Mon Sep 17 00:00:00 2001
From: "Alexander E. Patrakov" <patrakov@gmail.com>
Date: Mon, 15 Sep 2014 20:17:47 +0600
Subject: [PATCH] pcm, rate: hw_avail must not be negative before starting the
 stream

If it is, then it means most likely the driver problem, so we should
return error immediately instead.

Signed-off-by: Alexander E. Patrakov <patrakov@gmail.com>
---

As suggested by Takashi Iwai.

 src/pcm/pcm_rate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index 736d558..c76db25 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -1069,7 +1069,10 @@ static int snd_pcm_rate_start(snd_pcm_t *pcm)
 	gettimestamp(&rate->trigger_tstamp, pcm->tstamp_type);
 
 	avail = snd_pcm_mmap_playback_hw_avail(rate->gen.slave);
-	if (avail <= 0) {
+	if (avail < 0) /* can't happen on healthy drivers */
+		return -EBADFD;
+
+	if (avail == 0) {
 		/* postpone the trigger since we have no data committed yet */
 		rate->start_pending = 1;
 		return 0;
-- 
2.1.0


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]