* [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations
@ 2021-08-24 9:47 Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 1/5] aplay: Fix conversion of unsigned samples in peak calculation Takashi Iwai
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-08-24 9:47 UTC (permalink / raw)
To: alsa-devel
The code in aplay for sample peak calculations and VU-meter
representation seems to have a lot of bugs. Let's fix them.
Takashi
===
Takashi Iwai (5):
aplay: Fix conversion of unsigned samples in peak calculation
aplay: Handle 16bit sample negative overflow in peak calculations
aplay: Don't pass most negative integer to abs() in peak calculations
aplay: Handle upper bound in peak calculations
aplay: Fix out-of-bound access in stereo VU meter drawing
aplay/aplay.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH alsa-utils 1/5] aplay: Fix conversion of unsigned samples in peak calculation
2021-08-24 9:47 [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations Takashi Iwai
@ 2021-08-24 9:47 ` Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 2/5] aplay: Handle 16bit sample negative overflow in peak calculations Takashi Iwai
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-08-24 9:47 UTC (permalink / raw)
To: alsa-devel
The XOR with the mask has to be applied before calculating abs value.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH alsa-utils 2/5] aplay: Handle 16bit sample negative overflow in peak calculations
2021-08-24 9:47 [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 1/5] aplay: Fix conversion of unsigned samples in peak calculation Takashi Iwai
@ 2021-08-24 9:47 ` Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 3/5] aplay: Don't pass most negative integer to abs() " Takashi Iwai
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-08-24 9:47 UTC (permalink / raw)
To: alsa-devel
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 <tiwai@suse.de>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH alsa-utils 3/5] aplay: Don't pass most negative integer to abs() in peak calculations
2021-08-24 9:47 [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 1/5] aplay: Fix conversion of unsigned samples in peak calculation Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 2/5] aplay: Handle 16bit sample negative overflow in peak calculations Takashi Iwai
@ 2021-08-24 9:47 ` Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 4/5] aplay: Handle upper bound " Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 5/5] aplay: Fix out-of-bound access in stereo VU meter drawing Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-08-24 9:47 UTC (permalink / raw)
To: alsa-devel
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 <tiwai@suse.de>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH alsa-utils 4/5] aplay: Handle upper bound in peak calculations
2021-08-24 9:47 [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations Takashi Iwai
` (2 preceding siblings ...)
2021-08-24 9:47 ` [PATCH alsa-utils 3/5] aplay: Don't pass most negative integer to abs() " Takashi Iwai
@ 2021-08-24 9:47 ` Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 5/5] aplay: Fix out-of-bound access in stereo VU meter drawing Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-08-24 9:47 UTC (permalink / raw)
To: alsa-devel
Make sure that the calculated max_peak[] won't go beyond the sample
max resolution.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH alsa-utils 5/5] aplay: Fix out-of-bound access in stereo VU meter drawing
2021-08-24 9:47 [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations Takashi Iwai
` (3 preceding siblings ...)
2021-08-24 9:47 ` [PATCH alsa-utils 4/5] aplay: Handle upper bound " Takashi Iwai
@ 2021-08-24 9:47 ` Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-08-24 9:47 UTC (permalink / raw)
To: alsa-devel
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 <tiwai@suse.de>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-24 9:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 9:47 [PATCH alsa-utils 0/5] aplay: Fix bugs in peak calculations Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 1/5] aplay: Fix conversion of unsigned samples in peak calculation Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 2/5] aplay: Handle 16bit sample negative overflow in peak calculations Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 3/5] aplay: Don't pass most negative integer to abs() " Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 4/5] aplay: Handle upper bound " Takashi Iwai
2021-08-24 9:47 ` [PATCH alsa-utils 5/5] aplay: Fix out-of-bound access in stereo VU meter drawing Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.