From d8d33ac419691b57fae091e9dd46c1bb974ab11b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Dec 2023 21:52:51 -0500 Subject: [PATCH 01/10] Add a backup strategy for non-standard formats. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 44 ++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 642d837ec..3b8eb56a4 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -103,6 +103,28 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di } } + // If searching for sector prologues didn't work, look for runs of FF FF FF FF FF. + if(sync_starts.empty()) { + for(size_t index = 0; index < track_data.size(); ++index) { + if(track_data[index] == 0xff) { + size_t length = 0; + size_t end = index; + while(track_data[end] == 0xff && length < 5) { + end = (end + 1) % track_data.size(); + ++length; + } + + if(length == 5) { + sync_starts.insert(index); + + while(track_data[index] == 0xff && index < track_data.size()) { + ++index; + } + } + } + } + } + PCMSegment segment; // If the track started in a sync block, write sync first. @@ -110,6 +132,15 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di segment += Encodings::AppleGCR::six_and_two_sync(int(start_index)); } + // Cap slip bits per location to avoid packing too many bits onto the track + // and thereby making it over-dense. + // + // The magic constant 51,024 comes from the quantity that most DSKs are encoded to; + // the minimum of 5 is the minimum number of FFs that must have slip bits in order to + // guarantee synchronisation. + const int max_slip_bytes_per_location = + std::max(5, int((51024 - (track_data.size() * 8)) / sync_starts.size())); + std::size_t index = start_index; for(const auto location: sync_starts) { // Write data from index to sync_start. @@ -129,8 +160,17 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di while(index < track_length && track_data[index] == 0xff) ++index; - if(index - location) - segment += Encodings::AppleGCR::six_and_two_sync(int(index - location)); + int leadin_length = int(index - location); + if(leadin_length) { + // If this is more bytes than are permitted slip bits, encode the first bunch as non-slipping FFs. + if(leadin_length > max_slip_bytes_per_location) { + std::vector ffs(size_t(leadin_length - max_slip_bytes_per_location), 0xff); + segment += PCMSegment(ffs); + leadin_length = max_slip_bytes_per_location; + } + + segment += Encodings::AppleGCR::six_and_two_sync(leadin_length); + } } // If there's still data remaining on the track, write it out. If a sync ran over From c481577f9f7c52dbf9a80430f64fdb67c09d739f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Dec 2023 23:12:41 -0500 Subject: [PATCH 02/10] Simplify, and attempt to avoid reconversions. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 141 +++++++------------------ Storage/Disk/DiskImage/Formats/NIB.hpp | 9 ++ 2 files changed, 50 insertions(+), 100 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 3b8eb56a4..13ae3d87f 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -57,133 +57,73 @@ 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. if(address.head) return nullptr; + if(address.position.as_quarter() & 2) return nullptr; long offset = file_offset(address); std::vector 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. + // about overall track positioning. The attempt works by locating + // any runs of FF and marking the last five as including slip bits. std::size_t start_index = 0; - std::set sync_starts; + std::set sync_locations; - // Establish where syncs start by finding instances of 0xd5 0xaa and then regressing - // from each along all preceding FFs. 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; - } - - // 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; - } + // 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 searching for sector prologues didn't work, look for runs of FF FF FF FF FF. - if(sync_starts.empty()) { - for(size_t index = 0; index < track_data.size(); ++index) { - if(track_data[index] == 0xff) { - size_t length = 0; - size_t end = index; - while(track_data[end] == 0xff && length < 5) { - end = (end + 1) % track_data.size(); - ++length; - } - - if(length == 5) { - sync_starts.insert(index); - - while(track_data[index] == 0xff && index < track_data.size()) { - ++index; - } - } + // 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(index); } } } 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)); - } - - // Cap slip bits per location to avoid packing too many bits onto the track - // and thereby making it over-dense. - // - // The magic constant 51,024 comes from the quantity that most DSKs are encoded to; - // the minimum of 5 is the minimum number of FFs that must have slip bits in order to - // guarantee synchronisation. - const int max_slip_bytes_per_location = - std::max(5, int((51024 - (track_data.size() * 8)) / sync_starts.size())); - 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. + 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)); + } + + // Deal with regular data. + const auto data_start = index; + while(sync_locations.find(index) == sync_locations.end() && index < track_data.size()) { + ++index; + } + if(index != data_start) { std::vector data_segment( - track_data.begin() + ptrdiff_t(index), - track_data.begin() + ptrdiff_t(location)); + track_data.begin() + ptrdiff_t(data_start), + track_data.begin() + ptrdiff_t(index)); segment += PCMSegment(data_segment); } - - // 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) - ++index; - - int leadin_length = int(index - location); - if(leadin_length) { - // If this is more bytes than are permitted slip bits, encode the first bunch as non-slipping FFs. - if(leadin_length > max_slip_bytes_per_location) { - std::vector ffs(size_t(leadin_length - max_slip_bytes_per_location), 0xff); - segment += PCMSegment(ffs); - leadin_length = max_slip_bytes_per_location; - } - - segment += Encodings::AppleGCR::six_and_two_sync(leadin_length); - } } - // 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 data_segment( - track_data.begin() + ptrdiff_t(index), - track_data.end()); - segment += PCMSegment(data_segment); - } - - return std::make_shared(segment); + std::lock_guard lock_guard(file_.get_file_access_mutex()); + cached_offset_ = offset; + cached_track_ = std::make_shared(segment);; + return cached_track_; } void NIB::set_tracks(const std::map> &tracks) { @@ -234,4 +174,5 @@ void NIB::set_tracks(const std::map> &tra file_.seek(file_offset(track.first), SEEK_SET); file_.write(track.second); } + cached_track_ = nullptr; // Conservative, but safe. } diff --git a/Storage/Disk/DiskImage/Formats/NIB.hpp b/Storage/Disk/DiskImage/Formats/NIB.hpp index 960729519..74129eae3 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.hpp +++ b/Storage/Disk/DiskImage/Formats/NIB.hpp @@ -11,6 +11,9 @@ #include "../DiskImage.hpp" #include "../../../FileHolder.hpp" +#include "../../Track/PCMTrack.hpp" + +#include 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 cached_track_; }; } From 3c3cff568a6ca13dbd06a6fc666f6a0371c71218 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Dec 2023 23:25:56 -0500 Subject: [PATCH 03/10] Remove dead variable. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 13ae3d87f..64ddf2370 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -73,9 +73,7 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di // NIB files leave sync bytes implicit and make no guarantees // about overall track positioning. The attempt works by locating // any runs of FF and marking the last five as including slip bits. - std::size_t start_index = 0; std::set sync_locations; - for(size_t index = 0; index < track_data.size(); ++index) { // Count the number of FFs starting from here. size_t length = 0; @@ -95,8 +93,7 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di } PCMSegment segment; - - std::size_t index = start_index; + std::size_t index = 0; while(index < track_data.size()) { // Deal with a run of sync values, if present. const auto sync_start = index; From 371f109a84a7560a38bafa0284782fdc461ad825 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Dec 2023 23:41:14 -0500 Subject: [PATCH 04/10] Fix mark locations. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 64ddf2370..53434b529 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -72,7 +72,8 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di // NIB files leave sync bytes implicit and make no guarantees // about overall track positioning. The attempt works by locating - // any runs of FF and marking the last five as including slip bits. + // any single run of FF that is sufficiently long and marking the last + // five as including slip bits. std::set sync_locations; for(size_t index = 0; index < track_data.size(); ++index) { // Count the number of FFs starting from here. @@ -87,8 +88,9 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di if(length >= 5) { for(int c = 0; c < 5; c++) { end = (end + track_data.size() - 1) % track_data.size(); - sync_locations.insert(index); + sync_locations.insert(end); } +// break; } } From 28cb0ad029b0fbe9887a88d655b6a9de2c0cd933 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Dec 2023 23:44:26 -0500 Subject: [PATCH 05/10] Try permitting a single sync section only. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 53434b529..bcec46e4b 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -90,7 +90,7 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di end = (end + track_data.size() - 1) % track_data.size(); sync_locations.insert(end); } -// break; + break; } } From 39f2c8097e0de379ec333165c318a5aa0f90b03a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Dec 2023 23:55:27 -0500 Subject: [PATCH 06/10] Remove second semicolon. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index bcec46e4b..1e1b0e3e8 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -121,7 +121,7 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di std::lock_guard lock_guard(file_.get_file_access_mutex()); cached_offset_ = offset; - cached_track_ = std::make_shared(segment);; + cached_track_ = std::make_shared(segment); return cached_track_; } From cbe5e69aa1bedce931dad82ea45941c8b1f00a01 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Dec 2023 23:56:43 -0500 Subject: [PATCH 07/10] Add exposition. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 1e1b0e3e8..b9c9d1834 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -90,6 +90,11 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di end = (end + track_data.size() - 1) % track_data.size(); sync_locations.insert(end); } + + // 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; } } From ada627d02767a35b68d3df458c58e2d4a783b21f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 9 Dec 2023 15:36:47 -0500 Subject: [PATCH 08/10] Decline to try to surface tracks past the end of the disk. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index b9c9d1834..cf2eaf8b0 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -58,6 +58,7 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di // NIBs contain data for even-numbered tracks underneath a single head only. if(address.head) return nullptr; if(address.position.as_quarter() & 2) return nullptr; + if(address.position.as_int() >= number_of_tracks) return nullptr; long offset = file_offset(address); std::vector track_data; From d658e00f261eb876bf314d42541567e0b51beddc Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 10 Dec 2023 22:59:50 -0500 Subject: [PATCH 09/10] Add exposition, expand recorded data. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index cf2eaf8b0..a85e395a8 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -55,12 +55,17 @@ 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() & 2) return nullptr; - if(address.position.as_int() >= number_of_tracks) 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 track_data; { std::lock_guard lock_guard(file_.get_file_access_mutex()); From ad6fe7529696503cbe7cb96710200b555202dc4e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 10 Dec 2023 23:07:02 -0500 Subject: [PATCH 10/10] Add yucky disk speed coupling. --- Components/DiskII/DiskII.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index b8137fb9a..d08636035 100644 --- a/Components/DiskII/DiskII.cpp +++ b/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);