diff mbox

Cannot combine audio devices with more than 64 channels

Message ID s5hh94khiao.wl-tiwai@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Iwai Jan. 27, 2017, 11:08 a.m. UTC
On Thu, 26 Jan 2017 17:13:50 +0100,
Clemens Ladisch wrote:
> 
> Jörg Müller wrote:
> > I created an .asoundrc with 194 inputs for each card.
> >
> > pcm_multi.c:1060: snd_pcm_multi_open: Assertion `!slave_map[sidxs[i]][schannels[i]]' failed.
> >
> > To me, this looks like a bug.
> 
> --8<---------------------------------------------------------------->8--
> pcm: multi: allocate slave_map dynamically
> 
> Using a fixed-size buffer for an arbitrarily-sized list is not a good idea.
> 
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

It turned out that we don't need the array at all.
I'll apply the following cleanup (and fix) patch.  My previous one was
buggy.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] pcm: multi: Drop the fixed slave_map[] in snd_pcm_multi_open()

slave_map[] in snd_pcm_multi_open() is a fixed size array and
obviously we have no overflow check, and eventually the program gets
an error when more than 64 channels are used.

Although we can modify the code to allocate the array dynamically, it
turned out that we can drop the whole slave_map[] thingy in this
function when looking at the code closely.  In the past, it was used
to identify the one-to-many mapping.  But the check was dropped, and
now it's nothing more than a sanity check.

Reported-by: Jörg Müller <joerg.mueller7744@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 src/pcm/pcm_multi.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Jörg Müller Jan. 27, 2017, 1:03 p.m. UTC | #1
Thanks for the patch, that was fast!

So I want to apply that patch to my system (Ubuntu Studio 14.04). How
do I proceed? I guess, the patch wont be available via apt-get soon,
right?
So I'd have to compile from source.
1. What source code packages do I need to download?
2. I'm familiar with the usual "/.configure", "make" and "sudo make
install" but in the case of this specific alsa-module, is there
anything more I'd need to do?
Takashi Iwai Jan. 27, 2017, 1:16 p.m. UTC | #2
On Fri, 27 Jan 2017 14:03:16 +0100,
Jörg Müller wrote:
> 
> Thanks for the patch, that was fast!
> 
> So I want to apply that patch to my system (Ubuntu Studio 14.04). How
> do I proceed? I guess, the patch wont be available via apt-get soon,
> right?

I have no idea about Ubuntu.  Ask Ubuntu people.

> So I'd have to compile from source.
> 1. What source code packages do I need to download?

See alsa-lib git repo listed in:
  https://www.alsa-project.org/main/index.php/GIT_Server

> 2. I'm familiar with the usual "/.configure", "make" and "sudo make
> install" but in the case of this specific alsa-module, is there
> anything more I'd need to do?

No, it's only the usual stuff, autoreconf, configure, etc.


Takashi
John Rigg Jan. 27, 2017, 8:25 p.m. UTC | #3
On Fri, Jan 27, 2017 at 02:03:16PM +0100, Jörg Müller wrote:
> So I want to apply that patch to my system (Ubuntu Studio 14.04). How
> do I proceed? I guess, the patch wont be available via apt-get soon,
> right?
> So I'd have to compile from source.
> 1. What source code packages do I need to download?
> 2. I'm familiar with the usual "/.configure", "make" and "sudo make
> install" but in the case of this specific alsa-module, is there
> anything more I'd need to do?

One method is to download alsa-lib source, apply the patch, compile
and install over the top of the existing Ubuntu libasound2 files.

Another method is to build your own patched libasound2 .deb
package to replace the standard Ubuntu one.

I'll be applying the patch here on Debian using the first method.
If you need instructions on how to do that, feel free to email
me off list at 'john.rigg.audio at gmail.com' (gmail doesn't like
my own mail server so I need to use that address to send mail to
gmail users).

John
Takashi Sakamoto Jan. 27, 2017, 11:14 p.m. UTC | #4
On Jan 27 2017 22:16, Takashi Iwai wrote:
> On Fri, 27 Jan 2017 14:03:16 +0100,
> Jörg Müller wrote:
>>
>> Thanks for the patch, that was fast!
>>
>> So I want to apply that patch to my system (Ubuntu Studio 14.04). How
>> do I proceed? I guess, the patch wont be available via apt-get soon,
>> right?
>
> I have no idea about Ubuntu.  Ask Ubuntu people.

Ubuntu 14.04 is one of LTSs, while development for this release already 
finished three years ago. Basically, packages for the release are not 
updated unless some exceptions.

In your case, you can generate alsa-lib related packages with the patch 
by your own. But no one can care of your packages and you need to do 
everything to be required.; e.g. even if you encounter issues from the 
generated packages, no one can help you.

$ sudo apt-get install build-essentials devscripts
$ sudo apt-get build-dep alsa-lib
$ cd /tmp
$ apt-get source alsa-lib
$ cd alsa-lib-*
(apply the patch)
$ dpkg-buildpackage -rfakeroot -uc -b
(packages are generated in higher directory by one level)

Perhaps, the above command lines are not enough. Some packages are still 
missing for packaging.


Regards

Takashi Sakamoto
Jörg Müller Jan. 31, 2017, 5 p.m. UTC | #5
Thanks, that worked! However, in Ubuntu, the package is called
"libasound2" instead of "alsa-lib".

2017-01-28 0:14 GMT+01:00 Takashi Sakamoto <o-takashi@sakamocchi.jp>:
> On Jan 27 2017 22:16, Takashi Iwai wrote:
>>
>> On Fri, 27 Jan 2017 14:03:16 +0100,
>> Jörg Müller wrote:
>>>
>>>
>>> Thanks for the patch, that was fast!
>>>
>>> So I want to apply that patch to my system (Ubuntu Studio 14.04). How
>>> do I proceed? I guess, the patch wont be available via apt-get soon,
>>> right?
>>
>>
>> I have no idea about Ubuntu.  Ask Ubuntu people.
>
>
> Ubuntu 14.04 is one of LTSs, while development for this release already
> finished three years ago. Basically, packages for the release are not
> updated unless some exceptions.
>
> In your case, you can generate alsa-lib related packages with the patch by
> your own. But no one can care of your packages and you need to do everything
> to be required.; e.g. even if you encounter issues from the generated
> packages, no one can help you.
>
> $ sudo apt-get install build-essentials devscripts
> $ sudo apt-get build-dep alsa-lib
> $ cd /tmp
> $ apt-get source alsa-lib
> $ cd alsa-lib-*
> (apply the patch)
> $ dpkg-buildpackage -rfakeroot -uc -b
> (packages are generated in higher directory by one level)
>
> Perhaps, the above command lines are not enough. Some packages are still
> missing for packaging.
>
>
> Regards
>
> Takashi Sakamoto
Jörg Müller Jan. 31, 2017, 5:18 p.m. UTC | #6
I applied the patch to my system and it fixed the error!

However, I get the following error which seems to be caused by a code
not being able to handle more than 256 channels:
> wfs@wfs16:~$ jackd -R -d alsa -C madifx_record_all -P madifx_playback_all
> jackdmp 1.9.10
> Copyright 2001-2005 Paul Davis and others.
> Copyright 2004-2014 Grame.
> jackdmp comes with ABSOLUTELY NO WARRANTY
> This is free software, and you are welcome to redistribute it
> under certain conditions; see the file COPYING for details
> no message buffer overruns
> no message buffer overruns
> no message buffer overruns
> JACK server starting in realtime mode with priority 10
> self-connect-mode is "Don't restrict self connect requests"
> audio_reservation_init
> Acquire audio card Audio0
> creating alsa driver ... madifx_playback_all|madifx_record_all|1024|2|48000|0|0|nomon|swmeter|-|32bit
> configuring for 48000Hz, period = 1024 frames (21.3 ms), buffer = 2 periods
> ALSA: final selected sample format for capture: 32bit float little-endian
> ALSA: use 8 periods for capture
> ALSA: final selected sample format for playback: 32bit float little-endian
> ALSA: use 8 periods for playback
> jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256' failed.
> Aborted (core dumped)

What could be the problem here?

2017-01-27 12:08 GMT+01:00 Takashi Iwai <tiwai@suse.de>:
> On Thu, 26 Jan 2017 17:13:50 +0100,
> Clemens Ladisch wrote:
>>
>> Jörg Müller wrote:
>> > I created an .asoundrc with 194 inputs for each card.
>> >
>> > pcm_multi.c:1060: snd_pcm_multi_open: Assertion `!slave_map[sidxs[i]][schannels[i]]' failed.
>> >
>> > To me, this looks like a bug.
>>
>> --8<---------------------------------------------------------------->8--
>> pcm: multi: allocate slave_map dynamically
>>
>> Using a fixed-size buffer for an arbitrarily-sized list is not a good idea.
>>
>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>
> It turned out that we don't need the array at all.
> I'll apply the following cleanup (and fix) patch.  My previous one was
> buggy.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] pcm: multi: Drop the fixed slave_map[] in snd_pcm_multi_open()
>
> slave_map[] in snd_pcm_multi_open() is a fixed size array and
> obviously we have no overflow check, and eventually the program gets
> an error when more than 64 channels are used.
>
> Although we can modify the code to allocate the array dynamically, it
> turned out that we can drop the whole slave_map[] thingy in this
> function when looking at the code closely.  In the past, it was used
> to identify the one-to-many mapping.  But the check was dropped, and
> now it's nothing more than a sanity check.
>
> Reported-by: Jörg Müller <joerg.mueller7744@gmail.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  src/pcm/pcm_multi.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/src/pcm/pcm_multi.c b/src/pcm/pcm_multi.c
> index c4b1fba32cac..9e4be7122c0f 100644
> --- a/src/pcm/pcm_multi.c
> +++ b/src/pcm/pcm_multi.c
> @@ -1015,7 +1015,6 @@ int snd_pcm_multi_open(snd_pcm_t **pcmp, const char *name,
>         snd_pcm_multi_t *multi;
>         unsigned int i;
>         snd_pcm_stream_t stream;
> -       char slave_map[64][64] = { { 0 } };
>         int err;
>
>         assert(pcmp);
> @@ -1059,8 +1058,6 @@ int snd_pcm_multi_open(snd_pcm_t **pcmp, const char *name,
>                 bind->slave_channel = schannels[i];
>                 if (sidxs[i] < 0)
>                         continue;
> -               assert(!slave_map[sidxs[i]][schannels[i]]);
> -               slave_map[sidxs[i]][schannels[i]] = 1;
>         }
>         multi->channels_count = channels_count;
>
> --
> 2.11.0
>
John Rigg Jan. 31, 2017, 5:58 p.m. UTC | #7
On Tue, Jan 31, 2017 at 06:18:48PM +0100, Jörg Müller wrote:
> I applied the patch to my system and it fixed the error!
> 
> However, I get the following error which seems to be caused by a code
> not being able to handle more than 256 channels:
> > wfs@wfs16:~$ jackd -R -d alsa -C madifx_record_all -P madifx_playback_all
> > jackdmp 1.9.10
> > Copyright 2001-2005 Paul Davis and others.
> > Copyright 2004-2014 Grame.
> > jackdmp comes with ABSOLUTELY NO WARRANTY
> > This is free software, and you are welcome to redistribute it
> > under certain conditions; see the file COPYING for details
> > no message buffer overruns
> > no message buffer overruns
> > no message buffer overruns
> > JACK server starting in realtime mode with priority 10
> > self-connect-mode is "Don't restrict self connect requests"
> > audio_reservation_init
> > Acquire audio card Audio0
> > creating alsa driver ... madifx_playback_all|madifx_record_all|1024|2|48000|0|0|nomon|swmeter|-|32bit
> > configuring for 48000Hz, period = 1024 frames (21.3 ms), buffer = 2 periods
> > ALSA: final selected sample format for capture: 32bit float little-endian
> > ALSA: use 8 periods for capture
> > ALSA: final selected sample format for playback: 32bit float little-endian
> > ALSA: use 8 periods for playback
> > jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256' failed.
> > Aborted (core dumped)
> 
> What could be the problem here?

jackd only allows 256 ports maximum by default. Try using -p option
to increase it, eg. to 512:

jackd -R -p512 -d alsa....etc

BTW you don't need to specify -R on recent jackd versions as it's
realtime by default (use -r if you don't want realtime).

John
Takashi Iwai Jan. 31, 2017, 6:46 p.m. UTC | #8
On Tue, 31 Jan 2017 18:18:48 +0100,
Jörg Müller wrote:
> 
> I applied the patch to my system and it fixed the error!
> 
> However, I get the following error which seems to be caused by a code
> not being able to handle more than 256 channels:
> > wfs@wfs16:~$ jackd -R -d alsa -C madifx_record_all -P madifx_playback_all
> > jackdmp 1.9.10
> > Copyright 2001-2005 Paul Davis and others.
> > Copyright 2004-2014 Grame.
> > jackdmp comes with ABSOLUTELY NO WARRANTY
> > This is free software, and you are welcome to redistribute it
> > under certain conditions; see the file COPYING for details
> > no message buffer overruns
> > no message buffer overruns
> > no message buffer overruns
> > JACK server starting in realtime mode with priority 10
> > self-connect-mode is "Don't restrict self connect requests"
> > audio_reservation_init
> > Acquire audio card Audio0
> > creating alsa driver ... madifx_playback_all|madifx_record_all|1024|2|48000|0|0|nomon|swmeter|-|32bit
> > configuring for 48000Hz, period = 1024 frames (21.3 ms), buffer = 2 periods
> > ALSA: final selected sample format for capture: 32bit float little-endian
> > ALSA: use 8 periods for capture
> > ALSA: final selected sample format for playback: 32bit float little-endian
> > ALSA: use 8 periods for playback
> > jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256' failed.
> > Aborted (core dumped)
> 
> What could be the problem here?

It looks rather like a problem / limitation of jack itself.


Takashi
Jörg Müller Feb. 8, 2017, 2:12 p.m. UTC | #9
John Rigg informed me about the port-max-option of Jack. So I tried
running the following commands:
jackd --port-max 1024 -d alsa -C madifx_record_all -P madifx_playback_all
jackd -p 1024 -d alsa -C madifx_record_all -P madifx_playback_all
jackd -p1024 -d alsa -C madifx_record_all -P madifx_playback_all

I also tried ommiting either the option of -C or of -P. But I keep
getting the same error:
jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int
Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256'
failed.

So does that mean jackd doesn't recognize the parameter -p 1024?


2017-01-31 20:31 GMT+01:00 John Rigg <aldev2@jrigg.co.uk>:
> On Tue, Jan 31, 2017 at 06:18:48PM +0100, Jörg Müller wrote:
>> I applied the patch to my system and it fixed the error!
>>
>> However, I get the following error which seems to be caused by a code
>> not being able to handle more than 256 channels:
>> > wfs@wfs16:~$ jackd -R -d alsa -C madifx_record_all -P madifx_playback_all
>> > jackdmp 1.9.10
>> > Copyright 2001-2005 Paul Davis and others.
>> > Copyright 2004-2014 Grame.
>> > jackdmp comes with ABSOLUTELY NO WARRANTY
>> > This is free software, and you are welcome to redistribute it
>> > under certain conditions; see the file COPYING for details
>> > no message buffer overruns
>> > no message buffer overruns
>> > no message buffer overruns
>> > JACK server starting in realtime mode with priority 10
>> > self-connect-mode is "Don't restrict self connect requests"
>> > audio_reservation_init
>> > Acquire audio card Audio0
>> > creating alsa driver ... madifx_playback_all|madifx_record_all|1024|2|48000|0|0|nomon|swmeter|-|32bit
>> > configuring for 48000Hz, period = 1024 frames (21.3 ms), buffer = 2 periods
>> > ALSA: final selected sample format for capture: 32bit float little-endian
>> > ALSA: use 8 periods for capture
>> > ALSA: final selected sample format for playback: 32bit float little-endian
>> > ALSA: use 8 periods for playback
>> > jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256' failed.
>> > Aborted (core dumped)
>>
>> What could be the problem here?
>
> jackd only allows 256 ports maximum by default. Try using -p option
> to increase it, eg. to 512:
>
> jackd -R -p512 -d alsa....etc
>
> BTW you don't need to specify -R on recent jackd versions as it's
> realtime by default (use -r if you don't want realtime).
>
> John
John Rigg Feb. 8, 2017, 4:12 p.m. UTC | #10
On Wed, Feb 08, 2017 at 03:12:37PM +0100, Jörg Müller wrote:
> John Rigg informed me about the port-max-option of Jack. So I tried
> running the following commands:
> jackd --port-max 1024 -d alsa -C madifx_record_all -P madifx_playback_all
> jackd -p 1024 -d alsa -C madifx_record_all -P madifx_playback_all
> jackd -p1024 -d alsa -C madifx_record_all -P madifx_playback_all
> 
> I also tried ommiting either the option of -C or of -P. But I keep
> getting the same error:
> jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int
> Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256'
> failed.
> 
> So does that mean jackd doesn't recognize the parameter -p 1024?

I just tried jackd -p1024 here and there's no error message. I don't
have the hardware to easily test that many ports. I'm using pcm_multi
with three ice1712 cards and that accepts the jackd -p1024 option.
I tried adding jack clients until the default 256 ports was exceeded,
and there's no problem (tried up to 324 ports so far).

In jack2/common/JackGlobals.h it has the following:
#ifndefine PORT_NUM_MAX
#define PORT_NUM_MAX 4096
#endif

It looks like there's a restriction on driver capture and/or
playback channels somewhere, but you might have to grep through
the alsa and jack2 source code to find it.

John
John Rigg Feb. 8, 2017, 4:20 p.m. UTC | #11
On Wed, Feb 08, 2017 at 03:12:37PM +0100, Jörg Müller wrote:
> John Rigg informed me about the port-max-option of Jack. So I tried
> running the following commands:
> jackd --port-max 1024 -d alsa -C madifx_record_all -P madifx_playback_all
> jackd -p 1024 -d alsa -C madifx_record_all -P madifx_playback_all
> jackd -p1024 -d alsa -C madifx_record_all -P madifx_playback_all
> 
> I also tried ommiting either the option of -C or of -P. But I keep
> getting the same error:
> jackd: ../linux/alsa/JackAlsaDriver.cpp:122: virtual int
> Jack::JackAlsaDriver::Attach(): Assertion `fCaptureChannels < 256'
> failed.
> 
> So does that mean jackd doesn't recognize the parameter -p 1024?

I just tried jackd -p1024 here and there's no error message. I don't
have the hardware to easily test that many ports. I'm using pcm_multi
with three ice1712 cards and that accepts the jackd -p1024 option.
I tried adding jack clients until the default 256 ports was exceeded,
and there's no problem (tried up to 324 ports so far).

In jack2/common/JackGlobals.h it has the following:
#ifndefine PORT_NUM_MAX
#define PORT_NUM_MAX 4096
#endif

It looks like there's a restriction on driver capture and/or
playback channels somewhere, but you might have to grep through
the alsa and jack2 source code to find it.

John
John Rigg Feb. 8, 2017, 4:31 p.m. UTC | #12
On Wed, Feb 08, 2017 at 04:20:12PM +0000, John Rigg wrote:
> In jack2/common/JackGlobals.h it has the following:
> #ifndefine PORT_NUM_MAX
> #define PORT_NUM_MAX 4096
> #endif
> 

Sorry, that should be JackConstants.h

John
Jörg Müller Feb. 8, 2017, 5:22 p.m. UTC | #13
In common/JackConstants.h, there is the line:
#define DRIVER_PORT_NUM 256
(https://github.com/jackaudio/jack2/blob/master/common/JackConstants.h#L53)

in linux/alsa/JackAlsaDriver.cpp, there are the lines throwing the error:
assert(fCaptureChannels < DRIVER_PORT_NUM);
assert(fPlaybackChannels < DRIVER_PORT_NUM);
(https://github.com/jackaudio/jack2/blob/master/linux/alsa/JackAlsaDriver.cpp#L122-L123)

They both compare against DRIVER_PORT_NUM, which always caps them at
256 since its a constant. So the problem may be there?

2017-02-08 17:31 GMT+01:00 John Rigg <aldev2@jrigg.co.uk>:
> On Wed, Feb 08, 2017 at 04:20:12PM +0000, John Rigg wrote:
>> In jack2/common/JackGlobals.h it has the following:
>> #ifndefine PORT_NUM_MAX
>> #define PORT_NUM_MAX 4096
>> #endif
>>
>
> Sorry, that should be JackConstants.h
>
> John

2017-02-08 17:31 GMT+01:00 John Rigg <aldev2@jrigg.co.uk>:
> On Wed, Feb 08, 2017 at 04:20:12PM +0000, John Rigg wrote:
>> In jack2/common/JackGlobals.h it has the following:
>> #ifndefine PORT_NUM_MAX
>> #define PORT_NUM_MAX 4096
>> #endif
>>
>
> Sorry, that should be JackConstants.h
>
> John
diff mbox

Patch

diff --git a/src/pcm/pcm_multi.c b/src/pcm/pcm_multi.c
index c4b1fba32cac..9e4be7122c0f 100644
--- a/src/pcm/pcm_multi.c
+++ b/src/pcm/pcm_multi.c
@@ -1015,7 +1015,6 @@  int snd_pcm_multi_open(snd_pcm_t **pcmp, const char *name,
 	snd_pcm_multi_t *multi;
 	unsigned int i;
 	snd_pcm_stream_t stream;
-	char slave_map[64][64] = { { 0 } };
 	int err;
 
 	assert(pcmp);
@@ -1059,8 +1058,6 @@  int snd_pcm_multi_open(snd_pcm_t **pcmp, const char *name,
 		bind->slave_channel = schannels[i];
 		if (sidxs[i] < 0)
 			continue;
-		assert(!slave_map[sidxs[i]][schannels[i]]);
-		slave_map[sidxs[i]][schannels[i]] = 1;
 	}
 	multi->channels_count = channels_count;