From 06824aee2d106db18de3a7323030284c6cfbef43 Mon Sep 17 00:00:00 2001
From: lemonsqueeze <lemonsqueeze@gmx.com>
Date: Fri, 25 May 2018 23:14:56 +0200
Subject: [PATCH 1/2] Fixed inaccurate ETAs: use current average rate over last
 30 seconds instead of global rate

using global rate gives wildly inacurrate ETAs if rate changes over time.

this makes pv keep a history of progress over the last 30s interval and compute
current average rate over that, which fixes the issue.

fixed -a, --average-rate: show current average rate instead of global rate

[ quick test ]

- in one shell:
    $ dd if=/dev/zero of=/tmp/foo bs=1M count=5                 # simulate fast initial rate,
    $ (while true; do ls /*; sleep 1; done) >> /tmp/foo         # then slow rate. leave running ...

- in another shell:
    $ tail -f -n +1 /tmp/foo | pv -s 800M > /dev/null

    5.62MiB 0:00:17 [33.1KiB/s] [>                              ]  0% ETA 0:40:02
        bad:  (pv 1.6.6 - 1.6.20)
        ETA of a few minutes that keeps increasing ever and ever ...

    5.62MiB 0:00:16 [33.1KiB/s] [>                              ]  0% ETA 6:45:53
        good:
        ETA of 6+ hours, which is how long it'll take to reach 800Mb at 33k/s
---
 doc/quickref.1.in         |  6 ++--
 src/include/pv-internal.h | 15 +++++++++
 src/pv/display.c          | 65 ++++++++++++++++++++++++---------------
 src/pv/state.c            | 23 ++++++++++++++
 4 files changed, 81 insertions(+), 28 deletions(-)

diff --git a/doc/quickref.1.in b/doc/quickref.1.in
index a3131c9..823507f 100644
--- a/doc/quickref.1.in
+++ b/doc/quickref.1.in
@@ -127,7 +127,7 @@ Turn the timer on.  This will display the total elapsed time that
 has been running for.
 .TP
 .B \-e, \-\-eta
-Turn the ETA timer on.  This will attempt to guess, based on previous
+Turn the ETA timer on.  This will attempt to guess, based on current
 transfer rates and the total data size, how long it will be before
 completion.  This option will have no effect if the total data size cannot
 be determined.
@@ -142,8 +142,8 @@ Turn the rate counter on.  This will display the current rate of data
 transfer.
 .TP
 .B \-a, \-\-average\-rate
-Turn the average rate counter on.  This will display the average rate of
-data transfer so far.
+Turn the average rate counter on.  This will display the current average
+rate of data transfer (default: last 30s).
 .TP
 .B \-b, \-\-bytes
 Turn the total byte counter on.  This will display the total amount of
diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h
index db65eaa..cc94825 100644
--- a/src/include/pv-internal.h
+++ b/src/include/pv-internal.h
@@ -45,6 +45,12 @@ extern "C" {
 #define MAXIMISE_BUFFER_FILL	1
 
 
+typedef struct pvhistory {
+	long long   total_bytes;
+	long double elapsed_sec;
+} pvhistory_t;
+
+
 /*
  * Structure for holding PV internal state. Opaque outside the PV library.
  */
@@ -113,6 +119,15 @@ struct pvstate_s {
 	long double prev_elapsed_sec;
 	long double prev_rate;
 	long double prev_trans;
+
+	/* Keep track of progress over last intervals to compute current average rate. */
+	pvhistory_t *history;            /* state at previous intervals (circular buffer) */
+	int history_len;		 /* total size */
+	int history_interval;		 /* seconds between each history entry */
+	int history_first;
+	int history_last;
+	long double current_avg_rate;    /* current average rate over last history intervals */
+	
 	unsigned long long initial_offset;
 	char *display_buffer;
 	long display_buffer_size;
diff --git a/src/pv/display.c b/src/pv/display.c
index aff643b..aa7bb62 100644
--- a/src/pv/display.c
+++ b/src/pv/display.c
@@ -95,16 +95,14 @@ static long pv__calc_percentage(long long so_far, const long long total)
  * number of seconds until completion.
  */
 static long pv__calc_eta(const long long so_far, const long long total,
-			 const long elapsed)
+			 const long rate)
 {
 	long long amount_left;
 
-	if (so_far < 1)
+	if (so_far < 1 || !rate)
 		return 0;
 
-	amount_left = total - so_far;
-	amount_left *= (long long) elapsed;
-	amount_left /= so_far;
+	amount_left = (total - so_far) / rate;
 
 	return (long) amount_left;
 }
@@ -430,6 +428,38 @@ static long bound_long(long x, long min, long max)
 	return x < min ? min : x > max ? max : x;
 }
 
