From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 183D0C432BE for ; Tue, 24 Aug 2021 08:13:54 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B554C6125F for ; Tue, 24 Aug 2021 08:13:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B554C6125F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 0A5D884A; Tue, 24 Aug 2021 10:13:01 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 0A5D884A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1629792831; bh=/jTQiM15hGHds78+T+SjbreWiGpjU6SYo9CZn+6oAm8=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=tPhnA3kjssaZCT6EHHdGlDy4738EncQl0TKChRCD1oKydelX5zWHNb0+ZtqWXJkvb WNno7kazd44pM1hJIcTFi4fbPgkQblMmssZ62x//a7yd1hdZccFcU9pzIp9+bQV+Zs B+95l1XeOe9TPZQAtQEWiGV3Zb5gy0nJdLRJ6RUk= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 71CF1F801D5; Tue, 24 Aug 2021 10:13:00 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id B14B2F801D8; Tue, 24 Aug 2021 10:12:58 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 594E3F800AE for ; Tue, 24 Aug 2021 10:12:53 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 594E3F800AE Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="wmc6LN5M"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="EaAmUVec" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 3E65120048; Tue, 24 Aug 2021 08:12:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1629792772; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=p9sZP1YVMTWDd+QkGlDT4t5DlwUwdDbgOZUeMJpE1Y4=; b=wmc6LN5MBBhpH4fdhTv5oY/p8aPyh1zxXWXXFocOz0cLDADQWpTWW7yfRGHbGkYZMhQNHZ I642G0DRQ1mrDAIpDdIsMPTQOxsqkfih0e8zNRmmFVecAjq7aJrKU6v2DAOuy6dwS0ustx UMrT6enTVvz7ye1bVDHWXWqxJjZBTpw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1629792772; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=p9sZP1YVMTWDd+QkGlDT4t5DlwUwdDbgOZUeMJpE1Y4=; b=EaAmUVecqGDVZBDshHvFFyo8MPRoPQp1UtY4FXtKUtWwEdbGNrfias4MKKgI3h4kl74qYs 7FIDpMzqx3GcCEDg== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 3207DA3BBA; Tue, 24 Aug 2021 08:12:52 +0000 (UTC) Date: Tue, 24 Aug 2021 10:12:52 +0200 Message-ID: From: Takashi Iwai To: sujith kumar reddy Subject: Re: arecord is failing with -V stereo In-Reply-To: References: <20210819183450.GV890690@belle.intranet.vanheusden.com> <20210819190634.GW890690@belle.intranet.vanheusden.com> <20210823170332.GD890690@belle.intranet.vanheusden.com> <20210823184727.GE890690@belle.intranet.vanheusden.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: multipart/mixed; boundary="Multipart_Tue_Aug_24_10:12:52_2021-1" Cc: alsa-devel@alsa-project.org, folkert X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" --Multipart_Tue_Aug_24_10:12:52_2021-1 Content-Type: text/plain; charset=US-ASCII On Mon, 23 Aug 2021 21:06:05 +0200, sujith kumar reddy wrote: > > Hi Folkert, > > Tried with the above code.This is also getting p value negative. > > My point is p is negative it doesn't go to > > if (p >= bar_length) > p = bar_length - 1; > it is going to memset second argument p,that is negative so crashing. Gah, the code there contains lots of craps. A negative value must not have been passed there. Below is a series of fixes. Please give it a try. thanks, Takashi --Multipart_Tue_Aug_24_10:12:52_2021-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0001-aplay-Fix-conversion-of-unsigned-samples-in-peak-cal.patch" Content-Transfer-Encoding: 7bit >From 0ea7bfea83d97fefd18845948350322017a865c2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 Aug 2021 09:00:40 +0200 Subject: [PATCH 1/5] aplay: Fix conversion of unsigned samples in peak calculation The XOR with the mask has to be applied before calculating abs value. Signed-off-by: Takashi Iwai --- aplay/aplay.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/aplay/aplay.c b/aplay/aplay.c index cc51dcb48bba..91af244edb60 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -1828,7 +1828,8 @@ static void compute_max_peak(u_char *data, size_t samples) sval = le16toh(*valp); else sval = be16toh(*valp); - sval = abs(sval) ^ mask; + sval ^= mask; + sval = abs(sval); if (max_peak[c] < sval) max_peak[c] = sval; valp++; @@ -1848,11 +1849,12 @@ static void compute_max_peak(u_char *data, size_t samples) } else { val = (valp[0]<<16) | (valp[1]<<8) | valp[2]; } + val ^= mask; /* Correct signed bit in 32-bit value */ if (val & (1<<(bits_per_sample-1))) { val |= 0xff<<24; /* Negate upper bits too */ } - val = abs(val) ^ mask; + val = abs(val); if (max_peak[c] < val) max_peak[c] = val; valp += 3; @@ -1871,7 +1873,8 @@ static void compute_max_peak(u_char *data, size_t samples) val = le32toh(*valp); else val = be32toh(*valp); - val = abs(val) ^ mask; + val ^= mask; + val = abs(val); if (max_peak[c] < val) max_peak[c] = val; valp++; -- 2.26.2 --Multipart_Tue_Aug_24_10:12:52_2021-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0002-aplay-Handle-16bit-sample-negative-overflow-in-peak-.patch" Content-Transfer-Encoding: 7bit >From 5c4bf63a94ed0c20aca5bafb94ecd05893a45ec1 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 Aug 2021 09:36:33 +0200 Subject: [PATCH 2/5] aplay: Handle 16bit sample negative overflow in peak calculations The handling of 16bit samples in the peak calculations has a bug when a sample with 0x8000 is passed. As abs() treats 32bit int, it returns 0x8000. And yet the code stores back into 16bit value again. To fix that overflow, use 32bit value (i.e. val instead of sval) for the further calculations. Signed-off-by: Takashi Iwai --- aplay/aplay.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aplay/aplay.c b/aplay/aplay.c index 91af244edb60..c884346c9f25 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -1829,9 +1829,9 @@ static void compute_max_peak(u_char *data, size_t samples) else sval = be16toh(*valp); sval ^= mask; - sval = abs(sval); - if (max_peak[c] < sval) - max_peak[c] = sval; + val = abs(sval); + if (max_peak[c] < val) + max_peak[c] = val; valp++; if (vumeter == VUMETER_STEREO) c = !c; -- 2.26.2 --Multipart_Tue_Aug_24_10:12:52_2021-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0003-aplay-Don-t-pass-most-negative-integer-to-abs-in-pea.patch" Content-Transfer-Encoding: 7bit >From d9b31338153591944d72e62523bad7850b407c63 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 Aug 2021 09:58:29 +0200 Subject: [PATCH 3/5] aplay: Don't pass most negative integer to abs() in peak calculations The return value from abs() for the most negative integer is undefined. Cap it properly for the 32bit sample handling. Signed-off-by: Takashi Iwai --- aplay/aplay.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/aplay/aplay.c b/aplay/aplay.c index c884346c9f25..2543de5b6cd8 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -1874,7 +1874,10 @@ static void compute_max_peak(u_char *data, size_t samples) else val = be32toh(*valp); val ^= mask; - val = abs(val); + if (val == 0x80000000U) + val = 0x7fffffff; + else + val = abs(val); if (max_peak[c] < val) max_peak[c] = val; valp++; -- 2.26.2 --Multipart_Tue_Aug_24_10:12:52_2021-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0004-aplay-Handle-upper-bound-in-peak-calculations.patch" Content-Transfer-Encoding: 7bit >From 2efe124c31360cf0156dd0e5e7cdd52d1346a5c0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 Aug 2021 10:00:26 +0200 Subject: [PATCH 4/5] aplay: Handle upper bound in peak calculations Make sure that the calculated max_peak[] won't go beyond the sample max resolution. Signed-off-by: Takashi Iwai --- aplay/aplay.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/aplay/aplay.c b/aplay/aplay.c index 2543de5b6cd8..a51a37ba34bd 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -1898,6 +1898,8 @@ static void compute_max_peak(u_char *data, size_t samples) max = 0x7fffffff; for (c = 0; c < ichans; c++) { + if (max_peak[c] > max) + max_peak[c] = max; if (bits_per_sample > 16) perc[c] = max_peak[c] / (max / 100); else -- 2.26.2 --Multipart_Tue_Aug_24_10:12:52_2021-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0005-aplay-Fix-out-of-bound-access-in-stereo-VU-meter-dra.patch" Content-Transfer-Encoding: 7bit >From dea51861a8626694c6e80121c17a0a38efc2e33c Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 Aug 2021 10:06:05 +0200 Subject: [PATCH 5/5] aplay: Fix out-of-bound access in stereo VU meter drawing The left channel drawing of a stereo VU meter has a bug where it may access a negative array index. Signed-off-by: Takashi Iwai --- aplay/aplay.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/aplay/aplay.c b/aplay/aplay.c index a51a37ba34bd..63a4e3437fd9 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -1758,10 +1758,12 @@ static void print_vu_meter_stereo(int *perc, int *maxperc) if (c) memset(line + bar_length + 6 + 1, '#', p); else - memset(line + bar_length - p - 1, '#', p); - p = maxperc[c] * bar_length / 100; - if (p > bar_length) - p = bar_length; + memset(line + bar_length - p, '#', p); + p = maxperc[c] * bar_length / 100 - 1; + if (p < 0) + p = 0; + else if (p >= bar_length) + p = bar_length - 1; if (c) line[bar_length + 6 + 1 + p] = '+'; else -- 2.26.2 --Multipart_Tue_Aug_24_10:12:52_2021-1--