[xiph-commits] r12335 - in trunk/oss2pulse: . src/oss2pulse

xiphmont at svn.xiph.org xiphmont at svn.xiph.org
Wed Jan 17 05:32:03 PST 2007


Author: xiphmont
Date: 2007-01-17 05:32:01 -0800 (Wed, 17 Jan 2007)
New Revision: 12335

Modified:
   trunk/oss2pulse/README
   trunk/oss2pulse/src/oss2pulse/common.c
   trunk/oss2pulse/src/oss2pulse/dsp.c
   trunk/oss2pulse/src/oss2pulse/mixer.c
Log:
Eliminate last few places where a call could wait indefinitely, blocking fusd.



Modified: trunk/oss2pulse/README
===================================================================
--- trunk/oss2pulse/README	2007-01-17 13:11:51 UTC (rev 12334)
+++ trunk/oss2pulse/README	2007-01-17 13:32:01 UTC (rev 12335)
@@ -1,5 +1,5 @@
 This is a quick writeup to get the oss2pulse daemon running as things
-stand at 2007-01-11.  Things are as yet unfinished, and this applies
+stand at 2007-01-17.  Things are as yet unfinished, and this applies
 to both fusd and oss2pulse.
 
 WHAT IT DOES:
@@ -43,15 +43,11 @@
 
 1) no mmap support for /dev/dsp yet (should be there within two weeks). 
 
-2) there are as yet three places where the code simply blocks waiting
-   for a response from Pulse; this will block all applications talking
-   to oss2pulse.
+2) numerous unimplemented ioctl()s.
 
-3) numerous unimplemented ioctl()s.
+3) Mixer mapping is a bit of a hack. See the 'RFC' section.
 
-4) Mixer mapping is a bit of a hack. See the 'RFC' section.
-
-5) the fusd implementation as yet does not have fully generic class
+4) the fusd implementation as yet does not have fully generic class
    support.  It can only create devices of a class that doesn't
    already exist, but handles the 'sound' class as a special case.
    This requires a small kernel patch to correct and will be available
@@ -81,4 +77,4 @@
 in-daemon software scaling.
 
 Monty
-20070111
\ No newline at end of file
+20070117
\ No newline at end of file

Modified: trunk/oss2pulse/src/oss2pulse/common.c
===================================================================
--- trunk/oss2pulse/src/oss2pulse/common.c	2007-01-17 13:11:51 UTC (rev 12334)
+++ trunk/oss2pulse/src/oss2pulse/common.c	2007-01-17 13:32:01 UTC (rev 12335)
@@ -105,7 +105,8 @@
 }
 
 
-// TODO: eliminate the mainloop_wait
+// Only ever called from an async worker thread, so the mainloop_wait
+// can't block fusd.
 fd_info* fd_info_new(fd_info_type_t type, int *_errno) {
     fd_info *i;
     int sfds[2] = { -1, -1 };

Modified: trunk/oss2pulse/src/oss2pulse/dsp.c
===================================================================
--- trunk/oss2pulse/src/oss2pulse/dsp.c	2007-01-17 13:11:51 UTC (rev 12334)
+++ trunk/oss2pulse/src/oss2pulse/dsp.c	2007-01-17 13:32:01 UTC (rev 12335)
@@ -485,7 +485,11 @@
     }
 }
 
-// TODO: this blocks inside FUSD!  eliminate the pa_threaded_mainloop_wait(i->mainloop)!
+// the mainloop_wait below is unfortunately unavoidable; see the
+// comments for dsp_close.
+
+// Only ever called from an async worker thread, so the mainloop_wait
+// can't block fusd.
 int dsp_drain(fd_info *i) {
   if(i == NULL) return -1;
   
@@ -499,9 +503,6 @@
   
   pa_threaded_mainloop_lock(i->mainloop);
   
-  //if (dsp_empty_socket(i) < 0)
-  //   goto fail;
-  
   if (!i->play_stream)
     goto fail;
   
@@ -537,7 +538,8 @@
   return r;
 }
 
-// TODO: this blocks inside FUSD!  eliminate the pa_threaded_mainloop_wait(i->mainloop)!
+// Only ever called from an async worker thread, so the mainloop_wait
+// can't block fusd.
 static int dsp_trigger(fd_info *i) {
   if(i == NULL) return -1;
   pa_operation *o = NULL;
@@ -582,25 +584,37 @@
   return 0;
 }
 
