All of lore.kernel.org
 help / color / mirror / Atom feed
* arecord is failing with -V stereo
@ 2021-08-19 18:04 sujith kumar reddy
  2021-08-19 18:34 ` folkert
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-19 18:04 UTC (permalink / raw)
  To: alsa-devel

Hi All,

arecord is  failing vumeter option -V stereo only.

localhost ~ # arecord -Dhw:1,2 -r48000 -c2 -fS32_LE /tmp/test_record.wav -M
-d 1 -V stereo
Recording WAVE '/tmp/test_record.wav' : Signed 32 bit Little Endian, Rate
48000 Hz, Stereo
*** buffer overflow detected ***: terminated
Aborted by signal Aborted...

Please provide pointers to debug this option.

Thanks
Sujith

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-19 18:04 arecord is failing with -V stereo sujith kumar reddy
@ 2021-08-19 18:34 ` folkert
  2021-08-19 18:43   ` sujith kumar reddy
  0 siblings, 1 reply; 18+ messages in thread
From: folkert @ 2021-08-19 18:34 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel

> arecord is  failing vumeter option -V stereo only.

What version?
Because the commandline you provide works fine here with 1.2.4-1ubuntu3.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-19 18:34 ` folkert
@ 2021-08-19 18:43   ` sujith kumar reddy
  2021-08-19 19:06     ` folkert
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-19 18:43 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

1.2.1.2

On Fri, 20 Aug, 2021, 12:04 AM folkert, <folkert@vanheusden.com> wrote:

> > arecord is  failing vumeter option -V stereo only.
>
> What version?
> Because the commandline you provide works fine here with 1.2.4-1ubuntu3.
>
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-19 18:43   ` sujith kumar reddy
@ 2021-08-19 19:06     ` folkert
  2021-08-23 16:31       ` sujith kumar reddy
  0 siblings, 1 reply; 18+ messages in thread
From: folkert @ 2021-08-19 19:06 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel

> > > arecord is  failing vumeter option -V stereo only.
> >
> > What version?
> > Because the commandline you provide works fine here with 1.2.4-1ubuntu3.
>
> 1.2.1.2

See what happens if you upgrade to 1.2.4.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-19 19:06     ` folkert
@ 2021-08-23 16:31       ` sujith kumar reddy
  2021-08-23 17:03         ` folkert
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-23 16:31 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

Hi Folkert,

In all version it is crashing.
when i debug these i found that p value is coming negative at memset.
the function snippet is:
in aplay.c file in alsa-utils.

in  the below function for  c=0 it is working when c incremented to 1 the p
value is becoming negative . so memset is crashing.

static void print_vu_meter_stereo(int *perc, int *maxperc)
{
        const int bar_length = 35;
        char line[80];
        int c;

        memset(line, ' ', sizeof(line) - 1);
        line[bar_length + 3] = '|';

        for (c = 0; c < 2; c++) {
                int p = perc[c] * bar_length / 100;
                char tmp[4];
                if (p > bar_length)
                        p = bar_length;
                if (c)
                        memset(line + bar_length + 6 + 1, '#', p);
----------------//this is the line where it is crashing.here p value is
becoming negative at c=1.As we see if we change the p value to bar_length
it works fine ..As it is a player issue not a driver issue.


                else
                        memset(line + bar_length - p - 1, '#', p);
                p = maxperc[c] * bar_length / 100;
                if (p > bar_length)
                        p = bar_length;
                if (c)
                        line[bar_length + 6 + 1 + p] = '+';
                else
                        line[bar_length - p - 1] = '+';
                if (maxperc[c] > 99)
                        sprintf(tmp, "MAX");
                else
                        sprintf(tmp, "%02d%%", maxperc[c]);
                if (c)
                        memcpy(line + bar_length + 3 + 1, tmp, 3);
                else
                        memcpy(line + bar_length, tmp, 3);
        }
        line[bar_length * 2 + 6 + 2] = 0;
        fputs(line, stderr);
}
Could you please help on this.As the driver data is fine when we keep p
value to bar length and checked .
.


Thanks
Sujith





On Fri, Aug 20, 2021 at 12:36 AM folkert <folkert@vanheusden.com> wrote:

> > > > arecord is  failing vumeter option -V stereo only.
> > >
> > > What version?
> > > Because the commandline you provide works fine here with
> 1.2.4-1ubuntu3.
> >
> > 1.2.1.2
>
> See what happens if you upgrade to 1.2.4.
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 16:31       ` sujith kumar reddy
@ 2021-08-23 17:03         ` folkert
  2021-08-23 18:44           ` sujith kumar reddy
  0 siblings, 1 reply; 18+ messages in thread
From: folkert @ 2021-08-23 17:03 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel

>         const int bar_length = 35;
>         char line[80];
...
>                 if (p > bar_length)
>                         p = bar_length;
>                 if (c)
>                         memset(line + bar_length + 6 + 1, '#', p);
> ----------------//this is the line where it is crashing.here p value is
> becoming negative at c=1.As we see if we change the p value to bar_length
> it works fine ..As it is a player issue not a driver issue.

This is puzzling.
bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that fits
easlity in 80.

But wait:

                        line[bar_length - p - 1] = '+';

p is max bar_length, so we would end up putting '+' in line[-1]

Can you try this?


diff --git a/aplay/aplay.c b/aplay/aplay.c
index cc51dcb..9cfd52c 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
 			p = bar_length;
 		if (c)
 			line[bar_length + 6 + 1 + p] = '+';
-		else
+		else if (p < bar_length)
 			line[bar_length - p - 1] = '+';
 		if (ABS(maxperc[c]) > 99)
 			sprintf(tmp, "MAX");

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 17:03         ` folkert
@ 2021-08-23 18:44           ` sujith kumar reddy
  2021-08-23 18:47             ` folkert
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-23 18:44 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

Tried with the above code.This is also getting p value negative.

snippet code:

   fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
                if (p > bar_length)
                        p = bar_length;
                if (c)
                        memset(line + bar_length + 6 + 1, '#', p);
                else
                        memset(line + bar_length - p - 1, '#', p);


logs....
//  c=0 case

print_vu_meter_stereo sujith 35  ///value of p is 35
c =1 case

 print_vu_meter_stereo sujith -9227433 ///value is negative//

