[gst-devel] CDParanoia in GStreamer, and a patch

Erik Walthinsen omega at cse.ogi.edu
Tue Dec 12 08:15:39 CET 2000


In the last couple hours, I wrote a plugin to GStreamer, one of my
projects, to wrap CDParanoia.  GStreamer (www.gstreamer.net) allows you to
construct graphs of filters of any kind, in any arrangement.  A simple one
would be to connect a disksrc to a mpg123 to a audiosink, and voila,
you've got the guts of an mp3 player.

In this case, I can connect an instance of the newly written cdparanoia
source to an audiosink, and music comes out.  It even works in full
paranoid mode on my laptop, who's drive is only slightly faster than
realtime.  That's *with* about 1KB of debug information per *frame*, or
about 4.5 MB of output per minute, to an xterm. 

I have to say that Monty really knows what he's doing.  He designed
cdparanoia *correctly*, with a library to do all the hard work and a thin
wrapper for the default interface.  A sane API in the middle makes doing
what I just did a breeze.  I wish everyone programmed this way, I wouldn't
have spent days on end trying to make mpg123 and other packages
threadsafe, or just plain safe in general, so they could work in a plugin
environment.

I did have to do one thing to cdparanoia, though, which you can see in the
diff.  Only about half the places the callback was to be called actually
checked to see if the pointer was valid.  I just went in and put the
checks in every case.  I'm passing NULL now to paranoia_read(), and all is
well.

The only issue I have with the API at this point is the fact that I'm
being forced to copy the data from the buffer passed back by
paranoia_read(), since that buffer is owned by the paranoia library.  I'd
much rather be able to pass that buffer off and let whoever comes later
deal with freeing it, in the example case the audiosink would.  But in the
grand scheme of things, copying that much data isn't horrible, just
avoidable.

TTYL,
    Omega

P.S.  I solved the problem with ripping from two drives simultanously with
an strace(1).  Turns out that it matters quite a bit which device name I
give it.  Once I started handing it /dev/scdX instead, it figured out
which generic device to use, and all was well.  I'll try to track down
what went wrong a little more and see if there's some obvious fix I can
implement in the GStreamer plugin.

         Erik Walthinsen <omega at cse.ogi.edu> - Staff Programmer @ OGI
        Quasar project - http://www.cse.ogi.edu/DISC/projects/quasar/
   Video4Linux Two drivers and stuff - http://www.cse.ogi.edu/~omega/v4l2/
        __
       /  \             SEUL: Simple End-User Linux - http://www.seul.org/
      |    | M E G A           Helping Linux become THE choice
      _\  /_                          for the home or office user
