Merge pull request #1259 from TomHarte/NIBSlipBits

NIB: switch to a strategy supporting non-standard formats.

Components/DiskII/DiskII.cpp

@@ -23,8 +23,14 @@ DiskII::DiskII(int clock_rate) :
 	clock_rate_(clock_rate),
 	inputs_(input_command),
 	drives_{
-		Storage::Disk::Drive{clock_rate, 300, 1},
-		Storage::Disk::Drive{clock_rate, 300, 1}
+		// Bit of a hack here: drives are marginally slowed down compared to real drives
+		// in order to accomodate NIB files, which usually carry more data than will
+		// physically fit on a track once slip bits are reinserted.
+		//
+		// I don't like the coupling here.
+		// TODO: resolve better, somehow.
+		Storage::Disk::Drive{clock_rate, 295, 1},
+		Storage::Disk::Drive{clock_rate, 295, 1}
 	}
 {
 	drives_[0].set_clocking_hint_observer(this);

Storage/Disk/DiskImage/Formats/NIB.cpp

@@ -55,95 +55,85 @@ long NIB::file_offset(Track::Address address) {
 }
 
 std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Disk::Track::Address address) {
-	// NIBs contain data for even-numbered tracks underneath a single head only.
+	// NIBs contain data for a fixed quantity of integer-position tracks underneath a single head only.
+	//
+	// Therefore:
+	//	* reject any attempt to read from the second head;
+	//	* treat 3/4 of any physical track as formatted, the remaining quarter as unformatted; and
+	//	* reject any attempt to read beyond the defined number of tracks.
 	if(address.head) return nullptr;
+	if((address.position.as_quarter() & 3) == 3) return nullptr;
+	if(size_t(address.position.as_int()) >= number_of_tracks) return nullptr;
 
-	long offset = file_offset(address);
+	const long offset = file_offset(address);
 	std::vector<uint8_t> track_data;
 	{
 		std::lock_guard lock_guard(file_.get_file_access_mutex());
+		if(cached_offset_ == offset && cached_track_) {
+			return cached_track_;
+		}
 		file_.seek(offset, SEEK_SET);
 		track_data = file_.read(track_length);
 	}
 
 	// NIB files leave sync bytes implicit and make no guarantees
-	// about overall track positioning. My current best-guess attempt
-	// is to seek sector prologues then work backwards, inserting sync
-	// bits into [at most 5] preceding FFs. This is intended to put the
-	// Disk II into synchronisation just before each sector.
-	std::size_t start_index = 0;
-	std::set<size_t> sync_starts;
-
-	// Establish where syncs start by finding instances of 0xd5 0xaa and then regressing
-	// from each along all preceding FFs.
+	// about overall track positioning. The attempt works by locating
+	// any single run of FF that is sufficiently long and marking the last
+	// five as including slip bits.
+	std::set<size_t> sync_locations;
 	for(size_t index = 0; index < track_data.size(); ++index) {
-		// This is a D5 AA...
-		if(track_data[index] == 0xd5 && track_data[(index+1)%track_data.size()] == 0xaa) {
-			// ... count backwards to find out where the preceding FFs started.
-			size_t start = index - 1;
-			size_t length = 0;
-			while(track_data[start] == 0xff && length < 5) {
-				start = (start + track_data.size() - 1) % track_data.size();
-				++length;
+		// Count the number of FFs starting from here.
+		size_t length = 0;
+		size_t end = index;
+		while(track_data[end] == 0xff) {
+			end = (end + 1) % track_data.size();
+			++length;
+		}
+
+		// If that's at least five, regress and mark all as syncs.
+		if(length >= 5) {
+			for(int c = 0; c < 5; c++) {
+				end = (end + track_data.size() - 1) % track_data.size();
+				sync_locations.insert(end);
 			}
 
-			// Record a sync position only if there were at least five FFs, and
-			// sync only in the final five. One of the many crazy fictions of NIBs
-			// is the fixed track length in bytes, which is quite long. So the aim
-			// is to be as conservative as possible with sync placement.
-			if(length == 5) {
-				sync_starts.insert((start + 1) % track_data.size());
-
-				// If the apparent start of this sync area is 'after' the start, then
-				// this sync period overlaps position zero. So this track will start
-				// in a sync block.
-				if(start > index)
-					start_index = start;
-			}
+			// Experimental!! Permit only one run of sync locations.
+			// That should synchronise the Disk II to the nibble stream
+			// such that it remains synchronised from then on. At least,
+			// while this remains a read-only format.
+			break;
 		}
 	}
 
 	PCMSegment segment;
-
-	// If the track started in a sync block, write sync first.
-	if(start_index) {
-		segment += Encodings::AppleGCR::six_and_two_sync(int(start_index));
-	}
-
-	std::size_t index = start_index;
-	for(const auto location: sync_starts) {
-		// Write data from index to sync_start.
-		if(location > index) {
-			// This is the usual case; the only occasion on which it won't be true is
-			// when the initial sync was detected to carry over the index hole,
-			// in which case there's nothing to copy.
-			std::vector<uint8_t> data_segment(
-				track_data.begin() + ptrdiff_t(index),
-				track_data.begin() + ptrdiff_t(location));
-			segment += PCMSegment(data_segment);
+	std::size_t index = 0;
+	while(index < track_data.size()) {
+		// Deal with a run of sync values, if present.
+		const auto sync_start = index;
+		while(sync_locations.find(index) != sync_locations.end() && index < track_data.size()) {
+			++index;
+		}
+		if(index != sync_start) {
+			segment += Encodings::AppleGCR::six_and_two_sync(int(index - sync_start));
 		}
 
-		// Add a sync from sync_start to end of 0xffs, if there are
-		// any before the end of data.
-		index = location;
-		while(index < track_length && track_data[index] == 0xff)
+		// Deal with regular data.
+		const auto data_start = index;
+		while(sync_locations.find(index) == sync_locations.end() && index < track_data.size()) {
 			++index;
-
-		if(index - location)
-			segment += Encodings::AppleGCR::six_and_two_sync(int(index - location));
+		}
+		if(index != data_start) {
+			std::vector<uint8_t> data_segment(
+				track_data.begin() + ptrdiff_t(data_start),
+				track_data.begin() + ptrdiff_t(index));
+			segment += PCMSegment(data_segment);
+		}
 	}
 
-	// If there's still data remaining on the track, write it out. If a sync ran over
-	// the notional index hole, the loop above will already have completed the track
-	// with sync, so no need to deal with that case here.
-	if(index < track_length) {
-		std::vector<uint8_t> data_segment(
-			track_data.begin() + ptrdiff_t(index),
-			track_data.end());
-		segment += PCMSegment(data_segment);
-	}
-
-	return std::make_shared<PCMTrack>(segment);
+	std::lock_guard lock_guard(file_.get_file_access_mutex());
+	cached_offset_ = offset;
+	cached_track_ = std::make_shared<PCMTrack>(segment);
+	return cached_track_;
 }
 
 void NIB::set_tracks(const std::map<Track::Address, std::shared_ptr<Track>> &tracks) {
@@ -194,4 +184,5 @@ void NIB::set_tracks(const std::map<Track::Address, std::shared_ptr<Track>> &tra
 		file_.seek(file_offset(track.first), SEEK_SET);
 		file_.write(track.second);
 	}
+	cached_track_ = nullptr;	// Conservative, but safe.
 }

Storage/Disk/DiskImage/Formats/NIB.hpp

@@ -11,6 +11,9 @@
 
 #include "../DiskImage.hpp"
 #include "../../../FileHolder.hpp"
+#include "../../Track/PCMTrack.hpp"
+
+#include <memory>
 
 namespace Storage::Disk {
 
@@ -33,6 +36,12 @@ class NIB: public DiskImage {
 		FileHolder file_;
 		long get_file_offset_for_position(Track::Address address);
 		long file_offset(Track::Address address);
+
+		// Cache for the last-generated track, given that head steps on an Apple II
+		// occur in quarter-track increments, so there'll routinely be four gets in
+		// a row for the same data.
+		long cached_offset_ = 0;
+		std::shared_ptr<Storage::Disk::PCMTrack> cached_track_;
 };
 
 }