[MPlayer-dev-eng] Patch: Jack audio output

[Reimar Döffinger]

Hi,

[...]
>  static int driver = 0;
> +long latency = 0;
> +long approx_bytes_in_jackd = 12288; // Rough estimate (=512 frames * 6 channels * 2 bytes * 2 buffers

when is this default used? Does jack always use 6 channels? This seems 
fishy to me.

> -long JACK_Write(int deviceID, char *data, unsigned long bytes); /* returns the number of bytes written */
> +long JACK_Write(int deviceID, unsigned char *data, unsigned long bytes); /* returns the number of bytes written */

This change is rather irrelevant. Leave it as it was I'd say

>  long JACK_GetJackLatency(int deviceID); /* return the latency in milliseconds of jack */
>  int  JACK_SetState(int deviceID, enum status_enum state); /* playing, paused, stopped */
> -int  JACK_SetVolumeForChannel(int deviceID, unsigned int channel, unsigned int volume);
> +int  JACK_SetAllVolume(int deviceID, unsigned int volume); /* returns 0 on success */
> +int  JACK_SetVolumeForChannel(int deviceID, unsigned int channel, unsigned int volume); /* return 0 on success */

You only added a comment to the line with JACK_SetVolumeForChannel. If 
you really want those comments put them on seperate lines and make them 
doxygen-style.

>  	unsigned int bits_per_sample;
> -	
> +
cosmetics.

> +	unsigned int jack_port_name_count=0;
> +	const char *jack_port_name=NULL;
> +
>  	mp_msg(MSGT_AO, MSGL_INFO, "AO: [Jack] Initialising library.\n");
>  	JACK_Init();
>  
> +        if (ao_subdevice) {
> +		jack_port_flags = 0;
> +		jack_port_name_count = 1;

What purpose does this variable have?

> +		jack_port_name = ao_subdevice;
> +		mp_msg(MSGT_AO, MSGL_INFO, "AO: [Jack] Trying to use jack device:%s.\n", ao_subdevice);
> +        }
> +
>  	switch (format) {
>  		case AFMT_U8:
>  		case AFMT_S8:
> @@ -116,8 +131,10 @@
>  	}
>  
>  	rate = rate_hz;
> -	
> -	err = JACK_Open(&driver, bits_per_sample, &rate, channels);
> +
> +	err = JACK_OpenEx(&driver, bits_per_sample, &rate, channels, channels,
> +			&jack_port_name, jack_port_name_count, jack_port_flags);
> +

You only removed a lonely tab in this last line...

>  	}
> -	
> +

cosmetics

> +        // Rather rough way to find out the rough number of bytes buffered
> +        approx_bytes_in_jackd = JACK_GetJackBufferedBytes(driver) * 2;
> +	

Maybe a bit too rough a way... This might break more than it fixes...

>  static int play(void* data,int len,int flags)
>  {
> -	return JACK_Write(driver, data, len);
> +	int ret=0;
> +	ret = JACK_Write(driver, data, len);
> +	return ret;
>  }

I can see no functional change here...

> @@ -181,6 +213,9 @@
>  static void reset()
>  {
>  	JACK_Reset(driver);
> +	latency = JACK_GetJackLatency(driver);
> +	// Rather rough way to find out the rough number of bytes buffered
> +	approx_bytes_in_jackd = JACK_GetJackBufferedBytes(driver);

Why is this done here?

>  }
>  
>  
> @@ -192,6 +227,8 @@
>  
>  static float get_delay()
>  {
> -	return (float )JACK_GetJackLatency(driver);
> +	float ret=0;
> +	ret = (JACK_GetBytesStored(driver) + approx_bytes_in_jackd + latency) / (float)ao_data.bps;
> +	return ret;
>  }

Either this is wrong or the JACK_GetJackLatency function has an idiotic 
name I think...

Greetings,
Reimar Döffinger