Settled on the new average-of-length approach to a PLL window sizing, eliminating the old errors-of-phase approach. Since it anchors automatically to the original target clocks per bit, killed the explicit mention of a tolerance.

Storage/Disk/DigitalPhaseLockedLoop.cpp

@@ -12,13 +12,11 @@
 
 using namespace Storage;
 
-DigitalPhaseLockedLoop::DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance, size_t length_of_history) :
+DigitalPhaseLockedLoop::DigitalPhaseLockedLoop(int clocks_per_bit, size_t length_of_history) :
 		clocks_per_bit_(clocks_per_bit),
-		tolerance_(tolerance),
 		phase_(0),
 		window_length_(clocks_per_bit),
-		phase_error_pointer_(0),
-		phase_error_history_(length_of_history, 0),
+		offset_history_pointer_(0),
 		offset_history_(length_of_history, 0),
 		offset_(0) {}
 
@@ -50,9 +48,11 @@ void DigitalPhaseLockedLoop::add_pulse() {
 }
 
 void DigitalPhaseLockedLoop::post_phase_offset(int phase, int offset) {
-	offset_history_[phase_error_pointer_] = offset;
-	phase_error_pointer_ = (phase_error_pointer_ + 1) % offset_history_.size();
+	offset_history_[offset_history_pointer_] = offset;
+	offset_history_pointer_ = (offset_history_pointer_ + 1) % offset_history_.size();
 
+	// use an unweighted average of the stored offsets to compute current window size,
+	// bucketing them by rounding to the nearest multiple of the base clocks per bit
 	int total_spacing = 0;
 	int total_divisor = 0;
 	for(int offset : offset_history_) {
@@ -63,25 +63,10 @@ void DigitalPhaseLockedLoop::post_phase_offset(int phase, int offset) {
 	}
 	if(total_divisor) {
 		window_length_ = total_spacing / total_divisor;
-		window_length_ = std::max(std::min(window_length_, clocks_per_bit_ + tolerance_), clocks_per_bit_ - tolerance_);
 	}
 
 	int error = phase - (window_length_ >> 1);
 
 	// use a simple spring mechanism as a lowpass filter for phase
 	phase_ -= (error + 1) >> 1;
-
-	// use the average of the last few errors to affect frequency
-/*	size_t phase_error_history_size = phase_error_history_.size();
-
-	phase_error_history_[phase_error_pointer_] = error;
-	phase_error_pointer_ = (phase_error_pointer_ + 1)%phase_error_history_size;
-
-	int total_error = 0;
-	for(size_t c = 0; c < phase_error_history_size; c++) {
-		total_error += phase_error_history_[c];
-	}
-	int denominator = (int)(phase_error_history_size * 4);
-	window_length_ += (total_error + (denominator >> 1)) / denominator;
-	window_length_ = std::max(std::min(window_length_, clocks_per_bit_ + tolerance_), clocks_per_bit_ - tolerance_);*/
 }

Storage/Disk/DigitalPhaseLockedLoop.hpp

@@ -20,10 +20,9 @@ class DigitalPhaseLockedLoop {
 			Instantiates a @c DigitalPhaseLockedLoop.
 
 			@param clocks_per_bit The expected number of cycles between each bit of input.
-			@param tolerance The maximum tolerance for bit windows — extremes will be clocks_per_bit ± tolerance.
 			@param length_of_history The number of historic pulses to consider in locking to phase.
 		*/
-		DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance, size_t length_of_history);
+		DigitalPhaseLockedLoop(int clocks_per_bit, size_t length_of_history);
 
 		/*!
 			Runs the loop, impliedly posting no pulses during that period.
@@ -52,10 +51,9 @@ class DigitalPhaseLockedLoop {
 		Delegate *delegate_;
 
 		void post_phase_offset(int phase, int offset);
-		std::vector<int> phase_error_history_;
-		size_t phase_error_pointer_;
 
 		std::vector<int> offset_history_;
+		size_t offset_history_pointer_;
 		int offset_;
 
 		int phase_;

Storage/Disk/DiskController.cpp

@@ -166,7 +166,7 @@ void Controller::set_expected_bit_length(Time bit_length) {
 	// this conversion doesn't need to be exact because there's a lot of variation to be taken
 	// account of in rotation speed, air turbulence, etc, so a direct conversion will do
 	int clocks_per_bit = (int)cycles_per_bit_.get_unsigned_int();
-	pll_.reset(new DigitalPhaseLockedLoop(clocks_per_bit, clocks_per_bit / 5, 3));
+	pll_.reset(new DigitalPhaseLockedLoop(clocks_per_bit, 3));
 	pll_->set_delegate(this);
 }
 

Storage/Tape/Parsers/Acorn.cpp

@@ -15,7 +15,7 @@ const int PLLClockRate = 1920000;
 }
 
 Parser::Parser() :
-	::Storage::Tape::PLLParser<SymbolType>(PLLClockRate, PLLClockRate / 4800, PLLClockRate / 24000),
+	::Storage::Tape::PLLParser<SymbolType>(PLLClockRate, PLLClockRate / 4800),
 	crc_(0x1021, 0x0000) {}
 
 int Parser::get_next_bit(const std::shared_ptr<Storage::Tape::Tape> &tape) {

Storage/Tape/Parsers/TapeParser.hpp

@@ -158,9 +158,9 @@ template <typename SymbolType> class PLLParser:
 	public Parser<SymbolType> {
 	public:
 		/// Instantiates a new parser with the supplied @c tape.
-		PLLParser(int clock_rate, int clocks_per_bit, int tolerance) :
+		PLLParser(int clock_rate, int clocks_per_bit) :
 			clock_rate_(clock_rate),
-			pll_(clocks_per_bit, tolerance, 15),
+			pll_(clocks_per_bit, 15),
 			input_bit_counter_(0),
 			input_pattern_(0),
 			was_high_(false) {