From 894d196b64207e40ee14de3b887f9822e3d756cd Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 24 Mar 2020 21:34:33 -0400 Subject: [PATCH 1/6] Avoids massive overallocation where sync blocks overlap the index hole. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 37 +++++++++++++++++--------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 6748aefc9..8a20d6cf8 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -12,6 +12,9 @@ #include "../../Track/TrackSerialiser.hpp" #include "../../Encodings/AppleGCR/Encoder.hpp" +#include "../../Encodings/AppleGCR/Encoder.hpp" +#include "../../Encodings/AppleGCR/SegmentParser.hpp" + #include using namespace Storage::Disk; @@ -48,7 +51,7 @@ bool NIB::get_is_read_only() { } long NIB::file_offset(Track::Address address) { - return static_cast(address.position.as_int()) * track_length; + return long(address.position.as_int()) * track_length; } std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Disk::Track::Address address) { @@ -101,16 +104,24 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di // If the track started in a sync block, write sync first. if(start_index) { - segment += Encodings::AppleGCR::six_and_two_sync(static_cast(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. - std::vector data_segment( - track_data.begin() + static_cast(index), - track_data.begin() + static_cast(location)); - segment += PCMSegment(data_segment); + 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 data_segment( + track_data.begin() + off_t(index), + track_data.begin() + off_t(location)); + segment += PCMSegment(data_segment); + } else { + printf(""); + } + // Add a sync from sync_start to end of 0xffs, if there are // any before the end of data. @@ -119,13 +130,15 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di ++index; if(index - location) - segment += Encodings::AppleGCR::six_and_two_sync(static_cast(index - location)); + segment += Encodings::AppleGCR::six_and_two_sync(int(index - location)); } - // If there's still data remaining on the track, write it out. + // 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() + static_cast(index), + track_data.begin() + off_t(index), track_data.end()); segment += PCMSegment(data_segment); } @@ -149,7 +162,7 @@ void NIB::set_tracks(const std::map> &tra int bit_count = 0; size_t sync_location = 0, location = 0; for(const auto bit: segment.data) { - shifter = static_cast((shifter << 1) | (bit ? 1 : 0)); + shifter = uint8_t((shifter << 1) | (bit ? 1 : 0)); ++bit_count; ++location; if(shifter & 0x80) { @@ -167,8 +180,8 @@ void NIB::set_tracks(const std::map> &tra track.resize(track_length); } else { while(track.size() < track_length) { - std::vector extra_data(static_cast(track_length) - track.size(), 0xff); - track.insert(track.begin() + static_cast(sync_location), extra_data.begin(), extra_data.end()); + std::vector extra_data(size_t(track_length) - track.size(), 0xff); + track.insert(track.begin() + off_t(sync_location), extra_data.begin(), extra_data.end()); } } From e5cbdfc67cbee48de8e7340a9c4e466360958a44 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 24 Mar 2020 21:59:55 -0400 Subject: [PATCH 2/6] It turns out that 5-and-3 disks have a different header prologue. --- Storage/Disk/Encodings/AppleGCR/Encoder.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/Encodings/AppleGCR/Encoder.hpp b/Storage/Disk/Encodings/AppleGCR/Encoder.hpp index 08fc69a6e..3f6cdd149 100644 --- a/Storage/Disk/Encodings/AppleGCR/Encoder.hpp +++ b/Storage/Disk/Encodings/AppleGCR/Encoder.hpp @@ -16,12 +16,14 @@ namespace Storage { namespace Encodings { namespace AppleGCR { -/// Describes the standard three-byte prologue that begins a header. -const uint8_t header_prologue[3] = {0xd5, 0xaa, 0x96}; +/// Describes the standard three-byte prologue that begins a header on both the Macintosh and the Apple II from DOS 3.3. +constexpr uint8_t header_prologue[3] = {0xd5, 0xaa, 0x96}; /// Describes the standard three-byte prologue that begins a data section. -const uint8_t data_prologue[3] = {0xd5, 0xaa, 0xad}; +constexpr uint8_t data_prologue[3] = {0xd5, 0xaa, 0xad}; /// Describes the epilogue that ends both data sections and headers. -const uint8_t epilogue[3] = {0xde, 0xaa, 0xeb}; +constexpr uint8_t epilogue[3] = {0xde, 0xaa, 0xeb}; +/// Describes the standard three-byte prologue used by DOS 3.2 and earlier. +constexpr uint8_t five_and_three_header_prologue[3] = {0xd5, 0xaa, 0xb5}; namespace AppleII { From 2320b5c1fe309d45bf6b4382438f57e9615a52a6 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 25 Mar 2020 00:15:31 -0400 Subject: [PATCH 3/6] Takes some steps towards five-and-three decoding. Now I 'just' need to figure out how bits are distributed within the decoded sector. The XORing and data checksum seem the same (?) --- .../Disk/Encodings/AppleGCR/SegmentParser.cpp | 87 +++++++++++++------ 1 file changed, 59 insertions(+), 28 deletions(-) diff --git a/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp b/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp index 14d5db8c4..9c531303d 100644 --- a/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp +++ b/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp @@ -36,6 +36,25 @@ uint8_t unmap_six_and_two(uint8_t source) { return six_and_two_unmapping[source - 0x96]; } +const uint8_t five_and_three_unmapping[] = { + /* 0xab */ 0x00, 0xff, 0x01, 0x02, 0x03, + /* 0xb0 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0x04, 0x05, 0x06, + /* 0xb8 */ 0xff, 0xff, 0x07, 0x08, 0xff, 0x09, 0x0a, 0x0b, + /* 0xc0 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + /* 0xc8 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + /* 0xd0 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x0c, 0x0d, + /* 0xd8 */ 0xff, 0xff, 0x0e, 0x0f, 0xff, 0x10, 0x11, 0x12, + /* 0xe0 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + /* 0xe8 */ 0xff, 0xff, 0x13, 0x14, 0xff, 0x15, 0x16, 0x17, + /* 0xf0 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0x18, 0x19, 0x1a, + /* 0xf8 */ 0xff, 0xff, 0x1b, 0x1c, 0xff, 0x1d, 0x1e, 0x1f, +}; + +uint8_t unmap_five_and_three(uint8_t source) { + if(source < 0xab) return 0xff; + return five_and_three_unmapping[source - 0xab]; +} + } using namespace Storage::Encodings::AppleGCR; @@ -56,6 +75,7 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment // sector is still being parsed. size_t bit = 0; int header_delay = 0; + bool is_five_and_three = false; while(bit < segment.data.size() || pointer != scanning_sentinel || header_delay) { shift_register = static_cast((shift_register << 1) | (segment.data[bit % segment.data.size()] ? 1 : 0)); ++bit; @@ -77,11 +97,16 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment scanner[0] == header_prologue[0] && scanner[1] == header_prologue[1] && ( + scanner[2] == five_and_three_header_prologue[2] || scanner[2] == header_prologue[2] || scanner[2] == data_prologue[2] )) { pointer = 0; + if(scanner[2] != data_prologue[2]) { + is_five_and_three = scanner[2] == five_and_three_header_prologue[2]; + } + // If this is the start of a data section, and at least // one header has been witnessed, start a sector. if(scanner[2] == data_prologue[2]) { @@ -97,7 +122,8 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment if(new_sector) { // Check whether the value just read is a legal GCR byte, in six-and-two // encoding (which is a strict superset of five-and-three). - if(unmap_six_and_two(value) == 0xff) { + const bool is_invalid = is_five_and_three ? (unmap_five_and_three(value) == 0xff) : (unmap_six_and_two(value) == 0xff); + if(is_invalid) { // The second byte of the standard epilogue is illegal, so this still may // be a valid sector. If the final byte was the first byte of an epilogue, // chop it off and see whether the sector is otherwise intelligible. @@ -123,8 +149,7 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment default: // This is not a decodeable sector. break; - // TODO: check for five-and-three / 13-sector form sectors. - + case 411: // Potentially this is an Apple II five-and-three sector. case 343: { // Potentially this is an Apple II six-and-two sector. // Check for apparent four and four encoding. uint_fast8_t header_mask = 0xff; @@ -137,13 +162,15 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment sector->address.sector = ((header[4] << 1) | 1) & header[5]; // Check the header checksum. - uint_fast8_t checksum = ((header[6] << 1) | 1) & header[7]; + // The 0x11 is reverse engineered from the game 'Alien Rain' and is present even on the boot sector, + // so probably isn't copy protection? + uint_fast8_t checksum = (((header[6] << 1) | 1) & header[7]) ^ (is_five_and_three ? 0x11 : 0x00); if(checksum != (sector->address.volume^sector->address.track^sector->address.sector)) continue; - // Unmap the sector contents as 6 and 2 data. + // Unmap the sector contents. bool out_of_bounds = false; for(auto &c : sector->data) { - c = unmap_six_and_two(c); + c = is_five_and_three ? unmap_five_and_three(c) : unmap_six_and_two(c); if(c == 0xff) { out_of_bounds = true; break; @@ -160,30 +187,34 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment // Having checked the checksum, remove it. sector->data.resize(sector->data.size() - 1); - // Undo the 6 and 2 mapping. - const uint8_t bit_reverse[] = {0, 2, 1, 3}; - #define unmap(byte, nibble, shift) \ - sector->data[86 + byte] = static_cast(\ - (sector->data[86 + byte] << 2) | bit_reverse[(sector->data[nibble] >> shift)&3]); + if(is_five_and_three) { + // TODO + } else { + // Undo the 6 and 2 mapping. + const uint8_t bit_reverse[] = {0, 2, 1, 3}; + #define unmap(byte, nibble, shift) \ + sector->data[86 + byte] = static_cast(\ + (sector->data[86 + byte] << 2) | bit_reverse[(sector->data[nibble] >> shift)&3]); - for(std::size_t c = 0; c < 84; ++c) { - unmap(c, c, 0); - unmap(c+86, c, 2); - unmap(c+172, c, 4); + for(std::size_t c = 0; c < 84; ++c) { + unmap(c, c, 0); + unmap(c+86, c, 2); + unmap(c+172, c, 4); + } + + unmap(84, 84, 0); + unmap(170, 84, 2); + unmap(85, 85, 0); + unmap(171, 85, 2); + + #undef unmap + + // Throw away the collection of two-bit chunks from the start of the sector. + sector->data.erase(sector->data.begin(), sector->data.end() - 256); + + // Add this sector to the map. + sector->encoding = Sector::Encoding::SixAndTwo; } - - unmap(84, 84, 0); - unmap(170, 84, 2); - unmap(85, 85, 0); - unmap(171, 85, 2); - - #undef unmap - - // Throw away the collection of two-bit chunks from the start of the sector. - sector->data.erase(sector->data.begin(), sector->data.end() - 256); - - // Add this sector to the map. - sector->encoding = Sector::Encoding::SixAndTwo; result.insert(std::make_pair(sector_location, std::move(*sector))); } break; From 5fd2be3c8eed7debf32337fc30a4b18c02ff0b1a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 25 Mar 2020 20:50:26 -0400 Subject: [PATCH 4/6] Makes a genuine attempt at five and three decoding. --- .../xcschemes/Clock Signal.xcscheme | 2 +- .../Disk/Encodings/AppleGCR/SegmentParser.cpp | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 7f370216e..f049730f4 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -67,7 +67,7 @@ Storage::Encodings::AppleGCR::sectors_from_segment sector->data.resize(sector->data.size() - 1); if(is_five_and_three) { - // TODO + std::vector buffer(256); + for(size_t c = 0; c < 0x33; ++c) { + const uint8_t *const base = §or->data[0x032 - c]; + + buffer[(c * 5) + 0] = uint8_t((base[0x000] << 3) | (base[0x100] >> 2)); + buffer[(c * 5) + 1] = uint8_t((base[0x033] << 3) | (base[0x133] >> 2)); + buffer[(c * 5) + 2] = uint8_t((base[0x066] << 3) | (base[0x166] >> 2)); + buffer[(c * 5) + 3] = uint8_t((base[0x099] << 3) | ((base[0x100] & 2) << 1) | (base[0x133] & 2) | ((base[0x166] & 2) >> 1)); + buffer[(c * 5) + 4] = uint8_t((base[0x0cc] << 3) | ((base[0x100] & 1) << 2) | ((base[0x133] & 1) << 1) | (base[0x166] & 1)); + } + buffer[255] = uint8_t((sector->data[0x0ff] << 3) | (sector->data[0x199] >> 2)); + + sector->data = std::move(buffer); + sector->encoding = Sector::Encoding::FiveAndThree; } else { // Undo the 6 and 2 mapping. const uint8_t bit_reverse[] = {0, 2, 1, 3}; @@ -212,9 +225,9 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment // Throw away the collection of two-bit chunks from the start of the sector. sector->data.erase(sector->data.begin(), sector->data.end() - 256); - // Add this sector to the map. sector->encoding = Sector::Encoding::SixAndTwo; } + // Add this sector to the map. result.insert(std::make_pair(sector_location, std::move(*sector))); } break; From ea26f4f7bff56a4299c562fafdeab73437822cc3 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 25 Mar 2020 21:22:30 -0400 Subject: [PATCH 5/6] Eliminates test code, adds a caveat. --- Storage/Disk/DiskImage/Formats/NIB.cpp | 8 ++++---- Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/NIB.cpp b/Storage/Disk/DiskImage/Formats/NIB.cpp index 8a20d6cf8..4b9eb4d35 100644 --- a/Storage/Disk/DiskImage/Formats/NIB.cpp +++ b/Storage/Disk/DiskImage/Formats/NIB.cpp @@ -87,7 +87,10 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di ++length; } - // Record a sync position only if there were at least five FFs. + // 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()); @@ -118,11 +121,8 @@ std::shared_ptr<::Storage::Disk::Track> NIB::get_track_at_position(::Storage::Di track_data.begin() + off_t(index), track_data.begin() + off_t(location)); segment += PCMSegment(data_segment); - } else { - printf(""); } - // Add a sync from sync_start to end of 0xffs, if there are // any before the end of data. index = location; diff --git a/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp b/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp index d310812ae..c4a6e2d79 100644 --- a/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp +++ b/Storage/Disk/Encodings/AppleGCR/SegmentParser.cpp @@ -188,6 +188,11 @@ std::map Storage::Encodings::AppleGCR::sectors_from_segment sector->data.resize(sector->data.size() - 1); if(is_five_and_three) { + // TODO: the above is almost certainly incorrect; Beneath Apple DOS partly documents + // the process, enough to give the basic outline below of how five source bytes are + // mapped to eight five-bit quantities, but isn't clear on the order those bytes will + // end up in on disk. + std::vector buffer(256); for(size_t c = 0; c < 0x33; ++c) { const uint8_t *const base = §or->data[0x032 - c]; From 39380c63cbc6e758f01ceb49386eb64b06e34326 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 25 Mar 2020 21:25:50 -0400 Subject: [PATCH 6/6] Throws in some consts. --- Analyser/Static/DiskII/StaticAnalyser.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Analyser/Static/DiskII/StaticAnalyser.cpp b/Analyser/Static/DiskII/StaticAnalyser.cpp index 56c7c5d3f..4c66719b7 100644 --- a/Analyser/Static/DiskII/StaticAnalyser.cpp +++ b/Analyser/Static/DiskII/StaticAnalyser.cpp @@ -20,7 +20,7 @@ namespace { Analyser::Static::Target *AppleTarget(const Storage::Encodings::AppleGCR::Sector *sector_zero) { using Target = Analyser::Static::AppleII::Target; - auto *target = new Target; + auto *const target = new Target; if(sector_zero && sector_zero->encoding == Storage::Encodings::AppleGCR::Sector::Encoding::FiveAndThree) { target->disk_controller = Target::DiskController::ThirteenSector; @@ -33,7 +33,7 @@ Analyser::Static::Target *AppleTarget(const Storage::Encodings::AppleGCR::Sector Analyser::Static::Target *OricTarget(const Storage::Encodings::AppleGCR::Sector *sector_zero) { using Target = Analyser::Static::Oric::Target; - auto *target = new Target; + auto *const target = new Target; target->rom = Target::ROM::Pravetz; target->disk_interface = Target::DiskInterface::Pravetz; target->loading_command = "CALL 800\n"; @@ -47,8 +47,8 @@ Analyser::Static::TargetList Analyser::Static::DiskII::GetTargets(const Media &m if(media.disks.empty()) return {}; // Grab track 0, sector 0: the boot sector. - auto track_zero = media.disks.front()->get_track_at_position(Storage::Disk::Track::Address(0, Storage::Disk::HeadPosition(0))); - auto sector_map = Storage::Encodings::AppleGCR::sectors_from_segment( + const auto track_zero = media.disks.front()->get_track_at_position(Storage::Disk::Track::Address(0, Storage::Disk::HeadPosition(0))); + const auto sector_map = Storage::Encodings::AppleGCR::sectors_from_segment( Storage::Disk::track_serialisation(*track_zero, Storage::Time(1, 50000))); const Storage::Encodings::AppleGCR::Sector *sector_zero = nullptr; @@ -75,7 +75,7 @@ Analyser::Static::TargetList Analyser::Static::DiskII::GetTargets(const Media &m // If the boot sector looks like it's intended for the Oric, create an Oric. // Otherwise go with the Apple II. - auto disassembly = Analyser::Static::MOS6502::Disassemble(sector_zero->data, Analyser::Static::Disassembler::OffsetMapper(0xb800), {0xb800}); + const auto disassembly = Analyser::Static::MOS6502::Disassemble(sector_zero->data, Analyser::Static::Disassembler::OffsetMapper(0xb800), {0xb800}); bool did_read_shift_register = false; bool is_oric = false;