+/* Update history and current average rate */
+static void update_history_avg_rate(pvstate_t state, long long total_bytes,
+				    long double elapsed_sec, long double rate)
+{
+	int first = state->history_first;
+	int last = state->history_last;
+	long double last_elapsed = state->history[last].elapsed_sec;
+
+	if (!(last_elapsed == 0.0 ||  /* Empty */
+	      elapsed_sec > last_elapsed + state->history_interval))
+		return;
+	
+	if (last_elapsed) { /* Not empty, add new entry in circular buffer */
+		int len = state->history_len;
+		state->history_last = last = (last + 1) % len;
+		if (last == first)
+			state->history_first = first = (first + 1) % len;
+	}
+	
+	state->history[last].elapsed_sec = elapsed_sec;
+	state->history[last].total_bytes = total_bytes;
+	
+	if (first == last)
+		state->current_avg_rate = rate;
+	else {
+		long long bytes = (state->history[last].total_bytes -
+				   state->history[first].total_bytes);
+		long double sec = (state->history[last].elapsed_sec -
+				   state->history[first].elapsed_sec);
+		state->current_avg_rate = bytes / sec;
+	}
+}
 
 /*
  * Return a pointer to a string (which must not be freed), containing status
@@ -490,24 +520,9 @@ static char *pv__format(pvstate_t state,
 	}
 	state->prev_rate = rate;
 
-	/*
-	 * We only calculate the overall average rate if this is the last
-	 * update or if the average rate display is enabled. Otherwise it's
-	 * not worth the extra CPU cycles.
-	 */
-	average_rate = 0;
-	if ((bytes_since_last < 0)
-	    || ((state->components_used & PV_DISPLAY_AVERAGERATE) != 0)) {
-		/* Sanity check to avoid division by zero */
-		if (elapsed_sec < 0.000001)
-			elapsed_sec = 0.000001;
-		average_rate =
-		    (((long double) total_bytes) -
-		     ((long double) state->initial_offset)) /
-		    (long double) elapsed_sec;
-		if (bytes_since_last < 0)
-			rate = average_rate;
-	}
+	/* Update history and current average rate for ETA. */
+	update_history_avg_rate(state, total_bytes, elapsed_sec, rate);
+	average_rate = state->current_avg_rate;
 
 	if (state->size <= 0) {
 		/*
@@ -680,7 +695,7 @@ static char *pv__format(pvstate_t state,
 		eta =
 		    pv__calc_eta(total_bytes - state->initial_offset,
 				 state->size - state->initial_offset,
-				 elapsed_sec);
+				 state->current_avg_rate);
 
 		/*
 		 * Bounds check, so we don't overrun the suffix buffer. This
@@ -732,7 +747,7 @@ static char *pv__format(pvstate_t state,
 		eta =
 		    pv__calc_eta(total_bytes - state->initial_offset,
 				 state->size - state->initial_offset,
-				 elapsed_sec);
+				 state->current_avg_rate);
 
 		/*
 		 * Bounds check, so we don't overrun the suffix buffer. This
diff --git a/src/pv/state.c b/src/pv/state.c
index be41fa2..ed9ff7f 100644
--- a/src/pv/state.c
+++ b/src/pv/state.c
@@ -7,8 +7,23 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 
 
+/* alloc / realloc history buffer */
+static void pv_alloc_history(pvstate_t state)
+{
+	if (state->history)
+		free(state->history);
+	
+	assert(state->history_len);
+	assert(state->history_interval);
+
+	state->history = calloc(state->history_len, sizeof(state->history[0]));
+	state->history_first = state->history_last = 0;
+	state->history[0].elapsed_sec = 0.0;  /* to be safe, memset() not recommended for doubles */
+}
+
 /*
  * Create a new state structure, and return it, or 0 (NULL) on error.
  */
@@ -36,6 +51,10 @@ pvstate_t pv_state_alloc(const char *program_name)
 #endif				/* HAVE_SPLICE */
 	state->display_visible = false;
 
+	state->history_len = 30+1;  /* default history for current avg rate */
+	state->history_interval = 1;
+	pv_alloc_history(state);
+
 	return state;
 }
 
