[MPlayer-dev-eng] Unlimited jack output channels

[Reimar Döffinger]

On Fri, May 08, 2009 at 11:37:17AM +0200, Adrian Knoth wrote:
> Since jack is the low-latency audio framework for pro-audio production,
> and jack channels (read: ports) can be freely assigned between apps, 
> there is no need to limit the amount of output channels.

The "need" is that libaf currently only supports 6 channels (see AF_NCH).
Suporting a channel count that unfortunately will only "work if you are lucky"
wasn't that high priority.

> The attached patch removes the fixed channel limit. It also adds a new
> commandline option "autoconnect" (default:1), which models the current
> behaviour: the jack ao will connect to physical/predefined ports.

Except that you added a crash (or at least several error messages) when there aren't
enough of those predefined ports.

> IOW, the patch offers maximum flexibility. Just raising the channel
> limit to let's say 128 would be fine, too, but I think there's no use in
> replacing one limit with another.

Except that no such thing as "unlimited" exists, if your code looks like it
that's probably because your code is broken.
I don't mind if you want to spend the effort to do it in a flexible and correct
way, I just thought it a waste of time.

> @@ -191,7 +189,7 @@ static void silence(float **bufs, int cnt, int num_bufs) {
>   * Write silence into buffers if paused or an underrun occured
>   */
>  static int outputaudio(jack_nframes_t nframes, void *arg) {
> -  float *bufs[MAX_CHANS];
> +  float *bufs[num_ports];

And if e.g. num_ports is 1 million, this might be outside the stack and write wherever.

>    // create out output ports
> +  ports = (jack_port_t**) malloc (num_ports * sizeof (jack_port_t*));

Useless cast, the multiplication does not check to avoid an integer overflow (calloc is
the easy solution for that.

> @@ -298,10 +299,12 @@ static int init(int rate, int channels, int format, int flags) {
>      mp_msg(MSGT_AO, MSGL_FATAL, "[JACK] activate failed\n");
>      goto err_out;
>    }
> -  for (i = 0; i < num_ports; i++) {
> -    if (jack_connect(client, jack_port_name(ports[i]), matching_ports[i])) {
> -      mp_msg(MSGT_AO, MSGL_FATAL, "[JACK] connecting failed\n");
> -      goto err_out;
> +  if (autoconnect) {
> +    for (i = 0; i < num_ports; i++) {
> +      if (jack_connect(client, jack_port_name(ports[i]), matching_ports[i])) {
> +        mp_msg(MSGT_AO, MSGL_FATAL, "[JACK] connecting failed\n");
> +        goto err_out;
> +      }
>      }

Cosmetics (like reindenting here) are always done separately to make patches easier
to review.
Also since you no longer limit num_ports, matching_ports[i] might be invalid.
In particular, also your code no longer automatically reduces the channel count
if e.g. the sound card supports only two channels. Does not matter much since
currently MPlayer does not do downmixing anyway but still does not seem like
an improvement to me.

Greetings,
Reimar Döffinger