[MPlayer-dev-eng] [PATCH] make ao_oss use the subopt helper (was: Re: [MPlayer-DOCS] Re: CVS: main/DOCS/man/en mplayer.1, 1.1160, 1.1161)

Diego Biurrun diego at biurrun.de
Sun Dec 24 06:16:23 CET 2006


On Sun, Dec 24, 2006 at 06:13:54AM +0100, Diego Biurrun wrote:
> Digging through old mails ..
> 
> On Sun, Dec 18, 2005 at 12:25:38AM +0100, Tobias Diedrich wrote:
> > Diego Biurrun wrote:
> > > On Sun, Nov 27, 2005 at 07:22:35PM +0100, Tobias Diedrich wrote:
> > > > I think it was more clear before the change.
> > > > Most drivers use either the suboption parser or support only one
> > > > parameter.  The syntax explanation suggests that the suboption order
> > > > is not important.  Before it was made clear that oss is an exception
> > > > in that the order _is_ important, but maybe thats just me. :-)
> > > 
> > > I understand what your problem is now.  I'd say it needs a more general
> > > fix, though.
> > > 
> > > > I think it might be good to convert oss to use the suboptionparser
> > > > (breaks compatibility)
> > > 
> > > Definitely.  Breaking compatibility should not be a problem since this
> > > has only been in the CVS version so far.
> > 
> > How about the attached patch?
> 
> Here's a version that applies ..

/* no comment */

Diego
-------------- next part --------------
Index: DOCS/man/en/mplayer.1
===================================================================
RCS file: /cvsroot/mplayer/main/DOCS/man/en/mplayer.1,v
retrieving revision 1.1180
diff -u -r1.1180 mplayer.1
--- DOCS/man/en/mplayer.1	17 Dec 2005 19:58:24 -0000	1.1180
+++ DOCS/man/en/mplayer.1	17 Dec 2005 23:24:26 -0000
@@ -2001,11 +2001,11 @@
 OSS audio output driver
 .PD 0
 .RSs
-.IPs <dsp-device>
+.IPs device=<dsp-device>
 Sets the audio output device (default: /dev/\:dsp).
-.IPs <mixer-device>
+.IPs mixer=<mixer-device>
 Sets the audio mixer device (default: /dev/\:mixer).
-.IPs <mixer-channel>
+.IPs channel=<mixer-channel>
 Sets the audio mixer channel (default: pcm).
 .RE
 .PD 1
Index: libao2/ao_oss.c
===================================================================
RCS file: /cvsroot/mplayer/main/libao2/ao_oss.c,v
retrieving revision 1.57
diff -u -r1.57 ao_oss.c
--- libao2/ao_oss.c	28 Nov 2005 23:43:24 -0000	1.57
+++ libao2/ao_oss.c	17 Dec 2005 23:24:26 -0000
@@ -11,6 +11,7 @@
 #include <string.h>
 
 #include "config.h"
+#include "subopt-helper.h"
 #include "mp_msg.h"
 #include "mixer.h"
 #include "help_mp.h"
@@ -144,18 +145,19 @@
     return -1;
 }
 
-static char *dsp=PATH_DEV_DSP;
+static char *dsp = NULL;
 static audio_buf_info zz;
 static int audio_fd=-1;
 