@@ -56,6 +75,10 @@ void pv_state_free(pvstate_t state)
 		free(state->transfer_buffer);
 	state->transfer_buffer = NULL;
 
+	if (state->history)
+		free(state->history);
+	state->history = NULL;
+	
 	free(state);
 
 	return;

From 6bdb951af4c43fab6536889443264c1384cd3de8 Mon Sep 17 00:00:00 2001
From: lemonsqueeze <lemonsqueeze@gmx.com>
Date: Fri, 25 May 2018 23:17:58 +0200
Subject: [PATCH 2/2] Added -m, --eta-window option: set time window for
 average rate computation (default 30s)

---
 doc/quickref.1.in     |  7 ++++++-
 src/include/options.h |  1 +
 src/include/pv.h      |  1 +
 src/main/help.c       |  2 ++
 src/main/main.c       |  1 +
 src/main/options.c    |  8 +++++++-
 src/pv/state.c        | 17 +++++++++++++----
 7 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/doc/quickref.1.in b/doc/quickref.1.in
index 823507f..ab54171 100644
--- a/doc/quickref.1.in
+++ b/doc/quickref.1.in
@@ -143,7 +143,7 @@ transfer.
 .TP
 .B \-a, \-\-average\-rate
 Turn the average rate counter on.  This will display the current average
-rate of data transfer (default: last 30s).
+rate of data transfer (default: last 30s, see --eta-window).
 .TP
 .B \-b, \-\-bytes
 Turn the total byte counter on.  This will display the total amount of
@@ -265,6 +265,11 @@ Wait
 seconds between updates.  The default is to update every second.
 Note that this can be a decimal such as 0.1.
 .TP
+.B \-m SEC, \-\-eta-window SEC
+Compute current average rate over a
+.B SEC
+seconds window for ETA calculation (default 30s).
+.TP
 .B \-w WIDTH, \-\-width WIDTH
 Assume the terminal is
 .B WIDTH
diff --git a/src/include/options.h b/src/include/options.h
index 90d8f85..9de9e68 100644
--- a/src/include/options.h
+++ b/src/include/options.h
@@ -42,6 +42,7 @@ struct opts_s {           /* structure describing run-time options */
 	double delay_start;            /* delay before first display */
 	unsigned int watch_pid;	       /* process to watch fds of */
 	int watch_fd;		       /* fd to watch */
+	unsigned int eta_window;       /* time window in seconds for eta calculations */
 	unsigned int width;            /* screen width */
 	unsigned int height;           /* screen height */
 	char *name;                    /* process name, if any */
diff --git a/src/include/pv.h b/src/include/pv.h
index cde7d08..26130e9 100644
--- a/src/include/pv.h
+++ b/src/include/pv.h
@@ -93,6 +93,7 @@ extern void pv_state_name_set(pvstate_t, const char *);
 extern void pv_state_format_string_set(pvstate_t, const char *);
 extern void pv_state_watch_pid_set(pvstate_t, unsigned int);
 extern void pv_state_watch_fd_set(pvstate_t, int);
+extern void pv_state_eta_window_set(pvstate_t, int);
 
 extern void pv_state_inputfiles(pvstate_t, int, const char **);
 
diff --git a/src/main/help.c b/src/main/help.c
index 2ebab41..7d9da6f 100644
--- a/src/main/help.c
+++ b/src/main/help.c
@@ -40,6 +40,8 @@ void display_help(void)
 		 N_("show data transfer rate counter")},
 		{"-a", "--average-rate", 0,
 		 N_("show data transfer average rate counter")},
