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

[Reimar Döffinger]

Hi,
On Sun, Jan 30, 2005 at 03:47:53PM +0100, Paul Wilhelm Elsinghorst wrote:
> Concerning free():
> 
> +#ifdef USE_ESD
> +      if (strncmp(ao_driver[0], "esd", 3) == 0) {
> +        gfree(&gtkAOESDDevice);
> +        gtkAOESDDevice = gstrdup(getGtkEntryText(CEAudioDevice));
> +      }
> +#endif
> 
> Shouldn't this do the job, as included from the beginning?

No, that is unrelated (memory for the gtkAOESDDevice, I'm talking about
the tmp variable).

> +// --- software mixer
> +
> +	if ( soft_vol ) {
> +	soft_vol = 1;
> +	} else
> +	  soft_vol = 0;
> +

This is rather useless now...

> +#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";
> +	  gaddlist( &audio_driver_list,tmp );
> +	 }

I saw that it was done like this in other places, too, but it is wrong.
I fixed it in CVS, you can look there (or just replace tmp="esd" by
tmp=strdup("esd") and add free(tmp) after gaddlist).

> @@ -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 ) ||

You still have a tab here, while the other code uses only spaces

> @@ -727,6 +733,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 ) ||

Well, here the other code uses one tab + spaces, but you code uses only
spaces now... I know, it's weird/stupid/whatever but that's how
MPlayer's codebase is ;-)

Now would be a good time for some other people to test... Otherwise this
might become yet another forced-testing run for Gui-users ;-)

Greetings,
Reimar Döffinger