On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com> wrote:

> >         const int bar_length = 35;
> >         char line[80];
> ...
> >                 if (p > bar_length)
> >                         p = bar_length;
> >                 if (c)
> >                         memset(line + bar_length + 6 + 1, '#', p);
> > ----------------//this is the line where it is crashing.here p value is
> > becoming negative at c=1.As we see if we change the p value to bar_length
> > it works fine ..As it is a player issue not a driver issue.
>
> This is puzzling.
> bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that fits
> easlity in 80.
>
> But wait:
>
>                         line[bar_length - p - 1] = '+';
>
> p is max bar_length, so we would end up putting '+' in line[-1]
>
> Can you try this?
>
>
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index cc51dcb..9cfd52c 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc, int
> *maxperc)
>                         p = bar_length;
>                 if (c)
>                         line[bar_length + 6 + 1 + p] = '+';
> -               else
> +               else if (p < bar_length)
>                         line[bar_length - p - 1] = '+';
>                 if (ABS(maxperc[c]) > 99)
>                         sprintf(tmp, "MAX");
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 18:44           ` sujith kumar reddy
@ 2021-08-23 18:47             ` folkert
  2021-08-23 19:06               ` sujith kumar reddy
  0 siblings, 1 reply; 18+ messages in thread
From: folkert @ 2021-08-23 18:47 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel

Ah same bug.
Please try this.
The idea is that in both cases the extra '-1' breaks it.

diff --git a/aplay/aplay.c b/aplay/aplay.c
index cc51dcb..47db7e7 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
 	for (c = 0; c < 2; c++) {
 		int p = perc[c] * bar_length / 100;
 		char tmp[4];
-		if (p > bar_length)
-			p = bar_length;
+		if (p >= bar_length)
+			p = bar_length - 1;
 		if (c)
 			memset(line + bar_length + 6 + 1, '#', p);
 		else


On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy wrote:
> Tried with the above code.This is also getting p value negative.
> 
> snippet code:
> 
>    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
>                 if (p > bar_length)
>                         p = bar_length;
>                 if (c)
>                         memset(line + bar_length + 6 + 1, '#', p);
>                 else
>                         memset(line + bar_length - p - 1, '#', p);
> 
> 
> logs....
> //  c=0 case
> 
> print_vu_meter_stereo sujith 35  ///value of p is 35
> c =1 case
> 
>  print_vu_meter_stereo sujith -9227433 ///value is negative//
> 
> On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com> wrote:
> 
> > >         const int bar_length = 35;
> > >         char line[80];
> > ...
> > >                 if (p > bar_length)
> > >                         p = bar_length;
> > >                 if (c)
> > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > ----------------//this is the line where it is crashing.here p value is
> > > becoming negative at c=1.As we see if we change the p value to bar_length
> > > it works fine ..As it is a player issue not a driver issue.
> >
> > This is puzzling.
> > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that fits
> > easlity in 80.
> >
> > But wait:
> >
> >                         line[bar_length - p - 1] = '+';
> >
> > p is max bar_length, so we would end up putting '+' in line[-1]
> >
> > Can you try this?
> >
> >
> > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > index cc51dcb..9cfd52c 100644
> > --- a/aplay/aplay.c
> > +++ b/aplay/aplay.c
> > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc, int
> > *maxperc)
> >                         p = bar_length;
> >                 if (c)
> >                         line[bar_length + 6 + 1 + p] = '+';
> > -               else
> > +               else if (p < bar_length)
> >                         line[bar_length - p - 1] = '+';
> >                 if (ABS(maxperc[c]) > 99)
> >                         sprintf(tmp, "MAX");
> >

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 18:47             ` folkert
@ 2021-08-23 19:06               ` sujith kumar reddy
  2021-08-23 19:08                 ` folkert
  2021-08-24  8:12                 ` Takashi Iwai
  0 siblings, 2 replies; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-23 19:06 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

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.


logs:
localhost ~ # ./sujith/arecord -D hw:1,2 -f S32_LE -r 48000 -c 2 1.wav -V s
Sujith entry: main
Recording WAVE '1.wav' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo

 print_vu_meter_stereo sujith 35  ////////////////////////// c=0 case
 print_vu_meter_stereo sujith* -9227433* /////////////////////c=1 case p
value negative which is going to memset second argument.

Thanks
Sujith



On Tue, Aug 24, 2021 at 12:17 AM folkert <folkert@vanheusden.com> wrote:

> Ah same bug.
> Please try this.
> The idea is that in both cases the extra '-1' breaks it.
>
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index cc51dcb..47db7e7 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int *perc, int
> *maxperc)
>         for (c = 0; c < 2; c++) {
>                 int p = perc[c] * bar_length / 100;
>                 char tmp[4];
> -               if (p > bar_length)
> -                       p = bar_length;
> +               if (p >= bar_length)
> +                       p = bar_length - 1;
>                 if (c)
>                         memset(line + bar_length + 6 + 1, '#', p);
>                 else
>
>
> On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy wrote:
> > Tried with the above code.This is also getting p value negative.
> >
> > snippet code:
> >
> >    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
> >                 if (p > bar_length)
> >                         p = bar_length;
> >                 if (c)
> >                         memset(line + bar_length + 6 + 1, '#', p);
> >                 else
> >                         memset(line + bar_length - p - 1, '#', p);
> >
> >
> > logs....
> > //  c=0 case
> >
> > print_vu_meter_stereo sujith 35  ///value of p is 35
> > c =1 case
> >
> >  print_vu_meter_stereo sujith -9227433 ///value is negative//
> >
> > On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com> wrote:
> >
> > > >         const int bar_length = 35;
> > > >         char line[80];
> > > ...
> > > >                 if (p > bar_length)
> > > >                         p = bar_length;
> > > >                 if (c)
> > > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > > ----------------//this is the line where it is crashing.here p value
> is
> > > > becoming negative at c=1.As we see if we change the p value to
> bar_length
> > > > it works fine ..As it is a player issue not a driver issue.
> > >
> > > This is puzzling.
> > > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that fits
> > > easlity in 80.
> > >
> > > But wait:
> > >
> > >                         line[bar_length - p - 1] = '+';
> > >
> > > p is max bar_length, so we would end up putting '+' in line[-1]
> > >
> > > Can you try this?
> > >
> > >
> > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > index cc51dcb..9cfd52c 100644
> > > --- a/aplay/aplay.c
> > > +++ b/aplay/aplay.c
> > > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc, int
> > > *maxperc)
> > >                         p = bar_length;
> > >                 if (c)
> > >                         line[bar_length + 6 + 1 + p] = '+';
> > > -               else
> > > +               else if (p < bar_length)
> > >                         line[bar_length - p - 1] = '+';
> > >                 if (ABS(maxperc[c]) > 99)
> > >                         sprintf(tmp, "MAX");
> > >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 19:06               ` sujith kumar reddy
@ 2021-08-23 19:08                 ` folkert
  2021-08-23 19:33                   ` sujith kumar reddy
  2021-08-24  8:12                 ` Takashi Iwai
  1 sibling, 1 reply; 18+ messages in thread
From: folkert @ 2021-08-23 19:08 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel

:-)

If this also doesn't work, then I give up.

diff --git a/aplay/aplay.c b/aplay/aplay.c
index cc51dcb..03a4f73 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -1753,8 +1753,10 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
 	for (c = 0; c < 2; c++) {
 		int p = perc[c] * bar_length / 100;
 		char tmp[4];
-		if (p > bar_length)
-			p = bar_length;
+		if (p >= bar_length)
+			p = bar_length - 1;
+		if (p < 0)
+			p = 0;
 		if (c)
 			memset(line + bar_length + 6 + 1, '#', p);
 		else

On Tue, Aug 24, 2021 at 12:36:05AM +0530, 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.
> 
> 
> logs:
> localhost ~ # ./sujith/arecord -D hw:1,2 -f S32_LE -r 48000 -c 2 1.wav -V s
> Sujith entry: main
> Recording WAVE '1.wav' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo
> 
>  print_vu_meter_stereo sujith 35  ////////////////////////// c=0 case
>  print_vu_meter_stereo sujith* -9227433* /////////////////////c=1 case p
> value negative which is going to memset second argument.
> 
> Thanks
> Sujith
> 
> 
> 
> On Tue, Aug 24, 2021 at 12:17 AM folkert <folkert@vanheusden.com> wrote:
> 
> > Ah same bug.
> > Please try this.
> > The idea is that in both cases the extra '-1' breaks it.
> >
> > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > index cc51dcb..47db7e7 100644
> > --- a/aplay/aplay.c
> > +++ b/aplay/aplay.c
> > @@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int *perc, int
> > *maxperc)
> >         for (c = 0; c < 2; c++) {
> >                 int p = perc[c] * bar_length / 100;
> >                 char tmp[4];
> > -               if (p > bar_length)
> > -                       p = bar_length;
> > +               if (p >= bar_length)
> > +                       p = bar_length - 1;
> >                 if (c)
> >                         memset(line + bar_length + 6 + 1, '#', p);
> >                 else
> >
> >
> > On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy wrote:
> > > Tried with the above code.This is also getting p value negative.
> > >
> > > snippet code:
> > >
> > >    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
> > >                 if (p > bar_length)
> > >                         p = bar_length;
> > >                 if (c)
> > >                         memset(line + bar_length + 6 + 1, '#', p);
> > >                 else
> > >                         memset(line + bar_length - p - 1, '#', p);
> > >
> > >
> > > logs....
> > > //  c=0 case
> > >
> > > print_vu_meter_stereo sujith 35  ///value of p is 35
> > > c =1 case
> > >
> > >  print_vu_meter_stereo sujith -9227433 ///value is negative//
> > >
> > > On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com> wrote:
> > >
> > > > >         const int bar_length = 35;
> > > > >         char line[80];
> > > > ...
> > > > >                 if (p > bar_length)
> > > > >                         p = bar_length;
> > > > >                 if (c)
> > > > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > > > ----------------//this is the line where it is crashing.here p value
> > is
> > > > > becoming negative at c=1.As we see if we change the p value to
> > bar_length
> > > > > it works fine ..As it is a player issue not a driver issue.
> > > >
> > > > This is puzzling.
> > > > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that fits
> > > > easlity in 80.
> > > >
> > > > But wait:
> > > >
> > > >                         line[bar_length - p - 1] = '+';
> > > >
> > > > p is max bar_length, so we would end up putting '+' in line[-1]
> > > >
> > > > Can you try this?
> > > >
> > > >
> > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > index cc51dcb..9cfd52c 100644
> > > > --- a/aplay/aplay.c
> > > > +++ b/aplay/aplay.c
> > > > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc, int
> > > > *maxperc)
> > > >                         p = bar_length;
> > > >                 if (c)
> > > >                         line[bar_length + 6 + 1 + p] = '+';
> > > > -               else
> > > > +               else if (p < bar_length)
> > > >                         line[bar_length - p - 1] = '+';
> > > >                 if (ABS(maxperc[c]) > 99)
> > > >                         sprintf(tmp, "MAX");
> > > >
> >


Folkert van Heusden

-- 
Multitail es una herramienta flexible que permite visualizar los "log
file" y seguir la ejecución de comandos. Permite filtrar, añadir
colores, combinar archivos, la visualización de diferencias (diff-
view), etc.  http://www.vanheusden.com/multitail/
----------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 19:08                 ` folkert
@ 2021-08-23 19:33                   ` sujith kumar reddy
  2021-08-23 20:16                     ` folkert
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-23 19:33 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