-------------- next part --------------
--- paranoia.c.orig	Mon Dec 13 20:28:05 1999
+++ paranoia.c	Mon Dec 11 22:59:40 2000
@@ -343,7 +343,7 @@
 
   while(ptr && ptr!=new){
 
-    (*callback)(cb(new),PARANOIA_CB_VERIFY);
+    if(callback)(*callback)(cb(new),PARANOIA_CB_VERIFY);
     i_iterate_stage1(p,ptr,new,callback);
 
     ptr=c_prev(ptr);
@@ -392,7 +392,7 @@
   if(min(fe(v)+p->dynoverlap,re(root))-
     max(fb(v)-p->dynoverlap,rb(root))<=0)return(0);
 
-  (*callback)(fb(v),PARANOIA_CB_VERIFY);
+  if(callback)(*callback)(fb(v),PARANOIA_CB_VERIFY);
 
   /* just a bit of v; determine the correct area */
   fbv=max(fb(v),rb(root)-p->dynoverlap);
@@ -568,7 +568,7 @@
 	  /* a problem with root */
 	  if(matchA>0){
 	    /* dropped bytes; add back from v */
-	    (*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DROPPED);
+	    if(callback)(*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DROPPED);
 	    if(rb(root)+begin<p->root.returnedlimit)
 	      break;
 	    else{
@@ -580,7 +580,7 @@
 	    }
 	  }else{
 	    /* duplicate bytes; drop from root */
-	    (*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DUPED);
+	    if(callback)(*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DUPED);
 	    if(rb(root)+begin+matchA<p->root.returnedlimit) 
 	      break;
 	    else{
@@ -594,13 +594,13 @@
 	  /* a problem with the fragment */
 	  if(matchB>0){
 	    /* dropped bytes */
-	    (*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DROPPED);
+	    if(callback)(*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DROPPED);
 	    c_insert(l,beginL,rv(root)+begin-matchB,
 			 matchB);
 	    offset+=matchB;
 	  }else{
 	    /* duplicate bytes */
-	    (*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DUPED);
+	    if(callback)(*callback)(begin+rb(root)-1,PARANOIA_CB_FIXUP_DUPED);
 	    c_remove(l,beginL+matchB,-matchB);
 	    offset+=matchB;
 	  }
@@ -620,7 +620,7 @@
 	  /* Did not determine nature of difficulty... 
 	     report and bail */
 	    
-	  /*RRR(*callback)(post,PARANOIA_CB_XXX);*/
+	  /*RRRif(callback)(*callback)(post,PARANOIA_CB_XXX);*/
 	  break;
 	}
 	/* not the most efficient way, but it will do for now */
@@ -658,13 +658,13 @@
 	  /* a problem with root */
 	  if(matchA>0){
 	    /* dropped bytes; add back from v */
-	    (*callback)(end+rb(root),PARANOIA_CB_FIXUP_DROPPED);
+	    if(callback)(*callback)(end+rb(root),PARANOIA_CB_FIXUP_DROPPED);
 	    if(end+rb(root)<p->root.returnedlimit)
 	      break;
 	    c_insert(rc(root),end,cv(l)+endL,matchA);
 	  }else{
 	    /* duplicate bytes; drop from root */
-	    (*callback)(end+rb(root),PARANOIA_CB_FIXUP_DUPED);
+	    if(callback)(*callback)(end+rb(root),PARANOIA_CB_FIXUP_DUPED);
 	    if(end+rb(root)<p->root.returnedlimit)
 	      break;
 	    c_remove(rc(root),end,-matchA);
@@ -673,11 +673,11 @@
 	  /* a problem with the fragment */
 	  if(matchB>0){
 	    /* dropped bytes */
-	    (*callback)(end+rb(root),PARANOIA_CB_FIXUP_DROPPED);
+	    if(callback)(*callback)(end+rb(root),PARANOIA_CB_FIXUP_DROPPED);
 	    c_insert(l,endL,rv(root)+end,matchB);
 	  }else{
 	    /* duplicate bytes */
-	    (*callback)(end+rb(root),PARANOIA_CB_FIXUP_DUPED);
+	    if(callback)(*callback)(end+rb(root),PARANOIA_CB_FIXUP_DUPED);
 	    c_remove(l,endL,-matchB);
 	  }
 	}else if(matchC){
@@ -710,7 +710,7 @@
 	    /* Could not determine nature of difficulty... 
 	       report and bail */
 	    
-	    /*RRR(*callback)(post,PARANOIA_CB_XXX);*/
+	    /*RRRif(callback)(*callback)(post,PARANOIA_CB_XXX);*/
 	  }
 	  break;
 	}
@@ -927,7 +927,7 @@
   }
   if(post==-1)post=0;
 
-  (*callback)(post,PARANOIA_CB_SKIP);
+  if(callback)(*callback)(post,PARANOIA_CB_SKIP);
   
   /* We want to add a sector.  Look for a c_block that spans,
      preferrably a verified area */
@@ -1139,7 +1139,7 @@
 	/* Uhhh... right.  Make something up. But don't make us seek
            backward! */
 
-	(*callback)((adjread+thisread)*CD_FRAMEWORDS,PARANOIA_CB_READERR);  
+	if(callback)(*callback)((adjread+thisread)*CD_FRAMEWORDS,PARANOIA_CB_READERR);  
 	memset(buffer+(sofar+thisread)*CD_FRAMEWORDS,0,
 	       CD_FRAMESIZE_RAW*(secread-thisread));
 	if(flags)memset(flags+(sofar+thisread)*CD_FRAMEWORDS,2,
@@ -1160,7 +1160,7 @@
       if(adjread+secread-1==p->current_lastsector)
 	new->lastsector=-1;
       
-      (*callback)((adjread+secread-1)*CD_FRAMEWORDS,PARANOIA_CB_READ);
+      if(callback)(*callback)((adjread+secread-1)*CD_FRAMEWORDS,PARANOIA_CB_READ);
       
       sofar+=secread;
       readat=adjread+secread; 
@@ -1291,7 +1291,7 @@
 	    p->dynoverlap*=1.5;
 	    if(p->dynoverlap>MAX_SECTOR_OVERLAP*CD_FRAMEWORDS)
 	      p->dynoverlap=MAX_SECTOR_OVERLAP*CD_FRAMEWORDS;
-	    (*callback)(p->dynoverlap,PARANOIA_CB_OVERLAP);
+	    if(callback)(*callback)(p->dynoverlap,PARANOIA_CB_OVERLAP);
 	  }
 	}
       }


More information about the gstreamer-devel mailing list