[xiph-commits] r15177 - in trunk/cdparanoia: . interface paranoia

xiphmont at svn.xiph.org xiphmont at svn.xiph.org
Tue Aug 12 23:30:14 PDT 2008


Author: xiphmont
Date: 2008-08-12 23:30:12 -0700 (Tue, 12 Aug 2008)
New Revision: 15177

Modified:
   trunk/cdparanoia/interface/cdda_interface.h
   trunk/cdparanoia/interface/common_interface.c
   trunk/cdparanoia/interface/cooked_interface.c
   trunk/cdparanoia/interface/interface.c
   trunk/cdparanoia/interface/scsi_interface.c
   trunk/cdparanoia/interface/toc.c
   trunk/cdparanoia/main.c
   trunk/cdparanoia/paranoia/paranoia.c
Log:
Correct a small raft of transport errors that have crept in due to
insufficient testing while keeping up with constant kernel API churn.
Among other things, density det for very old drives has been broken
for some time, as was command set scanning.

Correct SGIO error detection such that sense keys are being properly
parsed and then the results actually returned somewhere when errors
happen.

Add a new error return code for the cdrom going missing in the middle
of an operation (404 amusingly enough; no really, it was the next in
sequence) and propogate that error upwards through the various layers;
errno is also set to ENOMEDIUM.

Allow main application read loop to bail on such a 404.

Continue working on cache detection fun




Modified: trunk/cdparanoia/interface/cdda_interface.h
===================================================================
--- trunk/cdparanoia/interface/cdda_interface.h	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/interface/cdda_interface.h	2008-08-13 06:30:12 UTC (rev 15177)
@@ -197,6 +197,7 @@
 401: Invalid track number
 402: Track not audio data
 403: No audio tracks on disc
+404: No medium present
 
 */
 #endif

Modified: trunk/cdparanoia/interface/common_interface.c
===================================================================
--- trunk/cdparanoia/interface/common_interface.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/interface/common_interface.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -174,11 +174,30 @@
    drives have a FUA facility, but it's not clear how many ignore it.
    For that reason, we need to empirically determine cache size used
    for reads */
+#if 0
+int cdda_cache_sectors(cdrom_drive *d){
 
-int cdda_cache_sectors(cdrom_drive *d){
-  /* let's assume the (physically) fastest drives are 100x.  Although
-     the physical disc itself can't spin at faster than 50x or so,
-     many drives use multiple read heads and interleave access. */
+  /* Some assumptions about timing: 
+
+     The physically fastest drives are about 50x, and usually only at
+     the rim.  This has been stable for nearly ten years at this
+     point.  It's possible to make faster drives using multiple read
+     pickups and interleaving, but it doesn't appear anyone cares to.
+
+     The slowest interface we're likely to see is UDMA40, which would
+     probably be able to maintain 100x in practice to a chain with a
+     single device on an otherwise unloaded machine.  This is a bit
+     too close for comfort to rely on simple timing thresholding,
+     especially as the kernel is going to be inserting its own
+     unpredictable timing latencies.
+
+     The bus itself adds a timing overhead; the SATA 150 machines at my disposal appear to fairly universally insert a roughly .06ms/sector (~200x) transfer latency. PATA UDMA 133 appears to be ~ .1 ms/sector.
+
+It's possible to make
+     drives faster (multiread, etc), but actual bus throughput at the
+     moment only abounts to 100x-200x,
+
+  /* let's assume the (physically) fastest drives are 60x; this is true in practice, and drives that fast are usually only that fast out at the rim */
   float ms_per_sector = 1./75./100.;
   int i;
   int lo = 75;
@@ -192,7 +211,6 @@
 
   cdmessage(d,"\nChecking CDROM drive cache behavior...\n");
 
-
   /* find the longest stretch of available audio data */
 
   for(i=0;i<d->tracks;i++){
@@ -249,6 +267,8 @@
 		return(-1);
 	      }
 	    }else{
+
+	      fprintf(stderr,">%d:%dms ",readsectors, d->private->last_milliseconds);
 	      fulltime += d->private->last_milliseconds;
 	      break;
 	    }
@@ -266,7 +286,7 @@
 
     current*=2;
   } 