Hi folkert,

The above you suggested p=0 is  working .
i tried like
 if (p > bar_length || p < 0)
       p = bar_length;
This is also working.

Which is the perfect one we can use?
Is there any impact of p with driver data or this is just a
application calculated value.

Thanks
Sujith


On Tue, Aug 24, 2021 at 12:39 AM folkert <folkert@vanheusden.com> wrote:

> :-)
>
> If this also doesn't work, then I give up.
>
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index cc51dcb..03a4f73 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -1753,8 +1753,10 @@ static void print_vu_meter_stereo(int *perc, int
> *maxperc)
>         for (c = 0; c < 2; c++) {
>                 int p = perc[c] * bar_length / 100;
>                 char tmp[4];
> -               if (p > bar_length)
> -                       p = bar_length;
> +               if (p >= bar_length)
> +                       p = bar_length - 1;
> +               if (p < 0)
> +                       p = 0;
>                 if (c)
>                         memset(line + bar_length + 6 + 1, '#', p);
>                 else
>
> On Tue, Aug 24, 2021 at 12:36:05AM +0530, 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.
> >
> >
> > logs:
> > localhost ~ # ./sujith/arecord -D hw:1,2 -f S32_LE -r 48000 -c 2 1.wav
> -V s
> > Sujith entry: main
> > Recording WAVE '1.wav' : Signed 32 bit Little Endian, Rate 48000 Hz,
> Stereo
> >
> >  print_vu_meter_stereo sujith 35  ////////////////////////// c=0 case
> >  print_vu_meter_stereo sujith* -9227433* /////////////////////c=1 case p
> > value negative which is going to memset second argument.
> >
> > Thanks
> > Sujith
> >
> >
> >
> > On Tue, Aug 24, 2021 at 12:17 AM folkert <folkert@vanheusden.com> wrote:
> >
> > > Ah same bug.
> > > Please try this.
> > > The idea is that in both cases the extra '-1' breaks it.
> > >
> > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > index cc51dcb..47db7e7 100644
> > > --- a/aplay/aplay.c
> > > +++ b/aplay/aplay.c
> > > @@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int *perc, int
> > > *maxperc)
> > >         for (c = 0; c < 2; c++) {
> > >                 int p = perc[c] * bar_length / 100;
> > >                 char tmp[4];
> > > -               if (p > bar_length)
> > > -                       p = bar_length;
> > > +               if (p >= bar_length)
> > > +                       p = bar_length - 1;
> > >                 if (c)
> > >                         memset(line + bar_length + 6 + 1, '#', p);
> > >                 else
> > >
> > >
> > > On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy wrote:
> > > > Tried with the above code.This is also getting p value negative.
> > > >
> > > > snippet code:
> > > >
> > > >    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
> > > >                 if (p > bar_length)
> > > >                         p = bar_length;
> > > >                 if (c)
> > > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > >                 else
> > > >                         memset(line + bar_length - p - 1, '#', p);
> > > >
> > > >
> > > > logs....
> > > > //  c=0 case
> > > >
> > > > print_vu_meter_stereo sujith 35  ///value of p is 35
> > > > c =1 case
> > > >
> > > >  print_vu_meter_stereo sujith -9227433 ///value is negative//
> > > >
> > > > On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com>
> wrote:
> > > >
> > > > > >         const int bar_length = 35;
> > > > > >         char line[80];
> > > > > ...
> > > > > >                 if (p > bar_length)
> > > > > >                         p = bar_length;
> > > > > >                 if (c)
> > > > > >                         memset(line + bar_length + 6 + 1, '#',
> p);
> > > > > > ----------------//this is the line where it is crashing.here p
> value
> > > is
> > > > > > becoming negative at c=1.As we see if we change the p value to
> > > bar_length
> > > > > > it works fine ..As it is a player issue not a driver issue.
> > > > >
> > > > > This is puzzling.
> > > > > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that
> fits
> > > > > easlity in 80.
> > > > >
> > > > > But wait:
> > > > >
> > > > >                         line[bar_length - p - 1] = '+';
> > > > >
> > > > > p is max bar_length, so we would end up putting '+' in line[-1]
> > > > >
> > > > > Can you try this?
> > > > >
> > > > >
> > > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > > index cc51dcb..9cfd52c 100644
> > > > > --- a/aplay/aplay.c
> > > > > +++ b/aplay/aplay.c
> > > > > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc,
> int
> > > > > *maxperc)
> > > > >                         p = bar_length;
> > > > >                 if (c)
> > > > >                         line[bar_length + 6 + 1 + p] = '+';
> > > > > -               else
> > > > > +               else if (p < bar_length)
> > > > >                         line[bar_length - p - 1] = '+';
> > > > >                 if (ABS(maxperc[c]) > 99)
> > > > >                         sprintf(tmp, "MAX");
> > > > >
> > >
>
>
> Folkert van Heusden
>
> --
> Multitail es una herramienta flexible que permite visualizar los "log
> file" y seguir la ejecución de comandos. Permite filtrar, añadir
> colores, combinar archivos, la visualización de diferencias (diff-
> view), etc.  http://www.vanheusden.com/multitail/
> ----------------------------------------------------------------------
> Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 19:33                   ` sujith kumar reddy
@ 2021-08-23 20:16                     ` folkert
  2021-08-24  1:15                       ` sujith kumar reddy
  0 siblings, 1 reply; 18+ messages in thread
From: folkert @ 2021-08-23 20:16 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel

p=0 is more correct I think

this is application only

On Tue, Aug 24, 2021 at 01:03:27AM +0530, sujith kumar reddy wrote:
> Hi folkert,
> 
> The above you suggested p=0 is  working .
> i tried like
>  if (p > bar_length || p < 0)
>        p = bar_length;
> This is also working.
> 
> Which is the perfect one we can use?
> Is there any impact of p with driver data or this is just a
> application calculated value.
> 
> Thanks
> Sujith
> 
> 
> On Tue, Aug 24, 2021 at 12:39 AM folkert <folkert@vanheusden.com> wrote:
> 
> > :-)
> >
> > If this also doesn't work, then I give up.
> >
> > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > index cc51dcb..03a4f73 100644
> > --- a/aplay/aplay.c
> > +++ b/aplay/aplay.c
> > @@ -1753,8 +1753,10 @@ static void print_vu_meter_stereo(int *perc, int
> > *maxperc)
> >         for (c = 0; c < 2; c++) {
> >                 int p = perc[c] * bar_length / 100;
> >                 char tmp[4];
> > -               if (p > bar_length)
> > -                       p = bar_length;
> > +               if (p >= bar_length)
> > +                       p = bar_length - 1;
> > +               if (p < 0)
> > +                       p = 0;
> >                 if (c)
> >                         memset(line + bar_length + 6 + 1, '#', p);
> >                 else
> >
> > On Tue, Aug 24, 2021 at 12:36:05AM +0530, 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.
> > >
> > >
> > > logs:
> > > localhost ~ # ./sujith/arecord -D hw:1,2 -f S32_LE -r 48000 -c 2 1.wav
> > -V s
> > > Sujith entry: main
> > > Recording WAVE '1.wav' : Signed 32 bit Little Endian, Rate 48000 Hz,
> > Stereo
> > >
> > >  print_vu_meter_stereo sujith 35  ////////////////////////// c=0 case
> > >  print_vu_meter_stereo sujith* -9227433* /////////////////////c=1 case p
> > > value negative which is going to memset second argument.
> > >
> > > Thanks
> > > Sujith
> > >
> > >
> > >
> > > On Tue, Aug 24, 2021 at 12:17 AM folkert <folkert@vanheusden.com> wrote:
> > >
> > > > Ah same bug.
> > > > Please try this.
> > > > The idea is that in both cases the extra '-1' breaks it.
> > > >
> > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > index cc51dcb..47db7e7 100644
> > > > --- a/aplay/aplay.c
> > > > +++ b/aplay/aplay.c
> > > > @@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int *perc, int
> > > > *maxperc)
> > > >         for (c = 0; c < 2; c++) {
> > > >                 int p = perc[c] * bar_length / 100;
> > > >                 char tmp[4];
> > > > -               if (p > bar_length)
> > > > -                       p = bar_length;
> > > > +               if (p >= bar_length)
> > > > +                       p = bar_length - 1;
> > > >                 if (c)
> > > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > >                 else
> > > >
> > > >
> > > > On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy wrote:
> > > > > Tried with the above code.This is also getting p value negative.
> > > > >
> > > > > snippet code:
> > > > >
> > > > >    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
> > > > >                 if (p > bar_length)
> > > > >                         p = bar_length;
> > > > >                 if (c)
> > > > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > > >                 else
> > > > >                         memset(line + bar_length - p - 1, '#', p);
> > > > >
> > > > >
> > > > > logs....
> > > > > //  c=0 case
> > > > >
> > > > > print_vu_meter_stereo sujith 35  ///value of p is 35
> > > > > c =1 case
> > > > >
> > > > >  print_vu_meter_stereo sujith -9227433 ///value is negative//
> > > > >
> > > > > On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com>
> > wrote:
> > > > >
> > > > > > >         const int bar_length = 35;
> > > > > > >         char line[80];
> > > > > > ...
> > > > > > >                 if (p > bar_length)
> > > > > > >                         p = bar_length;
> > > > > > >                 if (c)
> > > > > > >                         memset(line + bar_length + 6 + 1, '#',
> > p);
> > > > > > > ----------------//this is the line where it is crashing.here p
> > value
> > > > is
> > > > > > > becoming negative at c=1.As we see if we change the p value to
> > > > bar_length
> > > > > > > it works fine ..As it is a player issue not a driver issue.
> > > > > >
> > > > > > This is puzzling.
> > > > > > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that
> > fits
> > > > > > easlity in 80.
> > > > > >
> > > > > > But wait:
> > > > > >
> > > > > >                         line[bar_length - p - 1] = '+';
> > > > > >
> > > > > > p is max bar_length, so we would end up putting '+' in line[-1]
> > > > > >
> > > > > > Can you try this?
> > > > > >
> > > > > >
> > > > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > > > index cc51dcb..9cfd52c 100644
> > > > > > --- a/aplay/aplay.c
> > > > > > +++ b/aplay/aplay.c
> > > > > > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int *perc,
> > int
> > > > > > *maxperc)
> > > > > >                         p = bar_length;
> > > > > >                 if (c)
> > > > > >                         line[bar_length + 6 + 1 + p] = '+';
> > > > > > -               else
> > > > > > +               else if (p < bar_length)
> > > > > >                         line[bar_length - p - 1] = '+';
> > > > > >                 if (ABS(maxperc[c]) > 99)
> > > > > >                         sprintf(tmp, "MAX");
> > > > > >
> > > >
> >
> >
> > Folkert van Heusden
> >
> > --
> > Multitail es una herramienta flexible que permite visualizar los "log
> > file" y seguir la ejecución de comandos. Permite filtrar, añadir
> > colores, combinar archivos, la visualización de diferencias (diff-
> > view), etc.  http://www.vanheusden.com/multitail/
> > ----------------------------------------------------------------------
> > Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
> >


Folkert van Heusden

-- 
Multitail es una herramienta flexible que permite visualizar los "log
file" y seguir la ejecución de comandos. Permite filtrar, añadir
colores, combinar archivos, la visualización de diferencias (diff-
view), etc.  http://www.vanheusden.com/multitail/
----------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 20:16                     ` folkert
@ 2021-08-24  1:15                       ` sujith kumar reddy
  2021-08-24  6:11                         ` sujith kumar reddy
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-24  1:15 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

