[MPlayer-dev-eng] [PATCH] Remote ESD Support in gmplayer

[Reimar Döffinger]

On Sun, Jan 30, 2005 at 11:31:12AM +0100, Paul Wilhelm Elsinghorst wrote:
> Adds remote ESD support and software mixer selection to the gui.
> Software mixer is automatically selected when using remote esd.
> 
> The indentation is fine on my computer. Maybe evolution is messing it
> up. Should i send a compressed copy?

No, it is not fine. I just _looks_ fine. You use a tab instead of spaces
like the other code. Change your tabwidth and you'll see it.

> +Elsinghorst, Paul Wilhelm <paul at uni-bonn.de>
> +    * remote ESD support including software mixer

I don't want to annoy you, but please add "for the Gui" or something
like this, as the functionality is already there if you use the
commandline.

> +int    gtkSoftVol = 0;

don't create a new variable, just use soft_vol directly I'd say (you
_might_ have to include mixer.h)

> @@ -108,6 +112,7 @@
>  
>   { "ao_driver",&audio_driver_list,CONF_TYPE_STRING_LIST,0,0,0,NULL },
>   { "ao_volnorm",&gtkAONorm,CONF_TYPE_FLAG,0,0,1,NULL },
> + { "soft_vol",&gtkSoftVol,CONF_TYPE_FLAG,0,0,1,NULL },

I'd say name it "softvol", as that is the name of the corresponding
command-line option.

> +#ifdef USE_ESD
> +	if ( audio_driver_list && !gstrncmp( audio_driver_list[0],"esd",3 ) )
> +	 {
> +	  char *tmp;
> +	  if (gtkAOESDDevice) {
> +	  tmp = calloc( 1,strlen( gtkAOESDDevice ) + 10 );
> +	  sprintf( tmp,"esd:%s",gtkAOESDDevice );
> +	  } else
> +	    tmp = "esd";

This is still not fixed. You must free() allocated memory!

> +	  gaddlist( &audio_driver_list,tmp );
> +	  if (gtkAOESDDevice) {
> +	  soft_vol = 1;
> +	  } else
> +	    soft_vol = 0;
> +	 }

Hmm... If there is no better way to find out if it is remote, I think
you should not do autodetection (as a stupid example, somebody might
enter "127.0.0.1" as ip Address)...

Greetings,
Reimar Döffinger