-#if 0
+
   if(current > max){
     cdmessage(d,"\nGiving up; drive cache is too large to defeat using overflow.\n");
     cdmessage(d,"\n(Drives should not cache redbook reads, this drive does anyway."
@@ -276,10 +296,11 @@
     return INT_MAX;
   }
 
-#endif
   return 0;
 }
+#endif
 
+
 /************************************************************************/
 /* Here we fix up a couple of things that will never happen.  yeah,
    right.  The multisession stuff is from Hannu's code; it assumes it

Modified: trunk/cdparanoia/interface/cooked_interface.c
===================================================================
--- trunk/cdparanoia/interface/cooked_interface.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/interface/cooked_interface.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -114,6 +114,12 @@
 	  ret=-300;
 	  goto done;
 	}
+      case ENXIO:
+      case EBADF:
+      case ENOMEDIUM:
+	errno=ENOMEDIUM;
+	ret=0;
+	goto done;
       default:
 	if(sectors==1){
 	    

Modified: trunk/cdparanoia/interface/interface.c
===================================================================
--- trunk/cdparanoia/interface/interface.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/interface/interface.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -103,7 +103,7 @@
     if(sectors>0){
       sectors=d->read_audio(d,buffer,beginsector,sectors);
 
-      if(sectors!=-1){
+      if(sectors>0){
 	/* byteswap? */
 	if(d->bigendianp==-1) /* not determined yet */
 	  d->bigendianp=data_bigendianp(d);
@@ -115,7 +115,7 @@
 	  
 	  for(i=0;i<els;i++)p[i]=swap16(p[i]);
 	}
-      }
+      }	
     }
     return(sectors);
   }

Modified: trunk/cdparanoia/interface/scsi_interface.c
===================================================================
--- trunk/cdparanoia/interface/scsi_interface.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/interface/scsi_interface.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -104,12 +104,18 @@
   }
 }
 