Ok thank you folkert .



On Tue, 24 Aug, 2021, 1:46 AM folkert, <folkert@vanheusden.com> wrote:

> p=0 is more correct I think
>
> this is application only
>
> On Tue, Aug 24, 2021 at 01:03:27AM +0530, sujith kumar reddy wrote:
> > Hi folkert,
> >
> > The above you suggested p=0 is  working .
> > i tried like
> >  if (p > bar_length || p < 0)
> >        p = bar_length;
> > This is also working.
> >
> > Which is the perfect one we can use?
> > Is there any impact of p with driver data or this is just a
> > application calculated value.
> >
> > Thanks
> > Sujith
> >
> >
> > On Tue, Aug 24, 2021 at 12:39 AM folkert <folkert@vanheusden.com> wrote:
> >
> > > :-)
> > >
> > > If this also doesn't work, then I give up.
> > >
> > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > index cc51dcb..03a4f73 100644
> > > --- a/aplay/aplay.c
> > > +++ b/aplay/aplay.c
> > > @@ -1753,8 +1753,10 @@ static void print_vu_meter_stereo(int *perc, int
> > > *maxperc)
> > >         for (c = 0; c < 2; c++) {
> > >                 int p = perc[c] * bar_length / 100;
> > >                 char tmp[4];
> > > -               if (p > bar_length)
> > > -                       p = bar_length;
> > > +               if (p >= bar_length)
> > > +                       p = bar_length - 1;
> > > +               if (p < 0)
> > > +                       p = 0;
> > >                 if (c)
> > >                         memset(line + bar_length + 6 + 1, '#', p);
> > >                 else
> > >
> > > On Tue, Aug 24, 2021 at 12:36:05AM +0530, 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.
> > > >
> > > >
> > > > logs:
> > > > localhost ~ # ./sujith/arecord -D hw:1,2 -f S32_LE -r 48000 -c 2
> 1.wav
> > > -V s
> > > > Sujith entry: main
> > > > Recording WAVE '1.wav' : Signed 32 bit Little Endian, Rate 48000 Hz,
> > > Stereo
> > > >
> > > >  print_vu_meter_stereo sujith 35  ////////////////////////// c=0 case
> > > >  print_vu_meter_stereo sujith* -9227433* /////////////////////c=1
> case p
> > > > value negative which is going to memset second argument.
> > > >
> > > > Thanks
> > > > Sujith
> > > >
> > > >
> > > >
> > > > On Tue, Aug 24, 2021 at 12:17 AM folkert <folkert@vanheusden.com>
> wrote:
> > > >
> > > > > Ah same bug.
> > > > > Please try this.
> > > > > The idea is that in both cases the extra '-1' breaks it.
> > > > >
> > > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > > index cc51dcb..47db7e7 100644
> > > > > --- a/aplay/aplay.c
> > > > > +++ b/aplay/aplay.c
> > > > > @@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int *perc,
> int
> > > > > *maxperc)
> > > > >         for (c = 0; c < 2; c++) {
> > > > >                 int p = perc[c] * bar_length / 100;
> > > > >                 char tmp[4];
> > > > > -               if (p > bar_length)
> > > > > -                       p = bar_length;
> > > > > +               if (p >= bar_length)
> > > > > +                       p = bar_length - 1;
> > > > >                 if (c)
> > > > >                         memset(line + bar_length + 6 + 1, '#', p);
> > > > >                 else
> > > > >
> > > > >
> > > > > On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy wrote:
> > > > > > Tried with the above code.This is also getting p value negative.
> > > > > >
> > > > > > snippet code:
> > > > > >
> > > > > >    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
> > > > > >                 if (p > bar_length)
> > > > > >                         p = bar_length;
> > > > > >                 if (c)
> > > > > >                         memset(line + bar_length + 6 + 1, '#',
> p);
> > > > > >                 else
> > > > > >                         memset(line + bar_length - p - 1, '#',
> p);
> > > > > >
> > > > > >
> > > > > > logs....
> > > > > > //  c=0 case
> > > > > >
> > > > > > print_vu_meter_stereo sujith 35  ///value of p is 35
> > > > > > c =1 case
> > > > > >
> > > > > >  print_vu_meter_stereo sujith -9227433 ///value is negative//
> > > > > >
> > > > > > On Mon, Aug 23, 2021 at 10:33 PM folkert <folkert@vanheusden.com
> >
> > > wrote:
> > > > > >
> > > > > > > >         const int bar_length = 35;
> > > > > > > >         char line[80];
> > > > > > > ...
> > > > > > > >                 if (p > bar_length)
> > > > > > > >                         p = bar_length;
> > > > > > > >                 if (c)
> > > > > > > >                         memset(line + bar_length + 6 + 1,
> '#',
> > > p);
> > > > > > > > ----------------//this is the line where it is crashing.here
> p
> > > value
> > > > > is
> > > > > > > > becoming negative at c=1.As we see if we change the p value
> to
> > > > > bar_length
> > > > > > > > it works fine ..As it is a player issue not a driver issue.
> > > > > > >
> > > > > > > This is puzzling.
> > > > > > > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77, that
> > > fits
> > > > > > > easlity in 80.
> > > > > > >
> > > > > > > But wait:
> > > > > > >
> > > > > > >                         line[bar_length - p - 1] = '+';
> > > > > > >
> > > > > > > p is max bar_length, so we would end up putting '+' in line[-1]
> > > > > > >
> > > > > > > Can you try this?
> > > > > > >
> > > > > > >
> > > > > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > > > > index cc51dcb..9cfd52c 100644
> > > > > > > --- a/aplay/aplay.c
> > > > > > > +++ b/aplay/aplay.c
> > > > > > > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int
> *perc,
> > > int
> > > > > > > *maxperc)
> > > > > > >                         p = bar_length;
> > > > > > >                 if (c)
> > > > > > >                         line[bar_length + 6 + 1 + p] = '+';
> > > > > > > -               else
> > > > > > > +               else if (p < bar_length)
> > > > > > >                         line[bar_length - p - 1] = '+';
> > > > > > >                 if (ABS(maxperc[c]) > 99)
> > > > > > >                         sprintf(tmp, "MAX");
> > > > > > >
> > > > >
> > >
> > >
> > > Folkert van Heusden
> > >
> > > --
> > > Multitail es una herramienta flexible que permite visualizar los "log
> > > file" y seguir la ejecución de comandos. Permite filtrar, añadir
> > > colores, combinar archivos, la visualización de diferencias (diff-
> > > view), etc.  http://www.vanheusden.com/multitail/
> > > ----------------------------------------------------------------------
> > > Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
> > >
>
>
> Folkert van Heusden
>
> --
> Multitail es una herramienta flexible que permite visualizar los "log
> file" y seguir la ejecución de comandos. Permite filtrar, añadir
> colores, combinar archivos, la visualización de diferencias (diff-
> view), etc.  http://www.vanheusden.com/multitail/
> ----------------------------------------------------------------------
> Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-24  1:15                       ` sujith kumar reddy
@ 2021-08-24  6:11                         ` sujith kumar reddy
  0 siblings, 0 replies; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-24  6:11 UTC (permalink / raw)
  To: folkert; +Cc: alsa-devel

