]> sjero.net Git - wget/blobdiff - src/progress.c
[svn] Don't allow initial_length to exceed total_length.
[wget] / src / progress.c
index 2e889b1233fda16a6a241ea2e44dac967a73b984..644c1b2b9dc1571bb3d3dbabbbc5c4bad27f6dbc 100644 (file)
@@ -41,10 +41,10 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.  */
 
 struct progress_implementation {
   char *name;
-  void *(*create) (long, long);
-  void (*update) (void *, long, long);
-  void (*finish) (void *, long);
-  void (*set_params) (const char *);
+  void *(*create) PARAMS ((long, long));
+  void (*update) PARAMS ((void *, long, long));
+  void (*finish) PARAMS ((void *, long));
+  void (*set_params) PARAMS ((const char *));
 };
 
 /* Necessary forward declarations. */
@@ -401,11 +401,16 @@ dot_set_params (const char *params)
    create_image will overflow the buffer.  */
 #define MINIMUM_SCREEN_WIDTH 45
 
-/* Number of recent packets we keep the stats for. */
-#define RECENT_ARRAY_SIZE 30
-
 static int screen_width = DEFAULT_SCREEN_WIDTH;
 
+/* Size of the history table for download speeds. */
+#define DLSPEED_HISTORY_SIZE 30
+
+/* The time interval in milliseconds below which we increase old
+   history entries rather than overwriting them.  That interval
+   represents the scope of the download speed history. */
+#define DLSPEED_HISTORY_MAX_INTERVAL 3000
+
 struct bar_progress {
   long initial_length;         /* how many bytes have been downloaded
                                   previously. */
@@ -428,16 +433,16 @@ struct bar_progress {
                                   is not known. */
 
   /* The following variables (kept in a struct for namespace reasons)
-     keep track of how long it took to read recent packets.  See
-     bar_update() for explanation.  */
-  struct {
-    long previous_time;
-    long times[RECENT_ARRAY_SIZE];
-    long bytes[RECENT_ARRAY_SIZE];
-    int count;
+     keep track of recent download speeds.  See bar_update() for
+     details.  */
+  struct bar_progress_hist {
+    int pos;
+    long times[DLSPEED_HISTORY_SIZE];
+    long bytes[DLSPEED_HISTORY_SIZE];
     long summed_times;
     long summed_bytes;
-  } recent;
+    long previous_time;
+  } hist;
 
   /* create_image() uses these to make sure that ETA information
      doesn't flash. */
@@ -456,6 +461,11 @@ bar_create (long initial, long total)
 
   memset (bp, 0, sizeof (*bp));
 
+  /* In theory, our callers should take care of this pathological
+     case, but it can sometimes happen. */
+  if (initial > total)
+    total = initial;
+
   bp->initial_length = initial;
   bp->total_length   = total;
 
@@ -476,8 +486,9 @@ static void
 bar_update (void *progress, long howmuch, long dltime)
 {
   struct bar_progress *bp = progress;
+  struct bar_progress_hist *hist = &bp->hist;
   int force_screen_update = 0;
-  int rec_index;
+  long delta_time = dltime - hist->previous_time;
 
   bp->count += howmuch;
   if (bp->total_length > 0
@@ -487,60 +498,54 @@ bar_update (void *progress, long howmuch, long dltime)
        adjust bp->total_length to the new reality, so that the code in
        create_image() that depends on total size being smaller or
        equal to the expected size doesn't abort.  */
-    bp->total_length = bp->count + bp->initial_length;
-
-  /* The progress bar is supposed to display the "current download
-     speed".  The first version of the progress bar calculated it by
-     dividing the total amount of data with the total time needed to
-     download it.  The problem with this was that stalled or suspended
-     download could unduly influence the "current" time.  Taking just
-     the time needed to download the current packet would not work
-     either because packets arrive too fast and the varitions would be
-     too jerky.
-
-     It would be preferrable to show the speed that pertains to a
-     recent period, say over the past several seconds.  But to do this
-     accurately, we would have to record all the packets received
-     during the last five seconds.
+    bp->total_length = bp->initial_length + bp->count;
 
-     What we do instead is maintain a history of a fixed number of
-     packets.  It actually makes sense if you think about it -- faster
-     downloads will have a faster response to speed changes.  */
+  /* This code attempts to determine the current download speed.  We
+     measure the speed over the interval of approximately three
+     seconds, in subintervals no smaller than 0.1s.  In other words,
+     we maintain and use the history of 30 most recent reads, where a
+     "read" consists of one or more network reads, up until the point
+     where a subinterval is filled. */
 
-  rec_index = bp->recent.count % RECENT_ARRAY_SIZE;
-  ++bp->recent.count;
-
-  /* Instead of calculating the sum of times[] and bytes[], we
-     maintain the summed quantities.  To maintain each sum, we must
-     make sure that it gets increased by the newly downloaded amount,
-     but also that it gets decreased by the amount we're overwriting
-     in (erasing from) the cyclical buffer.  */
-  bp->recent.summed_times -= bp->recent.times[rec_index];
-  bp->recent.summed_bytes -= bp->recent.bytes[rec_index];
-
-  bp->recent.times[rec_index] = dltime - bp->recent.previous_time;
-  bp->recent.bytes[rec_index] = howmuch;
-
-  bp->recent.summed_times += bp->recent.times[rec_index];
-  bp->recent.summed_bytes += bp->recent.bytes[rec_index];
+  if (hist->times[hist->pos]
+      >= DLSPEED_HISTORY_MAX_INTERVAL / DLSPEED_HISTORY_SIZE)
+    {
+      /* The subinterval at POS has been used up.  Move on to the next
+        position. */
+      if (++hist->pos == DLSPEED_HISTORY_SIZE)
+       hist->pos = 0;
+
+      /* Invalidate old data (from the previous cycle) at this
+        position. */
+      hist->summed_times -= hist->times[hist->pos];
+      hist->summed_bytes -= hist->bytes[hist->pos];
+      hist->times[hist->pos] = delta_time;
+      hist->bytes[hist->pos] = howmuch;
+    }
+  else
+    {
+      /* Increment the data at POS. */
+      hist->times[hist->pos] += delta_time;
+      hist->bytes[hist->pos] += howmuch;
+    }
 
-  bp->recent.previous_time = dltime;
+  hist->summed_times += delta_time;
+  hist->summed_bytes += howmuch;
+  hist->previous_time = dltime;
 
 #if 0
   /* Sledgehammer check that summed_times and summed_bytes are
      accurate.  */
   {
-    int num = bp->recent.count;
     int i;
-    int upper = num < RECENT_ARRAY_SIZE ? num : RECENT_ARRAY_SIZE;
     long sumt = 0, sumb = 0;
-    for (i = 0; i < upper; i++)
+    for (i = 0; i < DLSPEED_HISTORY_SIZE; i++)
       {
-       sumt += bp->recent.times[i];
-       sumb += bp->recent.bytes[i];
+       sumt += hist->times[i];
+       sumb += hist->bytes[i];
       }
-    assert (sumt == bp->recent.summed_times);
-    assert (sumb == bp->recent.summed_bytes);
+    assert (sumt == hist->summed_times);
+    assert (sumb == hist->summed_bytes);
   }
 #endif
 
@@ -565,11 +570,10 @@ bar_finish (void *progress, long dltime)
 {
   struct bar_progress *bp = progress;
 
-  if (dltime == 0)
-    /* If the download was faster than the granularity of the timer,
-       fake some output so that we don't get the ugly "----.--" rate
-       at the download finish.  */
-    dltime = 1;
+  if (bp->total_length > 0
+      && bp->count + bp->initial_length > bp->total_length)
+    /* See bar_update() for explanation. */
+    bp->total_length = bp->initial_length + bp->count;
 
   create_image (bp, dltime);
   display_image (bp->buffer);
@@ -598,8 +602,7 @@ create_image (struct bar_progress *bp, long dl_total_time)
   char *size_legible = legible (size);
   int size_legible_len = strlen (size_legible);
 
-  long recent_time = bp->recent.summed_times;
-  long recent_bytes = bp->recent.summed_bytes;
+  struct bar_progress_hist *hist = &bp->hist;
 
   /* The progress bar should look like this:
      xx% [=======>             ] nn,nnn 12.34K/s ETA 00:00
@@ -698,11 +701,12 @@ create_image (struct bar_progress *bp, long dl_total_time)
   p += strlen (p);
 
   /* " 1012.45K/s" */
-  if (recent_time && recent_bytes)
+  if (hist->summed_times && hist->summed_bytes)
     {
       static char *short_units[] = { "B/s", "K/s", "M/s", "G/s" };
       int units = 0;
-      double dlrate = calc_rate (recent_bytes, recent_time, &units);
+      double dlrate;
+      dlrate = calc_rate (hist->summed_bytes, hist->summed_times, &units);
       sprintf (p, " %7.2f%s", dlrate, short_units[units]);
       p += strlen (p);
     }
@@ -710,22 +714,28 @@ create_image (struct bar_progress *bp, long dl_total_time)
     APPEND_LITERAL ("   --.--K/s");
 
   /* " ETA xx:xx:xx" */
-  if (bp->total_length > 0 && recent_bytes > 0)
+  if (bp->total_length > 0 && dl_total_time > 3000)
     {
       long eta;
       int eta_hrs, eta_min, eta_sec;
 
       /* Don't change the value of ETA more than approximately once
         per second; doing so would cause flashing without providing
-        any value to the user.  */
+        any value to the user. */
       if (dl_total_time - bp->last_eta_time < 900
          && bp->last_eta_value != 0)
        eta = bp->last_eta_value;
       else
        {
-         double tm_sofar = (double)recent_time / 1000;
+         /* Calculate ETA using the average download speed to predict
+            the future speed.  If you want to use the current speed
+            instead, replace dl_total_time with hist->summed_times
+            and bp->count with hist->summed_bytes.  I found that
+            doing that results in a very jerky and ultimately
+            unreliable ETA.  */
+         double time_sofar = (double)dl_total_time / 1000;
          long bytes_remaining = bp->total_length - size;
-         eta = (long) (tm_sofar * bytes_remaining / recent_bytes);
+         eta = (long) (time_sofar * bytes_remaining / bp->count);
          bp->last_eta_value = eta;
          bp->last_eta_time = dl_total_time;
        }
@@ -734,34 +744,29 @@ create_image (struct bar_progress *bp, long dl_total_time)
       eta_min = eta / 60,   eta %= 60;
       eta_sec = eta;
 
-      /* Pad until the end of screen.  The padding is dependent on the
-        hour value.  */
-      if (eta_hrs == 0 || eta_hrs > 99)
-       /* Hours not printed: pad with three spaces (two digits and
-          colon). */
-       APPEND_LITERAL ("   ");
-      else if (eta_hrs < 10)
-       /* Hours printed with one digit: pad with one space. */
-       *p++ = ' ';
-      else
-       /* Hours printed with two digits: we're using maximum width,
-          don't pad. */
-       ;
-
-      APPEND_LITERAL (" ETA ");
-
       if (eta_hrs > 99)
-       /* Bogus value, probably due to a calculation overflow.  Print
-          something safe to avoid overstepping the buffer bounds. */
-       sprintf (p, "--:--");
-      else if (eta_hrs > 0)
-       sprintf (p, "%d:%02d:%02d", eta_hrs, eta_min, eta_sec);
+       goto no_eta;
+
+      if (eta_hrs == 0)
+       {
+         /* Hours not printed: pad with three spaces. */
+         APPEND_LITERAL ("   ");
+         sprintf (p, " ETA %02d:%02d", eta_min, eta_sec);
+       }
       else
-       sprintf (p, "%02d:%02d", eta_min, eta_sec);
+       {
+         if (eta_hrs < 10)
+           /* Hours printed with one digit: pad with one space. */
+           *p++ = ' ';
+         sprintf (p, " ETA %d:%02d:%02d", eta_hrs, eta_min, eta_sec);
+       }
       p += strlen (p);
     }
   else if (bp->total_length > 0)
-    APPEND_LITERAL ("    ETA --:--");
+    {
+    no_eta:
+      APPEND_LITERAL ("             ");
+    }
 
   assert (p - bp->buffer <= bp->width);