-static int check_sbp_error(const unsigned char *sbp) {
-  if (sbp[0]) {
-    char key = sbp[2] & 0xf;
-    char ASC = sbp[12];
-    char ASCQ = sbp[13];
-    
+static int check_sbp_error(const unsigned char status,
+			   const unsigned char *sbp) {
+  char key = sbp[2] & 0xf;
+  char ASC = sbp[12];
+  char ASCQ = sbp[13];
+  
+  if(status==0)
+    return 0;
+
+  if(status==8)return TR_BUSY;
+
+  if (sbp[0]) {  
     switch (key){
     case 0:
       if (errno==0)
@@ -118,9 +124,8 @@
     case 1:
       break;
     case 2:
-      if (errno==0)
-	errno = EBUSY;
-      return(TR_BUSY);
+      errno = ENOMEDIUM;
+      return(TR_NOTREADY);
     case 3: 
       if ((ASC==0x0C) & (ASCQ==0x09)) {
 	/* loss of streaming */
@@ -172,7 +177,7 @@
 
   memset(sg_hd,0,sizeof(sg_hd)); 
   memset(sense_buffer,0,SG_MAX_SENSE); 
-  memcpy(d->private->sg_buffer,cmd,cmd_len);
+  memcpy(d->private->sg_buffer,cmd,cmd_len+in_size);
   sg_hd->twelve_byte = cmd_len == 12;
   sg_hd->result = 0;
   sg_hd->reply_len = SG_OFF + out_size;
@@ -279,7 +284,7 @@
     return(TR_EREAD);
   }
 
-  status = check_sbp_error(sense_buffer);
+  status = check_sbp_error(sg_hd->target_status, sense_buffer);
   if(status)return status;
 
   /* Failed/Partial DMA transfers occasionally get through.  Why?  No clue,
@@ -324,7 +329,8 @@
 
   memset(&hdr,0,sizeof(hdr));
   memset(sense,0,sizeof(sense));
-  
+  memcpy(d->private->sg_buffer,cmd+cmd_len,in_size);
+
   hdr.cmdp = cmd;
   hdr.cmd_len = cmd_len;
   hdr.sbp = sense;
@@ -338,15 +344,16 @@
   if(bytecheck && out_size>in_size)
     memset(hdr.dxferp+in_size,bytefill,out_size-in_size); 
 
-
   if (in_size) {
     hdr.dxfer_len = in_size;
     hdr.dxfer_direction = SG_DXFER_TO_DEV;
     
     errno = 0;
     status = ioctl(d->ioctl_fd, SG_IO, &hdr);
-    if (status >= 0 && hdr.status)
-      status = check_sbp_error(hdr.sbp);
+    if (status >= 0 && hdr.status){
+      status = check_sbp_error(hdr.status,hdr.sbp);
+      if(status) return status;
+    }
     if (status < 0) return TR_EWRITE;
   }
 
@@ -360,8 +367,10 @@
 
     errno = 0;
     status = ioctl(d->ioctl_fd, SG_IO, &hdr);
-    if (status >= 0 && hdr.status)
-      status = check_sbp_error(hdr.sbp);
+    if (status >= 0 && hdr.status){
+      status = check_sbp_error(hdr.status,hdr.sbp);
+      if(status) return status;
+    }
     if (status < 0) return TR_EREAD;
   }
 
@@ -472,7 +481,7 @@
     b[2]=b[3];
     b[3]=b[7];
 
-    memmove(b+4,b+8,size);
+    memmove(b+4,b+8,size-4);
   }
   return(0);
 }
@@ -493,6 +502,10 @@
   cmd[4]=size;
   
   if (handle_scsi_cmd (d, cmd, 6, 0, size, '\377',1, sense)) return(1);
+
+  /* dump it all... */
+  
+
   return(0);
 }
 
@@ -502,7 +515,7 @@
   return(mode_sense_scsi(d,size,page));
 }
 
-/* Current SG/SGIO impleemntations specifically disallow mode set
+/* Current SG/SGIO impleenmtations specifically disallow mode set
    unless running as root (or setuid).  One can see why (could be
    disastrous on, eg, a SCSI disk), but it curtails what we can do
    with older SCSI cdroms. */
@@ -518,7 +531,7 @@
 			      0,    /* reserved */
 			      0,    /* reserved */
 			      0,    /* reserved */
-			      12,   /* sizeof(mode) */
+			      16,   /* sizeof(mode) */
 			      0,    /* reserved */
 			      
 			      /* mode parameter header */
@@ -532,9 +545,6 @@
 			      0, 0, 0};/* Blocklen */
     unsigned char *mode = cmd + 18;
     
-    
-    cmd[1]|=d->lun<<5;
-    
     /* prepare to read cds in the previous mode */
     mode [0] = density;
     mode [6] = secsize >> 8;   /* block length "msb" */
@@ -583,7 +593,7 @@
   return(mode_select(d,d->orgdens,secsize));
 }
 
-/* switch Toshiba/DEC and HP drives from/to cdda density */
+/* switch old Toshiba/DEC and HP drives from/to cdda density */
 int scsi_enable_cdda (cdrom_drive *d, int fAudioMode){
   if (fAudioMode) {
     if(mode_select(d,d->density,CD_FRAMESIZE_RAW)){
@@ -1054,8 +1064,7 @@
 	fprintf(stderr,"                 Transport error: %s\n",strerror_tr[err]);
 	fprintf(stderr,"                 System error: %s\n",strerror(errno));
       }
-
-      if(!d->error_retry)return(-7);
+      
       switch(errno){
       case EINTR:
 	usleep(100);
@@ -1077,6 +1086,10 @@
 	}
 	sectors--;
 	continue;
+      case ENOMEDIUM:
+	cderror(d,"404: No medium present\n");
+	return(-404);
+
       default:
 	if(sectors==1){
 	  if(errno==EIO)
@@ -1102,6 +1115,8 @@
            back to cdda */
 	reset_scsi(d);
       }
+      if(!d->error_retry)return(-7);
+
     }else{
 
       /* Did we get all the bytes we think we did, or did the kernel
@@ -1284,13 +1299,11 @@
     
     d->enable_cdda(d,0);
   }
-
   if(!audioflag){
     cdmessage(d,"\tCould not find any audio tracks on this disk.\n");
     return(-403);
   }
 
-
   {
     char *es="",*rs="";
     d->bigendianp=-1;
@@ -1386,7 +1399,6 @@
       case 14:
 	d->read_audio=scsi_read_D8;
 	rs="d8 0x,00";
-	j=-2;
 	break;
       }
       
@@ -1396,7 +1408,7 @@
 	  d->density=0;
 	  d->enable_cdda=Dummy;
 	  es="none    ";
-	  if(!densitypossible)i=-2; /* short circuit MMC style commands */
+	  if(!densitypossible)i=5; /* short circuit MMC style commands */
 	  break;
 	case 1:
 	  d->density=0;
@@ -1412,11 +1424,11 @@
 	  d->density=0x82;
 	  d->enable_cdda=scsi_enable_cdda;
 	  es="yes/0x82";
+	  break;
 	case 4:
 	  d->density=0x81;
 	  d->enable_cdda=scsi_enable_cdda;
 	  es="yes/0x81";
-	  i=-2;
 	  break;
 	}
 

Modified: trunk/cdparanoia/interface/toc.c
===================================================================
--- trunk/cdparanoia/interface/toc.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/interface/toc.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -13,14 +13,14 @@
 long cdda_track_firstsector(cdrom_drive *d,int track){
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }
 
   if (track == 0) {
     if (d->disc_toc[0].dwStartSector == 0) {
       /* first track starts at lba 0 -> no pre-gap */
       cderror(d,"401: Invalid track number\n");
-      return(-1);
+      return(-401);
     }
     else {
       return 0; /* pre-gap of first track always starts at lba 0 */
@@ -29,7 +29,7 @@
 
   if(track<0 || track>d->tracks){
     cderror(d,"401: Invalid track number\n");
-    return(-1);
+    return(-401);
   }
   return(d->disc_toc[track-1].dwStartSector);
 }
@@ -38,7 +38,7 @@
   int i;
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }
 
   /* look for an audio track */
@@ -51,20 +51,20 @@
     }
 
   cderror(d,"403: No audio tracks on disc\n");  
-  return(-1);
+  return(-403);
 }
 
 long cdda_track_lastsector(cdrom_drive *d,int track){
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }
 
   if (track == 0) {
     if (d->disc_toc[0].dwStartSector == 0) {
       /* first track starts at lba 0 -> no pre-gap */
       cderror(d,"401: Invalid track number\n");
-      return(-1);
+      return(-401);
     }
     else {
       return d->disc_toc[0].dwStartSector-1;
@@ -73,7 +73,7 @@
 
   if(track<1 || track>d->tracks){
     cderror(d,"401: Invalid track number\n");
-    return(-1);
+    return(-401);
   }
   /* Safe, we've always the leadout at disc_toc[tracks] */
   return(d->disc_toc[track].dwStartSector-1);
@@ -83,7 +83,7 @@
   int i;
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }
 
   /* look for an audio track */
@@ -92,13 +92,13 @@
       return(cdda_track_lastsector(d,i+1));
 
   cderror(d,"403: No audio tracks on disc\n");  
-  return(-1);
+  return(-403);
 }
 
 long cdda_tracks(cdrom_drive *d){
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }
   return(d->tracks);
 }
