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

xiphmont at svn.xiph.org xiphmont at svn.xiph.org
Thu Sep 11 13:09:41 PDT 2008


Author: xiphmont
Date: 2008-09-11 13:09:40 -0700 (Thu, 11 Sep 2008)
New Revision: 15298

Modified:
   trunk/cdparanoia/cachetest.c
   trunk/cdparanoia/cdparanoia.1
   trunk/cdparanoia/interface/test_interface.c
   trunk/cdparanoia/main.c
   trunk/cdparanoia/paranoia/cdda_paranoia.h
   trunk/cdparanoia/paranoia/p_block.c
   trunk/cdparanoia/paranoia/p_block.h
   trunk/cdparanoia/paranoia/paranoia.c
Log:
New cache modelling in place and functioning.



Modified: trunk/cdparanoia/cachetest.c
===================================================================
--- trunk/cdparanoia/cachetest.c	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/cachetest.c	2008-09-11 20:09:40 UTC (rev 15298)
@@ -28,6 +28,7 @@
    rely on them.  For that reason, we need to empirically determine
    cache size and strategy used for reads. */
 
+#include <unistd.h>
 #include <string.h>
 #include <stdio.h>
 #include <math.h>
@@ -45,12 +46,11 @@
 #define logC(...) {if(log){fprintf(log, __VA_ARGS__);}}
 
 static int time_drive(cdrom_drive *d, FILE *progress, FILE *log, int lba, int len, int initial_seek){
-  int i,j,x;
+  int i,x;
   int latency=0;
   double sum=0;
   double sumsq=0;
   int sofar;
-  int ret;
 
   logC("\n");
 
@@ -145,7 +145,7 @@
      we're consistently hitting latency on the same sector during
      initial collection, may need to move past it. */
 
-  int i,j,ret,x,x0;
+  int i,j,ret=0,x;
   int firstsector=-1;
   int lastsector=-1;
   int firsttest=-1;
@@ -250,7 +250,6 @@
 	  }
 	}
       }