Hi folkert,

Are you going to submit the fix  .

Thanks
Sujith

On Tue, Aug 24, 2021 at 6:45 AM sujith kumar reddy <
sujithreddy6192@gmail.com> wrote:

> Ok thank you folkert .
>
>
>
> On Tue, 24 Aug, 2021, 1:46 AM folkert, <folkert@vanheusden.com> wrote:
>
>> p=0 is more correct I think
>>
>> this is application only
>>
>> On Tue, Aug 24, 2021 at 01:03:27AM +0530, sujith kumar reddy wrote:
>> > Hi folkert,
>> >
>> > The above you suggested p=0 is  working .
>> > i tried like
>> >  if (p > bar_length || p < 0)
>> >        p = bar_length;
>> > This is also working.
>> >
>> > Which is the perfect one we can use?
>> > Is there any impact of p with driver data or this is just a
>> > application calculated value.
>> >
>> > Thanks
>> > Sujith
>> >
>> >
>> > On Tue, Aug 24, 2021 at 12:39 AM folkert <folkert@vanheusden.com>
>> wrote:
>> >
>> > > :-)
>> > >
>> > > If this also doesn't work, then I give up.
>> > >
>> > > diff --git a/aplay/aplay.c b/aplay/aplay.c
>> > > index cc51dcb..03a4f73 100644
>> > > --- a/aplay/aplay.c
>> > > +++ b/aplay/aplay.c
>> > > @@ -1753,8 +1753,10 @@ static void print_vu_meter_stereo(int *perc,
>> int
>> > > *maxperc)
>> > >         for (c = 0; c < 2; c++) {
>> > >                 int p = perc[c] * bar_length / 100;
>> > >                 char tmp[4];
>> > > -               if (p > bar_length)
>> > > -                       p = bar_length;
>> > > +               if (p >= bar_length)
>> > > +                       p = bar_length - 1;
>> > > +               if (p < 0)
>> > > +                       p = 0;
>> > >                 if (c)
>> > >                         memset(line + bar_length + 6 + 1, '#', p);
>> > >                 else
>> > >
>> > > On Tue, Aug 24, 2021 at 12:36:05AM +0530, 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.
>> > > >
>> > > >
>> > > > logs:
>> > > > localhost ~ # ./sujith/arecord -D hw:1,2 -f S32_LE -r 48000 -c 2
>> 1.wav
>> > > -V s
>> > > > Sujith entry: main
>> > > > Recording WAVE '1.wav' : Signed 32 bit Little Endian, Rate 48000 Hz,
>> > > Stereo
>> > > >
>> > > >  print_vu_meter_stereo sujith 35  ////////////////////////// c=0
>> case
>> > > >  print_vu_meter_stereo sujith* -9227433* /////////////////////c=1
>> case p
>> > > > value negative which is going to memset second argument.
>> > > >
>> > > > Thanks
>> > > > Sujith
>> > > >
>> > > >
>> > > >
>> > > > On Tue, Aug 24, 2021 at 12:17 AM folkert <folkert@vanheusden.com>
>> wrote:
>> > > >
>> > > > > Ah same bug.
>> > > > > Please try this.
>> > > > > The idea is that in both cases the extra '-1' breaks it.
>> > > > >
>> > > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
>> > > > > index cc51dcb..47db7e7 100644
>> > > > > --- a/aplay/aplay.c
>> > > > > +++ b/aplay/aplay.c
>> > > > > @@ -1753,8 +1753,8 @@ static void print_vu_meter_stereo(int
>> *perc, int
>> > > > > *maxperc)
>> > > > >         for (c = 0; c < 2; c++) {
>> > > > >                 int p = perc[c] * bar_length / 100;
>> > > > >                 char tmp[4];
>> > > > > -               if (p > bar_length)
>> > > > > -                       p = bar_length;
>> > > > > +               if (p >= bar_length)
>> > > > > +                       p = bar_length - 1;
>> > > > >                 if (c)
>> > > > >                         memset(line + bar_length + 6 + 1, '#', p);
>> > > > >                 else
>> > > > >
>> > > > >
>> > > > > On Tue, Aug 24, 2021 at 12:14:00AM +0530, sujith kumar reddy
>> wrote:
>> > > > > > Tried with the above code.This is also getting p value negative.
>> > > > > >
>> > > > > > snippet code:
>> > > > > >
>> > > > > >    fprintf(stderr, _(" %s sujith %d \n"), __func__,p);
>> > > > > >                 if (p > bar_length)
>> > > > > >                         p = bar_length;
>> > > > > >                 if (c)
>> > > > > >                         memset(line + bar_length + 6 + 1, '#',
>> p);
>> > > > > >                 else
>> > > > > >                         memset(line + bar_length - p - 1, '#',
>> p);
>> > > > > >
>> > > > > >
>> > > > > > logs....
>> > > > > > //  c=0 case
>> > > > > >
>> > > > > > print_vu_meter_stereo sujith 35  ///value of p is 35
>> > > > > > c =1 case
>> > > > > >
>> > > > > >  print_vu_meter_stereo sujith -9227433 ///value is negative//
>> > > > > >
>> > > > > > On Mon, Aug 23, 2021 at 10:33 PM folkert <
>> folkert@vanheusden.com>
>> > > wrote:
>> > > > > >
>> > > > > > > >         const int bar_length = 35;
>> > > > > > > >         char line[80];
>> > > > > > > ...
>> > > > > > > >                 if (p > bar_length)
>> > > > > > > >                         p = bar_length;
>> > > > > > > >                 if (c)
>> > > > > > > >                         memset(line + bar_length + 6 + 1,
>> '#',
>> > > p);
>> > > > > > > > ----------------//this is the line where it is
>> crashing.here p
>> > > value
>> > > > > is
>> > > > > > > > becoming negative at c=1.As we see if we change the p value
>> to
>> > > > > bar_length
>> > > > > > > > it works fine ..As it is a player issue not a driver issue.
>> > > > > > >
>> > > > > > > This is puzzling.
>> > > > > > > bar_length + 6 + 1 + p and thus 35 + 6 + 1 + 35 is max 77,
>> that
>> > > fits
>> > > > > > > easlity in 80.
>> > > > > > >
>> > > > > > > But wait:
>> > > > > > >
>> > > > > > >                         line[bar_length - p - 1] = '+';
>> > > > > > >
>> > > > > > > p is max bar_length, so we would end up putting '+' in
>> line[-1]
>> > > > > > >
>> > > > > > > Can you try this?
>> > > > > > >
>> > > > > > >
>> > > > > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
>> > > > > > > index cc51dcb..9cfd52c 100644
>> > > > > > > --- a/aplay/aplay.c
>> > > > > > > +++ b/aplay/aplay.c
>> > > > > > > @@ -1764,7 +1764,7 @@ static void print_vu_meter_stereo(int
>> *perc,
>> > > int
>> > > > > > > *maxperc)
>> > > > > > >                         p = bar_length;
>> > > > > > >                 if (c)
>> > > > > > >                         line[bar_length + 6 + 1 + p] = '+';
>> > > > > > > -               else
>> > > > > > > +               else if (p < bar_length)
>> > > > > > >                         line[bar_length - p - 1] = '+';
>> > > > > > >                 if (ABS(maxperc[c]) > 99)
>> > > > > > >                         sprintf(tmp, "MAX");
>> > > > > > >
>> > > > >
>> > >
>> > >
>> > > Folkert van Heusden
>> > >
>> > > --
>> > > Multitail es una herramienta flexible que permite visualizar los "log
>> > > file" y seguir la ejecución de comandos. Permite filtrar, añadir
>> > > colores, combinar archivos, la visualización de diferencias (diff-
>> > > view), etc.  http://www.vanheusden.com/multitail/
>> > > ----------------------------------------------------------------------
>> > > Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
>> > >
>>
>>
>> Folkert van Heusden
>>
>> --
>> Multitail es una herramienta flexible que permite visualizar los "log
>> file" y seguir la ejecución de comandos. Permite filtrar, añadir
>> colores, combinar archivos, la visualización de diferencias (diff-
>> view), etc.  http://www.vanheusden.com/multitail/
>> ----------------------------------------------------------------------
>> Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
>>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-23 19:06               ` sujith kumar reddy
  2021-08-23 19:08                 ` folkert
@ 2021-08-24  8:12                 ` Takashi Iwai
  2021-08-24  9:19                   ` sujith kumar reddy
  1 sibling, 1 reply; 18+ messages in thread
From: Takashi Iwai @ 2021-08-24  8:12 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel, folkert

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

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

[-- Attachment #2: 0001-aplay-Fix-conversion-of-unsigned-samples-in-peak-cal.patch --]
[-- Type: application/octet-stream, Size: 1539 bytes --]

From 0ea7bfea83d97fefd18845948350322017a865c2 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
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 <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


[-- Attachment #3: 0002-aplay-Handle-16bit-sample-negative-overflow-in-peak-.patch --]
[-- Type: application/octet-stream, Size: 1135 bytes --]

From 5c4bf63a94ed0c20aca5bafb94ecd05893a45ec1 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
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 <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


[-- Attachment #4: 0003-aplay-Don-t-pass-most-negative-integer-to-abs-in-pea.patch --]
[-- Type: application/octet-stream, Size: 914 bytes --]

From d9b31338153591944d72e62523bad7850b407c63 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
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 <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


[-- Attachment #5: 0004-aplay-Handle-upper-bound-in-peak-calculations.patch --]
[-- Type: application/octet-stream, Size: 801 bytes --]

From 2efe124c31360cf0156dd0e5e7cdd52d1346a5c0 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
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 <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


[-- Attachment #6: 0005-aplay-Fix-out-of-bound-access-in-stereo-VU-meter-dra.patch --]
[-- Type: application/octet-stream, Size: 1086 bytes --]

From dea51861a8626694c6e80121c17a0a38efc2e33c Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
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 <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] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-24  8:12                 ` Takashi Iwai
@ 2021-08-24  9:19                   ` sujith kumar reddy
  2021-08-24  9:50                     ` Takashi Iwai
  0 siblings, 1 reply; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-24  9:19 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, folkert

Hi Takashi,

With the above patches -V stereo is working fine.

Thanks
sujith

On Tue, Aug 24, 2021 at 1:42 PM Takashi Iwai <tiwai@suse.de> wrote:

> 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
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: arecord is failing with -V stereo
  2021-08-24  9:19                   ` sujith kumar reddy
@ 2021-08-24  9:50                     ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2021-08-24  9:50 UTC (permalink / raw)
  To: sujith kumar reddy; +Cc: alsa-devel, folkert

On Tue, 24 Aug 2021 11:19:46 +0200,
sujith kumar reddy wrote:
> 
> Hi Takashi,
> 
> With the above patches -V stereo is working fine.

Good to hear.  The patches have been submitted and merged now.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* arecord is failing with -V stereo
@ 2021-08-17  8:38 sujith kumar reddy
  0 siblings, 0 replies; 18+ messages in thread
From: sujith kumar reddy @ 2021-08-17  8:38 UTC (permalink / raw)
  To: alsa-devel

Hi All,

arecord is  failing vumeter option -V stereo only.

localhost ~ # arecord -Dhw:1,2 -r48000 -c2 -fS32_LE /tmp/test_record.wav -M
-d 1 -V stereo
Recording WAVE '/tmp/test_record.wav' : Signed 32 bit Little Endian, Rate
48000 Hz, Stereo
*** buffer overflow detected ***: terminated
Aborted by signal Aborted...

Please provide pointers to debug this option.

Thanks
Sujith

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-08-24  9:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 18:04 arecord is failing with -V stereo sujith kumar reddy
2021-08-19 18:34 ` folkert
2021-08-19 18:43   ` sujith kumar reddy
2021-08-19 19:06     ` folkert
2021-08-23 16:31       ` sujith kumar reddy
2021-08-23 17:03         ` folkert
2021-08-23 18:44           ` sujith kumar reddy
2021-08-23 18:47             ` folkert
2021-08-23 19:06               ` sujith kumar reddy
2021-08-23 19:08                 ` folkert
2021-08-23 19:33                   ` sujith kumar reddy
2021-08-23 20:16                     ` folkert
2021-08-24  1:15                       ` sujith kumar reddy
2021-08-24  6:11                         ` sujith kumar reddy
2021-08-24  8:12                 ` Takashi Iwai
2021-08-24  9:19                   ` sujith kumar reddy
2021-08-24  9:50                     ` Takashi Iwai
  -- strict thread matches above, loose matches on Subject: below --
2021-08-17  8:38 sujith kumar reddy

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.