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

[Reimar Döffinger]

Hi,
On Sat, Jan 29, 2005 at 07:58:28PM +0100, Paul Wilhelm Elsinghorst wrote:
> Fixed the indentation.
> 
> We could automatically set the software mixer when a remote esd is
> defined. But why not have the separate option to turn on the software
> mixer, as someone might need it.

Then remove the "(remote ESD)" from the option description I'd say...


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

add "for the Gui", your patch only concerns the Gui ;-)

>  	if (gtkAONorm)
>  	  greplace(&af_cfg.list, "volnorm", "volnorm");
> +	if (gtkAOSoftVol)
> +	  greplace(&soft_vol, "1", "1");
>  	if (gtkEnableAudioEqualizer)
>  	  greplace(&af_cfg.list, "equalizer", "equalizer");

This is nonsense. First of all, you should not place it inbetwee the two
options that both modify the af_cfg list.
Also, soft_vol is not a list, it is an int! So it probably would be best
not to make a new gtkAOSoftVol variable but use soft_vol directly. Or at
least modify this to soft_vol = 1;

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

where is this freed again?

> +	  sprintf( tmp,"esd:%s",gtkAOESDDevice );
> +	  } else
> +	    tmp = "esd";

_If_ the above is freed, then this is wrong and you must use strdup.

> @@ -207,6 +209,7 @@
>      gtk_widget_set_sensitive( AConfig,FALSE );
>      if ( !strncmp( ao_driver[0],"oss",3 ) ||
>           !strncmp( ao_driver[0],"alsa",4 ) ||
> +	  !strncmp( ao_driver[0],"esd",3 ) ||

> @@ -605,6 +610,7 @@
>          gtk_widget_set_sensitive( AConfig,FALSE );
>          if ( !strncmp( ao_driver[0],"oss",3 ) ||
>               !strncmp( ao_driver[0],"alsa",4 ) ||
> +	      !strncmp( ao_driver[0],"esd",3 ) ||

indentation is still wrong...

Greetings,
Reimar Döffinger