-static const char *oss_mixer_device = PATH_DEV_MIXER;
+static const char *oss_mixer_device = NULL;
 static int oss_mixer_channel = SOUND_MIXER_PCM;
 
 // to set/get/query special features/parameters
 static int control(int cmd,void *arg){
     switch(cmd){
 	case AOCONTROL_SET_DEVICE:
-	    dsp=(char*)arg;
+	    if (dsp) free(dsp);
+	    dsp=strdup((char*)arg);
 	    return CONTROL_OK;
 	case AOCONTROL_GET_DEVICE:
 	    *(char**)arg=dsp;
@@ -179,6 +181,9 @@
 	    if(ao_data.format == AF_FORMAT_AC3)
 		return CONTROL_TRUE;
     
+	    if(!oss_mixer_device)
+		return CONTROL_ERROR;
+
 	    if ((fd = open(oss_mixer_device, O_RDONLY)) > 0)
 	    {
 		ioctl(fd, SOUND_MIXER_READ_DEVMASK, &devs);
@@ -210,35 +215,50 @@
     return CONTROL_UNKNOWN;
 }
 
+static void print_help(){
+  mp_msg(MSGT_AO, MSGL_FATAL,
+         "\n-ao oss commandline help:\n"
+         "Example: mplayer -ao oss:device=/dev/dsp2:mixer=/dev/mixer:channel=pcm\n"
+         "\nOptions:\n"
+         "  device=<device-path>\n"
+         "    Path to oss device\n"
+         "  mixer=<mixer-path>\n"
+         "    Path to mixer device\n"
+         "  channel=<mixer-channel>\n"
+         "    The mixer channel to use for volume control\n\n");
+}
+
 // open & setup audio device
 // return: 1=success 0=fail
 static int init(int rate,int channels,int format,int flags){
   char *mixer_channels [SOUND_MIXER_NRDEVICES] = SOUND_DEVICE_NAMES;
   int oss_format;
-  char *mdev = mixer_device, *mchan = mixer_channel;
+  char *mchan = NULL;
+  opt_t subopts[] = {
+    {"device", OPT_ARG_MSTRZ, &dsp},
+    {"mixer", OPT_ARG_MSTRZ, &oss_mixer_device},
+    {"channel", OPT_ARG_MSTRZ, &mchan},
+    {NULL}
+  };
 
   mp_msg(MSGT_AO,MSGL_V,"ao2: %d Hz  %d chans  %s\n",rate,channels,
     af_fmt2str_short(format));
 
-  if (ao_subdevice) {
-    char *m,*c;
-    m = strchr(ao_subdevice,':');
-    if(m) {
-      c = strchr(m+1,':');
-      if(c) {
-        mchan = c+1;
-        c[0] = '\0';
-      }
-      mdev = m+1;
-      m[0] = '\0';
-    }
-    dsp = ao_subdevice;
+  if (subopt_parse(ao_subdevice, subopts) != 0) {
+    print_help();
+    return 0;
   }
 
-  if(mdev)
-    oss_mixer_device=mdev;
-  else
-    oss_mixer_device=PATH_DEV_MIXER;
+  if(!dsp)
+    dsp=strdup(PATH_DEV_DSP);
+  if(!oss_mixer_device) {
+    if (mixer_device)
+      oss_mixer_device=strdup(mixer_device);
+    else
+      oss_mixer_device=strdup(PATH_DEV_MIXER);
+  }
+  if(!mchan)
+    mchan=strdup(mixer_channel);
   
   if(mchan){
     int fd, devs, i;
@@ -267,6 +287,8 @@
     }
   } else
     oss_mixer_channel = SOUND_MIXER_PCM;
+  free(mchan);
+  mchan = NULL;
 
   mp_msg(MSGT_AO,MSGL_V,"audio_setup: using '%s' dsp device\n", dsp);
   mp_msg(MSGT_AO,MSGL_V,"audio_setup: using '%s' mixer device\n", oss_mixer_device);
@@ -279,14 +301,14 @@
 #endif
   if(audio_fd<0){
     mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_OSS_CantOpenDev, dsp, strerror(errno));
-    return 0;
+    goto err;
   }
 
 #ifdef __linux__
   /* Remove the non-blocking flag */
   if(fcntl(audio_fd, F_SETFL, 0) < 0) {
    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_OSS_CantMakeFd, strerror(errno));
-   return 0;
+   goto err;
   }  
 #endif
 
@@ -323,7 +345,7 @@
 #endif
 
   ao_data.format = oss2format(oss_format);
-  if (ao_data.format == -1) return 0;
+  if (ao_data.format == -1) goto err;
 
   mp_msg(MSGT_AO,MSGL_V,"audio_setup: sample format: %s (requested: %s)\n",
     af_fmt2str_short(ao_data.format), af_fmt2str_short(format));
@@ -335,14 +357,14 @@
       if ( ioctl(audio_fd, SNDCTL_DSP_CHANNELS, &ao_data.channels) == -1 ||
 	   ao_data.channels != channels ) {
 	mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_OSS_CantSetChans, channels);
-	return 0;
+	goto err;
       }
     }
     else {
       int c = ao_data.channels-1;
       if (ioctl (audio_fd, SNDCTL_DSP_STEREO, &c) == -1) {
 	mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_OSS_CantSetChans, ao_data.channels);
-	return 0;
+	goto err;
       }
       ao_data.channels=c+1;
     }
@@ -391,7 +413,7 @@
     free(data);
     if(ao_data.buffersize==0){
         mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_OSS_CantUseSelect);
-        return 0;
+        goto err;
     }
 #endif
   }
@@ -404,10 +426,23 @@
   ao_data.bps*=ao_data.samplerate;
 
     return 1;
+
+err:
+    if (oss_mixer_device) free(oss_mixer_device);
+    oss_mixer_device = NULL;
+    if (dsp) free(dsp);
+    dsp = NULL;
+    return 0;
 }
 
 // close audio device
 static void uninit(int immed){
+    /* FIXME: Can't free() in uninit, since seek calls uninit, but not init
+    if (oss_mixer_device) free(oss_mixer_device);
+    oss_mixer_device = NULL;
+    if (dsp) free(dsp);
+    dsp = NULL;
+    */
     if(audio_fd == -1) return;
 #ifdef SNDCTL_DSP_SYNC
     // to get the buffer played


More information about the MPlayer-dev-eng mailing list