@@ -106,7 +106,7 @@
 int cdda_sector_gettrack(cdrom_drive *d,long sector){
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }else{
     int i;
 
@@ -120,14 +120,14 @@
     }
 
     cderror(d,"401: Invalid track number\n");
-    return -1;
+    return -401;
   }
 }
 
 int cdda_track_bitmap(cdrom_drive *d,int track,int bit,int set,int clear){
   if(!d->opened){
     cderror(d,"400: Device not open\n");
-    return(-1);
+    return(-400);
   }
 
   if (track == 0)
@@ -135,7 +135,7 @@
 
   if(track<1 || track>d->tracks){
     cderror(d,"401: Invalid track number\n");
-    return(-1);
+    return(-401);
   }
   if ((d->disc_toc[track-1].bFlags & bit))
     return(set);

Modified: trunk/cdparanoia/main.c
===================================================================
--- trunk/cdparanoia/main.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/main.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -174,7 +174,7 @@
 	 "===========================================================");
   
   for(i=1;i<=d->tracks;i++)
-    if(cdda_track_audiop(d,i)){
+    if(cdda_track_audiop(d,i)>0){
       char buffer[256];
 
       long sec=cdda_track_firstsector(d,i);
@@ -928,9 +928,6 @@
     exit(1);
   }
 
-  /* Determine drive caching behavior */
-  cdda_cache_sectors(d);
-
   /* Dump the TOC */
   if(query_only || verbose)display_toc(d);
   if(query_only)exit(0);
@@ -1229,6 +1226,10 @@
 	  if(err)free(err);
 	  if(mes)free(mes);
 	  if(readbuf==NULL){
+	    if(errno==EBADF || errno==ENOMEDIUM){
+	      report("\nparanoia_read: CDROM drive unavailable, bailing.\n");
+	      exit(1);
+	    }
 	    skipped_flag=1;
 	    report("\nparanoia_read: Unrecoverable error, bailing.\n");
 	    break;

Modified: trunk/cdparanoia/paranoia/paranoia.c
===================================================================
--- trunk/cdparanoia/paranoia/paranoia.c	2008-08-13 00:27:18 UTC (rev 15176)
+++ trunk/cdparanoia/paranoia/paranoia.c	2008-08-13 06:30:12 UTC (rev 15177)
@@ -81,6 +81,7 @@
 #include "overlap.h"
 #include "gap.h"
 #include "isort.h"
+#include <errno.h>
 
 static inline long re(root_block *root){
   if(!root)return(-1);
@@ -183,7 +184,8 @@
  * offsets of the first and last matching samples in A.
  */
 static inline long i_paranoia_overlap2(int16_t *buffA,int16_t *buffB,
-				       char *flagsA,char *flagsB,
+				       unsigned char *flagsA,
+				       unsigned char *flagsB,
 				       long offsetA, long offsetB,
 				       long sizeA,long sizeB,
 				       long *ret_begin, long *ret_end){
@@ -252,10 +254,11 @@
  * on the CD and what B considers sample N.)
  */
 static inline long do_const_sync(c_block *A,
-				 sort_info *B,char *flagB,
+				 sort_info *B,
+				 unsigned char *flagB,
 				 long posA,long posB,
 				 long *begin,long *end,long *offset){
-  char *flagA=A->flags;
+  unsigned char *flagA=A->flags;
   long ret=0;
 
   /* If we're doing any verification whatsoever, we have flags in stage
@@ -318,14 +321,14 @@
    reference */
 
 static inline long try_sort_sync(cdrom_paranoia *p,
-				 sort_info *A,char *Aflags,
+				 sort_info *A,unsigned char *Aflags,
 				 c_block *B,
 				 long post,long *begin,long *end,
 				 long *offset,void (*callback)(long,int)){
   
   long dynoverlap=p->dynoverlap;
   sort_link *ptr=NULL;
-  char *Bflags=B->flags;
+  unsigned char *Bflags=B->flags;
 
   /* block flag matches FLAGS_UNREAD (and hence unmatchable) */
   if(Bflags==NULL || (Bflags[post-cb(B)]&FLAGS_UNREAD)==0){
@@ -2272,7 +2275,7 @@
   c_block *new=NULL;
   root_block *root=&p->root;
   int16_t *buffer=NULL;
-  char *flags=NULL;
+  unsigned char *flags=NULL;
   long sofar;
   long dynoverlap=(p->dynoverlap+CD_FRAMEWORDS-1)/CD_FRAMEWORDS; 
   long anyflag=0;
@@ -2392,7 +2395,16 @@
       if((thisread=cdda_read(p->d,buffer+sofar*CD_FRAMEWORDS,adjread,
 			    secread))<secread){
 
-	if(thisread<0)thisread=0;
+	if(thisread<0){
+	  if(errno==ENOMEDIUM){
+	    /* the one error we bail on immediately */
+	    if(new)free_c_block(new);
+	    if(buffer)free(buffer);
+	    if(flags)free(flags);
+	    return NULL;
+	  }
+	  thisread=0;
+	}
 
 	/* Uhhh... right.  Make something up. But don't make us seek
            backward! */
@@ -2502,6 +2514,11 @@
   long retry_count=0,lastend=-2;
   root_block *root=&p->root;
 
+  if(p->d->opened==0){
+    errno=EBADF;
+    return NULL;
+  }
+
   if(beginword>p->root.returnedlimit)p->root.returnedlimit=beginword;
   lastend=re(root);
 
@@ -2632,6 +2649,12 @@
 			  callback);
       
 	}
+      }else{
+
+	/* Was the medium removed or the device closed out from
+	   under us? */
+	if(errno==ENOMEDIUM) return NULL;
+      
       }
     }
 



More information about the commits mailing list