+		{"-m", "--eta-window", N_("SEC"),
+		 N_("compute current average rate over a SEC seconds window for ETA (default 30s)")},
 		{"-b", "--bytes", 0,
 		 N_("show number of bytes transferred")},
 		{"-T", "--buffer-percent", 0,
diff --git a/src/main/main.c b/src/main/main.c
index 75fbae2..5e0f75f 100644
--- a/src/main/main.c
+++ b/src/main/main.c
@@ -206,6 +206,7 @@ int main(int argc, char **argv)
 	pv_state_format_string_set(state, opts->format);
 	pv_state_watch_pid_set(state, opts->watch_pid);
 	pv_state_watch_fd_set(state, opts->watch_fd);
+	pv_state_eta_window_set(state, opts->eta_window);
 
 	pv_state_set_format(state, opts->progress, opts->timer, opts->eta,
 			    opts->fineta, opts->rate, opts->average_rate,
diff --git a/src/main/options.c b/src/main/options.c
index 0d370ed..35e501d 100644
--- a/src/main/options.c
+++ b/src/main/options.c
@@ -80,12 +80,13 @@ opts_t opts_parse(int argc, char **argv)
 		{"remote", 1, NULL, (int) 'R'},
 		{"pidfile", 1, NULL, (int) 'P'},
 		{"watchfd", 1, NULL, (int) 'd'},
+		{"eta-window", 1, NULL, (int) 'm'},
 		{NULL, 0, NULL, 0}
 	};
 	int option_index = 0;
 #endif
 	char *short_options =
-	    "hVpteIrabTA:fnqcWD:s:l0i:w:H:N:F:L:B:CESR:P:d:";
+	    "hVpteIrabTA:fnqcWD:s:l0i:w:H:N:F:L:B:CESR:P:d:m:";
 	int c, numopts;
 	unsigned int check_pid;
 	int check_fd;
@@ -121,6 +122,7 @@ opts_t opts_parse(int argc, char **argv)
 	opts->delay_start = 0;
 	opts->watch_pid = 0;
 	opts->watch_fd = -1;
+	opts->eta_window = 30;
 
 	do {
 #ifdef HAVE_GETOPT_LONG
@@ -144,6 +146,7 @@ opts_t opts_parse(int argc, char **argv)
 		case 'L':
 		case 'B':
 		case 'R':
+		case 'm':
 			if (pv_getnum_check(optarg, PV_NUMTYPE_INTEGER) !=
 			    0) {
 				fprintf(stderr, "%s: -%c: %s\n",
@@ -310,6 +313,9 @@ opts_t opts_parse(int argc, char **argv)
 			(void) sscanf(optarg, "%u:%d", &(opts->watch_pid),
 				      &(opts->watch_fd));
 			break;
+		case 'm':
+			opts->eta_window = pv_getnum_ui(optarg);
+			break;
 		default:
 #ifdef HAVE_GETOPT_LONG
 			fprintf(stderr,
diff --git a/src/pv/state.c b/src/pv/state.c
index ed9ff7f..96c27b7 100644
--- a/src/pv/state.c
+++ b/src/pv/state.c
@@ -51,10 +51,6 @@ pvstate_t pv_state_alloc(const char *program_name)
 #endif				/* HAVE_SPLICE */
 	state->display_visible = false;
 
-	state->history_len = 30+1;  /* default history for current avg rate */
-	state->history_interval = 1;
-	pv_alloc_history(state);
-
 	return state;
 }
 
@@ -234,6 +230,19 @@ void pv_state_watch_fd_set(pvstate_t state, int val)
 	state->watch_fd = val;
 };
 
+void pv_state_eta_window_set(pvstate_t state, int val)
+{
+	if (val >= 20) {
+		state->history_len = val / 5 + 1;
+		state->history_interval = 5;
+	}
+	else {
+		state->history_len = val + 1;
+		state->history_interval = 1;
+	}
+	pv_alloc_history(state);
+};
+
 
 /*
  * Set the array of input files.