-static int dsp_open(struct fusd_file_info* file){
+// fd_info_new can block and the mainloop thread manipulation within
+// can't be done from a callback.
+static void *dsp_open_thread(void *arg){
+  struct fusd_file_info* file = arg;
   fd_info *i;
-  int ret;
+  int ret = 0;
   int f;
   
-  debug(DEBUG_LEVEL_NORMAL, __FILE__": dsp_open()\n");
+  debug(DEBUG_LEVEL_NORMAL, __FILE__": dsp_open_worker()\n");
   
-  if (!(i = fd_info_new(FD_INFO_STREAM, &ret)))
-    return -ret;
-  
-  debug(DEBUG_LEVEL_NORMAL, __FILE__": dsp_open() succeeded\n");
+  if ((i = fd_info_new(FD_INFO_STREAM, &ret))){
 
-  fd_info_add_to_list(i);
-  fd_info_unref(i);
-  file->private_data = i;
-  
+    debug(DEBUG_LEVEL_NORMAL, __FILE__": dsp_open() succeeded\n");
+    
+    fd_info_add_to_list(i);
+    fd_info_unref(i);
+    file->private_data = i;
+  }
+
+  fusd_return(file, -ret);
   return 0;
 }
 
+static int dsp_open(struct fusd_file_info* file){
+  pthread_t dummy;
+  debug(DEBUG_LEVEL_NORMAL, __FILE__": dsp_open()\n");
+  if(pthread_create(&dummy,NULL,dsp_open_thread,file))
+    dsp_open_thread(file);
+  return -FUSD_NOREPLY;
+}
+
 static ssize_t dsp_read(struct fusd_file_info* file, 
 		    char* buffer, 
 		    size_t size, 

Modified: trunk/oss2pulse/src/oss2pulse/mixer.c
===================================================================
--- trunk/oss2pulse/src/oss2pulse/mixer.c	2007-01-17 13:11:51 UTC (rev 12334)
+++ trunk/oss2pulse/src/oss2pulse/mixer.c	2007-01-17 13:32:01 UTC (rev 12335)
@@ -72,95 +72,96 @@
     pa_operation_unref(o);
 }
 
-// Eliminate the mainloop_wait
-static int mixer_open(struct fusd_file_info* file){
+static void *mixer_open_thread(void *arg){
+  struct fusd_file_info *file = arg;
   fd_info *i;
   pa_operation *o = NULL;
   int ret = 0;
   
   debug(DEBUG_LEVEL_NORMAL, __FILE__": mixer_open()\n");
   
-  if (!(i = fd_info_new(FD_INFO_MIXER, &ret))) 
-    return -ret;
+  if ((i = fd_info_new(FD_INFO_MIXER, &ret))){
   
-  pa_threaded_mainloop_lock(i->mainloop);
+    pa_threaded_mainloop_lock(i->mainloop);
+    
+    pa_context_set_subscribe_callback(i->context, subscribe_cb, i);
   
-  pa_context_set_subscribe_callback(i->context, subscribe_cb, i);
-  
-  if (!(o = pa_context_subscribe(i->context, PA_SUBSCRIPTION_MASK_SINK | PA_SUBSCRIPTION_MASK_SOURCE, context_success_cb, i))) {
-    debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to subscribe to events: %s", pa_strerror(pa_context_errno(i->context)));
-    ret = -EIO;
-    goto fail;
+    if (!(o = pa_context_subscribe(i->context, PA_SUBSCRIPTION_MASK_SINK | PA_SUBSCRIPTION_MASK_SOURCE, context_success_cb, i))) {
+      debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to subscribe to events: %s", pa_strerror(pa_context_errno(i->context)));
+      ret = -EIO;
+      goto fail;
+    }
+    
+    i->operation_success = 0;
+    while (pa_operation_get_state(o) != PA_OPERATION_DONE) {
+      pa_threaded_mainloop_wait(i->mainloop);
+      CONTEXT_CHECK_DEAD_GOTO(i, fail);
+    }
+    
+    pa_operation_unref(o);
+    o = NULL;
+    
+    if (!i->operation_success) {
+      debug(DEBUG_LEVEL_NORMAL, __FILE__":Failed to subscribe to events: %s", pa_strerror(pa_context_errno(i->context)));
+      ret = -EIO;
+      goto fail;
+    }
+    
+    /* Get sink info */
+    
+    if (!(o = pa_context_get_sink_info_by_name(i->context, NULL, sink_info_cb, i))) {
+      debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get sink info: %s", pa_strerror(pa_context_errno(i->context)));
+      ret = -EIO;
+      goto fail;
+    }
+    
+    i->operation_success = 0;
+    while (pa_operation_get_state(o) != PA_OPERATION_DONE) {
+      pa_threaded_mainloop_wait(i->mainloop);
+      CONTEXT_CHECK_DEAD_GOTO(i, fail);
+    }
+    
+    pa_operation_unref(o);
+    o = NULL;
+    
+    if (!i->operation_success) {
+      debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get sink info: %s", pa_strerror(pa_context_errno(i->context)));
+      ret = -EIO;
+      goto fail;
+    }
+    
+    /* Get source info */
+    
+    if (!(o = pa_context_get_source_info_by_name(i->context, NULL, source_info_cb, i))) {
+      debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get source info: %s", pa_strerror(pa_context_errno(i->context)));
+      ret = -EIO;
+      goto fail;
+    }
+    
+    i->operation_success = 0;
+    while (pa_operation_get_state(o) != PA_OPERATION_DONE) {
+      pa_threaded_mainloop_wait(i->mainloop);
+      CONTEXT_CHECK_DEAD_GOTO(i, fail);
+    }
+    
+    pa_operation_unref(o);
+    o = NULL;
+    
+    if (!i->operation_success) {
+      debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get source info: %s", pa_strerror(pa_context_errno(i->context)));
+      ret = -EIO;
+      goto fail;
+    }
+    
+    file->private_data = i;
+    
+    pa_threaded_mainloop_unlock(i->mainloop);
+    
+    fd_info_add_to_list(i);
+    fd_info_unref(i);
   }
 
-  i->operation_success = 0;
-  while (pa_operation_get_state(o) != PA_OPERATION_DONE) {
-    pa_threaded_mainloop_wait(i->mainloop);
-    CONTEXT_CHECK_DEAD_GOTO(i, fail);
-  }
-  
-  pa_operation_unref(o);
-  o = NULL;
-  
-  if (!i->operation_success) {
-    debug(DEBUG_LEVEL_NORMAL, __FILE__":Failed to subscribe to events: %s", pa_strerror(pa_context_errno(i->context)));
-    ret = -EIO;
-    goto fail;
-  }
-  
-  /* Get sink info */
-  
-  if (!(o = pa_context_get_sink_info_by_name(i->context, NULL, sink_info_cb, i))) {
-    debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get sink info: %s", pa_strerror(pa_context_errno(i->context)));
-    ret = -EIO;
-    goto fail;
-  }
-  
-  i->operation_success = 0;
-  while (pa_operation_get_state(o) != PA_OPERATION_DONE) {
-    pa_threaded_mainloop_wait(i->mainloop);
-    CONTEXT_CHECK_DEAD_GOTO(i, fail);
-  }
-  
-  pa_operation_unref(o);
-  o = NULL;
-  
-  if (!i->operation_success) {
-    debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get sink info: %s", pa_strerror(pa_context_errno(i->context)));
-    ret = -EIO;
-    goto fail;
-  }
-  
-  /* Get source info */
-  
-  if (!(o = pa_context_get_source_info_by_name(i->context, NULL, source_info_cb, i))) {
-    debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get source info: %s", pa_strerror(pa_context_errno(i->context)));
-    ret = -EIO;
-    goto fail;
-  }
-  
-  i->operation_success = 0;
-  while (pa_operation_get_state(o) != PA_OPERATION_DONE) {
-    pa_threaded_mainloop_wait(i->mainloop);
-    CONTEXT_CHECK_DEAD_GOTO(i, fail);
-  }
-  
-  pa_operation_unref(o);
-  o = NULL;
-  
-  if (!i->operation_success) {
-    debug(DEBUG_LEVEL_NORMAL, __FILE__": Failed to get source info: %s", pa_strerror(pa_context_errno(i->context)));
-    ret = -EIO;
-    goto fail;
-  }
-
-  file->private_data = i;
-  
-  pa_threaded_mainloop_unlock(i->mainloop);
- 
-  fd_info_add_to_list(i);
-  fd_info_unref(i);
-    
+  fusd_return(file, 0);
   return 0;
 
 fail:
@@ -172,27 +173,40 @@
   if (i)
     fd_info_unref(i);
   
-  debug(DEBUG_LEVEL_NORMAL, __FILE__": mixer_open() failed\n");
+  if(ret)
+    debug(DEBUG_LEVEL_NORMAL, __FILE__": mixer_open() failed\n");
   
-  return ret;
+  fusd_return(file, ret);
+  return 0;
 }
 
-// Eliminate the mainloop_wait
-static int mixer_ioctl(struct fusd_file_info *file, int request, void *argp){
+static int mixer_open(struct fusd_file_info* file){
+  pthread_t dummy;
+  debug(DEBUG_LEVEL_NORMAL, __FILE__": mixer_open()\n");
+  if(pthread_create(&dummy,NULL,mixer_open_thread,file))
+    mixer_open_thread(file);
+  return -FUSD_NOREPLY;
+}
+
+static void *mixer_ioctl_thread(void *arg){
+  struct fusd_file_info *file = arg;
   fd_info *i = file->private_data;
+  int request = i->ioctl_request;
+  void *argp = i->ioctl_argp;
+  int ret = 0;
   
   switch (request) {
   case SOUND_MIXER_READ_DEVMASK :
     debug(DEBUG_LEVEL_NORMAL, __FILE__": SOUND_MIXER_READ_DEVMASK\n");
     
     *(int*) argp = SOUND_MASK_PCM | SOUND_MASK_IGAIN | SOUND_MASK_VOLUME;
-    return 0;
+    break;
     
   case SOUND_MIXER_READ_RECMASK :
     debug(DEBUG_LEVEL_NORMAL, __FILE__": SOUND_MIXER_READ_RECMASK\n");
     
     *(int*) argp = SOUND_MASK_IGAIN;
-    return 0;
+    break;
     
   case SOUND_MIXER_READ_STEREODEVS:
     debug(DEBUG_LEVEL_NORMAL, __FILE__": SOUND_MIXER_READ_STEREODEVS\n");
@@ -204,23 +218,23 @@
     if (i->source_volume.channels > 1)
       *(int*) argp |= SOUND_MASK_IGAIN;
     pa_threaded_mainloop_unlock(i->mainloop);
-    return 0;
+    break;
 
   case SOUND_MIXER_READ_RECSRC:
     debug(DEBUG_LEVEL_NORMAL, __FILE__": SOUND_MIXER_READ_RECSRC\n");
     
     *(int*) argp = SOUND_MASK_IGAIN;
-    return 0;
+    break;
     
   case SOUND_MIXER_WRITE_RECSRC:
     debug(DEBUG_LEVEL_NORMAL, __FILE__": SOUND_MIXER_WRITE_RECSRC\n");
-    return 0;
+    break;
     
   case SOUND_MIXER_READ_CAPS:
     debug(DEBUG_LEVEL_NORMAL, __FILE__": SOUND_MIXER_READ_CAPS\n");
     
     *(int*) argp = 0;
-    return 0;
+    break;
     
   case SOUND_MIXER_READ_PCM:
   case SOUND_MIXER_READ_VOLUME:
@@ -238,7 +252,7 @@
       
       pa_threaded_mainloop_unlock(i->mainloop);
     }
-    return 0;
+    break;
 
   case SOUND_MIXER_READ_IGAIN: 
     {
@@ -255,7 +269,7 @@
       
       pa_threaded_mainloop_unlock(i->mainloop);
     }
-    return 0;
+    break;
 
   case SOUND_MIXER_WRITE_PCM:
   case SOUND_MIXER_WRITE_VOLUME:
@@ -299,7 +313,7 @@
       
       pa_threaded_mainloop_unlock(i->mainloop);
     }   
-    return 0;
+    break;
     
   case SOUND_MIXER_WRITE_IGAIN: 
     {
@@ -343,7 +357,7 @@
       
       pa_threaded_mainloop_unlock(i->mainloop);
     }
-    return 0;
+    break;
 
   case SOUND_MIXER_INFO: 
     {
@@ -358,15 +372,35 @@
       mi->modify_counter = i->volume_modify_count;
       pa_threaded_mainloop_unlock(i->mainloop);
     }
-    return 0;
+    break;
 
   default:
     debug(DEBUG_LEVEL_NORMAL, __FILE__": unknown ioctl 0x%08lx\n", request);
     
-    return -EINVAL;
+    ret = -EINVAL;
+    break;
   }
+
+  fusd_return(file, ret);
+  return 0;
 }
 
+static int mixer_ioctl(struct fusd_file_info *file, int request, void *argp){
+  struct fd_info* i = file->private_data;
+  pthread_t dummy;
+
+  if(i == NULL) return -EBADFD;
+  if(i->unusable) return -EBADFD;
+
+  i->ioctl_request = request;
+  i->ioctl_argp = argp;
+  
+  if(pthread_create(&dummy,NULL,mixer_ioctl_thread,file))
+    mixer_ioctl_thread(file);
+
+  return -FUSD_NOREPLY;
+}
+
 int mixer_close(struct fusd_file_info* file){
   fd_info *i = file->private_data;
   



More information about the commits mailing list