-    next:
 
       if(iterating){
 	offset = (offset-firstsector+44999)/45000*45000+firstsector;
@@ -299,7 +298,7 @@
       
       for(i=0;i<25 && !under;i++){
 	for(j=0;;j++){
-	  int ret1,ret2;
+	  int ret1=0,ret2=0;
 	  if(i>=15){
 	    int sofar=0;
 	    
@@ -501,7 +500,7 @@
       reportC("\tTesting background readahead past read cursor... %d",readahead);
       printC("           \b\b\b\b\b\b\b\b\b\b\b");
       for(i=0;i<it;i++){
-	int sofar=0,ret,retry=0;
+	int sofar=0,ret;
 	logC("\n\t\t%d >>> ",i);
 
 	while(sofar<cachesize){
@@ -561,7 +560,7 @@
     rollbehind=cachesize;
     
     for(i=0;i<10 && rollbehind;){
-      int sofar=0,ret,retry=0;
+      int sofar=0,ret=0,retry=0;
       logC("\n\t\t>>> ");
       printC(".");
       while(sofar<cachesize){
@@ -639,7 +638,7 @@
   while(1){
     cachegran=cachesize+1;
     for(i=0;i<10 && cachegran;){
-      int sofar=0,ret,retry=0;
+      int sofar=0,ret=0,retry=0;
       logC("\n\t\t>>> ");
       printC(".");
       while(sofar<cachesize+1){
@@ -732,7 +731,7 @@
     float cachems;
     float readms;
     int readsize = cachesize-rollbehind-8;
-    int retry;
+    int retry=0;
 
     if(readsize>cachesize-1)readsize=cachesize-1;
 

Modified: trunk/cdparanoia/cdparanoia.1
===================================================================
--- trunk/cdparanoia/cdparanoia.1	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/cdparanoia.1	2008-09-11 20:09:40 UTC (rev 15298)
@@ -1,6 +1,6 @@
-.TH CDPARANOIA 1 "12 May 2008"
+.TH CDPARANOIA 1 "11 Sep 2008"
 .SH NAME
-cdparanoia 10.0 (Paranoia release III) \- an audio CD reading utility which includes extra data verification features
+cdparanoia 10.2 (Paranoia release III) \- an audio CD reading utility which includes extra data verification features
 .SH SYNOPSIS
 .B cdparanoia
 .RB [ options ]
@@ -21,19 +21,18 @@
 and scratch reconstruction capability.
 .SH OPTIONS
 
+.TP 
+.B \-A --analyze-drive
+Run and log a complete analysis of drive caching, timing and reading behavior;
+verifies that cdparanoia is correctly modelling a sprcific drive's cache and
+read behavior. Implies -vQL.
+
 .TP
 .B \-v --verbose
 Be absurdly verbose about the autosensing and reading process. Good
 for setup and debugging.
 
 .TP
-.B \-vv --verbose-debug
-In addition to verbose output, enable additional testing and debugging
-output for various analysis heuristics.  This option is intended to
-assist developers in remotely refining heuristics for problematic
-hardware.
-
-.TP
 .B \-q --quiet
 Do not print any progress or error information during the reading process.
 
@@ -42,10 +41,14 @@
 Force output of progress information to stderr (for wrapper scripts).
 
 .TP
-.B \-l --log-summary file
-Save result summary to file.
+.B \-l --log-summary [file]
+Save result summary to file, default filename cdparanoia.log.
 
 .TP
+.B \-L --log-debug [file]
+Save detailed device autosense and debugging output to a file, default filename cdparanoia.log.
+
+.TP
 .B \-V --version
 Print the program version and quit.
 

Modified: trunk/cdparanoia/interface/test_interface.c
===================================================================
--- trunk/cdparanoia/interface/test_interface.c	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/interface/test_interface.c	2008-09-11 20:09:40 UTC (rev 15298)
@@ -65,6 +65,11 @@
 
   if(!fd)fd=fdopen(d->cdda_fd,"r");
 
+  if(begin<lastread)
+    d->private->last_milliseconds=20;
+  else
+    d->private->last_milliseconds=sectors;
+
 #ifdef CDDA_TEST_UNDERRUN
   sectors-=1;
 #endif
@@ -158,7 +163,6 @@
     }else
       rbytes=fread(inner_buf,1,this_bytes,fd);
 
-    d->private->last_milliseconds=this_bytes/2352./75./20.*1000.;
 
     bytes_so_far+=rbytes;
     if(rbytes==0)break;
@@ -173,9 +177,9 @@
 #else
 #ifdef CDDA_TEST_SOMEJITTER
     jitter_flag=(drand48()>.9?1:0);
-  los_flag=(drand48()>.9?1:0);
+    los_flag=(drand48()>.9?1:0);
 #else
-  los_flag=1;
+    los_flag=1;
 #endif
 #endif
 #endif

Modified: trunk/cdparanoia/main.c
===================================================================
--- trunk/cdparanoia/main.c	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/main.c	2008-09-11 20:09:40 UTC (rev 15298)
@@ -348,7 +348,7 @@
 long callend;
 long callscript=0;
 
-static char *callback_strings[15]={"wrote",
+static char *callback_strings[16]={"wrote",
                                    "finished",
 				   "read",
 				   "verify",
@@ -362,7 +362,8 @@
 				   "overlap",
 				   "dropped",
 				   "duped",
-				   "transport error"};
+				   "transport error",
+                                   "cache error"};
 
 static int skipped_flag=0;
 static int abort_on_skip=0;
@@ -390,12 +391,27 @@
   static int slevel=0;
   static int slast=0;
   static int stimeout=0;
+  static int cacheerr=0;
   char *smilie="= :-)";
   
   if(callscript)
     fprintf(stderr,"##: %d [%s] @ %ld\n",
 	    function,(function>=-2&&function<=13?callback_strings[function+2]:
 		      ""),inpos);
+  else{
+    if(function==PARANOIA_CB_CACHEERR){
+      if(!cacheerr){
+	fprintf(stderr,
+		"\rWARNING: The CDROM drive appears to be seeking impossibly quickly.\n"
+		"This could be due to timer bugs, a drive that really is improbably fast,\n"
+		"or, most likely, a bug in cdparanoia's cache modelling.\n\n"
+		"Please consider using the -A option to perform an analysis run, then mail\n"
+		"the cdparanoia.log file produced by the analysis to paranoia-dev at xiph.org\n"
+		"to assist developers in correcting the problem.\n\n");
+      }
+      cacheerr++;
+    }
+  }
 
   if(!quiet){
     long test;
@@ -460,12 +476,17 @@
 	    break;
 	  case PARANOIA_CB_READERR:
 	    slevel=6;
-	    if(dispcache[position]!='V')
+	    if(dispcache[position]!='V' && dispcache[position]!='C')
 	      dispcache[position]='e';
 	    break;
+	  case PARANOIA_CB_CACHEERR:
+	    slevel=8;
+	    dispcache[position]='C';
+	    break;
 	  case PARANOIA_CB_SKIP:
 	    slevel=8;
-	    dispcache[position]='V';
+	    if(dispcache[position]!='C')
+	      dispcache[position]='V';
 	    break;
 	  case PARANOIA_CB_OVERLAP:
 	    overlap=osector;

Modified: trunk/cdparanoia/paranoia/cdda_paranoia.h
===================================================================
--- trunk/cdparanoia/paranoia/cdda_paranoia.h	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/paranoia/cdda_paranoia.h	2008-09-11 20:09:40 UTC (rev 15298)
@@ -22,6 +22,7 @@
 #define PARANOIA_CB_FIXUP_DROPPED 10
 #define PARANOIA_CB_FIXUP_DUPED   11
 #define PARANOIA_CB_READERR       12
+#define PARANOIA_CB_CACHEERR      13
 
 #define PARANOIA_MODE_FULL        0xff
 #define PARANOIA_MODE_DISABLE     0

Modified: trunk/cdparanoia/paranoia/p_block.c
===================================================================
--- trunk/cdparanoia/paranoia/p_block.c	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/paranoia/p_block.c	2008-09-11 20:09:40 UTC (rev 15298)
@@ -319,14 +319,15 @@
   p->fragments=new_list((void *)&i_vfragment_constructor,
 			(void *)&i_v_fragment_destructor);
 
-  p->readahead=CACHEMODEL_SECTORS;
-  p->sortcache=sort_alloc(p->readahead*CD_FRAMEWORDS);
+  p->cdcache_begin= 9999999;
+  p->cdcache_end= 9999999;
+  p->cdcache_size=CACHEMODEL_SECTORS;
+  p->sortcache=sort_alloc(p->cdcache_size*CD_FRAMEWORDS);
   p->d=d;
   p->dynoverlap=MAX_SECTOR_OVERLAP*CD_FRAMEWORDS;
   p->cache_limit=JIGGLE_MODULO;
   p->enable=PARANOIA_MODE_FULL;
   p->cursor=cdda_disc_firstsector(d);
-  p->lastread=LONG_MAX;
 
   /* One last one... in case data and audio tracks are mixed... */
   i_paranoia_firstlast(p);
@@ -336,8 +337,8 @@
 
 /* sectors < 0 indicates a query.  Returns the number of sectors before the call */
 int paranoia_cachemodel_size(cdrom_paranoia *p,int sectors){
-  int ret = p->readahead;
+  int ret = p->cdcache_size;
   if(sectors>=0)
-    p->readahead=sectors;
+    p->cdcache_size=sectors;
   return ret;
 }

Modified: trunk/cdparanoia/paranoia/p_block.h
===================================================================
--- trunk/cdparanoia/paranoia/p_block.h	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/paranoia/p_block.h	2008-09-11 20:09:40 UTC (rev 15298)
@@ -142,9 +142,11 @@
   linked_list *fragments; /* fragments of blocks that have been 'verified' */
   sort_info *sortcache;
 
-  int readahead;          /* sectors of readahead in each readop */
+  /* cache tracking */
+  int cdcache_size;
+  int cdcache_begin;
+  int cdcache_end;
   int jitter;           
-  long lastread;
 
   int enable;
   long cursor;

Modified: trunk/cdparanoia/paranoia/paranoia.c
===================================================================
--- trunk/cdparanoia/paranoia/paranoia.c	2008-09-11 12:19:18 UTC (rev 15297)
+++ trunk/cdparanoia/paranoia/paranoia.c	2008-09-11 20:09:40 UTC (rev 15298)
@@ -84,6 +84,8 @@
 #include "isort.h"
 #include <errno.h>
 
+#define MIN_SEEK_MS 6
+
 static inline long re(root_block *root){
   if(!root)return(-1);
   if(!root->vector)return(-1);
@@ -127,7 +129,6 @@
   FLAGS_VERIFIED=0x4  /**< block read and verified */
 } paranoia_read_flags;
 
-
 /**** matching and analysis code *****************************************/
 
 /* ===========================================================================
@@ -2226,8 +2227,56 @@
   return(ret);
 }
 
+static void cdrom_cache_update(cdrom_paranoia *p, int lba, int sectors){
 
+  if(lba+sectors > p->cdcache_size){
+    int end = lba+sectors;
+    lba=end-p->cdcache_size;
+    sectors = end-lba;
+  }
+    
+  if(lba < p->cdcache_begin){
+    /* a backseek flushes the cache */
+    p->cdcache_begin=lba;
+    p->cdcache_end=lba+sectors;
+  }else{
+    if(lba+sectors>p->cdcache_end)
+      p->cdcache_end = lba+sectors;
+    if(lba+sectors-p->cdcache_size > p->cdcache_begin){
+      if(lba+sectors-p->cdcache_size < p->cdcache_end){
+	p->cdcache_begin = lba+sectors-p->cdcache_size;
+      }else{
+	p->cdcache_begin = lba;
+      }
+    }
+  }
+}
 
+static void cdrom_cache_handler(cdrom_paranoia *p, int lba, void(*callback)(long,int)){
+  int seekpos;
+  int ms;
+  if(lba>=p->cdcache_end)return; /* nothing to do */
+
+  if(lba<0)lba=0;
+
+  if(lba<p->cdcache_begin){
+    /* should always trigger a backseek so let's do that here and look for the timing */
+    seekpos=(lba==0 || lba-1<cdda_disc_firstsector(p->d) ? lba : lba-1); /* keep reads linear when possible */
+  }else{
+    int pre = p->cdcache_begin-1;
+    int post = lba+p->cdcache_size;
+
+    seekpos = (pre<cdda_disc_firstsector(p->d) ? post : pre);
+  }
+
+  if(cdda_read_timed(p->d,NULL,seekpos,1,&ms)==1)
+    if(seekpos<p->cdcache_begin && ms<MIN_SEEK_MS)
+      callback(seekpos*CD_FRAMEWORDS,PARANOIA_CB_CACHEERR);
+  cdrom_cache_update(p,seekpos,1);
+  return;
+}
+
+
 /* ===========================================================================
  * read_c_block() (internal)
  *
@@ -2270,7 +2319,7 @@
    drives with unaddressable sectors behave more often). */
       
   long readat,firstread;
-  long totaltoread=p->readahead;
+  long totaltoread=p->cdcache_size;
   long sectatonce=p->d->nsectors;
   long driftcomp=(float)p->dyndrift/CD_FRAMEWORDS+.5;
   c_block *new=NULL;
@@ -2293,16 +2342,12 @@
   
   if(p->enable&(PARANOIA_MODE_VERIFY|PARANOIA_MODE_OVERLAP)){
     
-    /* we want to jitter the read alignment boundary */
     long target;
     if(rv(root)==NULL || rb(root)>beginword)
       target=p->cursor-dynoverlap; 
     else
       target=re(root)/(CD_FRAMEWORDS)-dynoverlap;
 	
-    if(target+MIN_SECTOR_BACKUP>p->lastread && target<=p->lastread)
-      target=p->lastread-MIN_SECTOR_BACKUP;
-      
     /* we want to jitter the read alignment boundary, as some
        drives, beginning from a specific point, will tend to
        lose bytes between sectors in the same place.  Also, as
@@ -2311,8 +2356,8 @@
     
     readat=(target&(~((long)JIGGLE_MODULO-1)))+p->jitter;
     if(readat>target)readat-=JIGGLE_MODULO;
-    p->jitter++;
-    if(p->jitter>=JIGGLE_MODULO)p->jitter=0;
+    p->jitter--;
+    if(p->jitter<0)p->jitter+=JIGGLE_MODULO;
      
   }else{
     readat=p->cursor; 
@@ -2338,19 +2383,15 @@
   sofar=0;
   firstread=-1;
   
-  /* Issue each of the low-level reads until we've read enough sectors
-   * to exhaust the drive's cache.
+  /* we have a read span; flush the drive cache if needed */
+  cdrom_cache_handler(p, readat, callback);
+
+  /* Issue each of the low-level reads; the optimal read size is
+   * approximately the cachemodel's cdrom cache size.  The only reason
+   * to read less would be memory considerations.
    *
    * p->readahead   = total number of sectors to read
    * p->d->nsectors = number of sectors to read per request
-   *
-   * The driver determines this latter number, which is the maximum
-   * number of sectors the kernel can reliably read per request.  In
-   * old Linux kernels, there was a hard limit of 8 sectors per read.
-   * While this limit has since been removed, certain motherboards
-   * can't handle DMA requests larger than 64K.  And other operating
-   * systems may have similar limitations.  So the method of splitting
-   * up reads is still useful.
    */
 
   /* actual read loop */
@@ -2393,6 +2434,7 @@
        * cases, you take what part of the read you know is good, and
        * you get substantially better performance. --Monty
        */
+
       if((thisread=cdda_read(p->d,buffer+sofar*CD_FRAMEWORDS,adjread,
 			    secread))<secread){
 
@@ -2440,21 +2482,12 @@
 	  flags[sofar*CD_FRAMEWORDS+i]|=FLAGS_EDGE;
       }
 
-
-      /* Move the read cursor ahead by the number of sectors we attempted
-       * to read.
-       *
-       * "???: Again, why not move it ahead by the number actually
-       * read?"  Because adding zero would not be moving
-       * ahead. --Monty
-       */
-      p->lastread=adjread+secread;
-      
       if(adjread+secread-1==p->current_lastsector)
 	new->lastsector=-1;
       
       if(callback)(*callback)((adjread+secread-1)*CD_FRAMEWORDS,PARANOIA_CB_READ);
       
+      cdrom_cache_update(p,adjread,secread);
       sofar+=secread;
       readat=adjread+secread; 
     }else /* secread <= 0 */



More information about the commits mailing list