#450 Cabal - DSK version not working
Krasimir Hristov has reported a problem loading disk images with sectors that have extra data:
https://sourceforge.net/p/fuse-emulator/mailman/message/36603534/
Affected images are (at least):
- 007 Trans-Master (1988)(ZX-Guaranteed)(Side A)[samdisk].dsk
- Action Countdown (1989)(US Gold)(Side A).dsk
- Action Countdown (1989)(US Gold)(Side B).dsk
- Cabal (1989)(Ocean)[samdisk].dsk
- NARC (1990)(Erbe Software)(Side A)[re-release].dsk
and Simon Owen has stated:
I can understand this disk not working in fuse, as some sectors have extra data. Cyl 2 Sector 17 is 256 bytes, but there are 256+350 bytes stored. Fuse probably doesn't skip the right amount of data before reading the next sector.
I've seen other programs with the same problem. The EDSK specification records the number of stored bytes, but some programs assume the sector size is the number of stored bytes. SAMdisk bends the rules, but the other programs have a bug. It's possible this is the bug in Fuse too.
I think it has a problem when processing the data inside the image, because it doesn't understand the extensions used. When it reaches a point where it doesn't understand the data it will return an error. I think the error is reported as "failed to open", but it means "failed to process".
Fuse needs to respect the "stored data size" for EDSK sectors. That seems to be the main cause of these failing disk images.
Related
Bugs: #451
Bugs: #452
Bugs: #454
Bugs: #472
Bugs: #473
Bugs: #474
Bugs: #475
Bugs: #476
Diff:
Hi!
Just to make clear, Fuse respect the "stored data size", so IMHO the problem is somewhere else (as Luis write before below:)
Last edit: Gergely Szasz 2019-04-06
Thanks for the heads up, Krasimir!
I fetched the disk images and the EDSK spec and found that it's missing the "Offset-Info" block description. The code takes this block into account: if present, is always at the end of the file and can be ignored. This is the case for all these 5 disk files.
As for the failures: these 5 disks fail for different reasons:
007 has a track (36) with 238 sectors... (the other tracks have 19). bpt > 12500 is currently rejected. Track info has a lot of suspicious data, maybe a protection (unknown to me).
Action Countdown fails due to incorrect sector padding, that should be done at the last sector of the track, to the next 0x100 boundary but is being done by a fixed amount (0x80 bytes per sector with extra data).
Also, weak sector detection is not correct, it's being assumed if the sector has more than the nominal amount of data (sector size times 256) but it should only be done if this extra length is a multiple of the nominal one.
Fixing these 2 issues allowed me to load Action Countdown successfully.
Finally Cabal and NARC. These need the fixes above, but it's still not enough to make them both work.
Debugging Fuse is a pain without an IDE. Can't I use a log file of stdout?
Thanks Luis,
Hmm.. yes. I do not know, where come from the "skip Offset-Info" part:
Because it is not in 'official' spec: CPC Wiki specification
BTW the EDSK specification link on Simon's SAM site is broken :( http://www.kjthacker.f2s.com/docs/extdsk.html and the 'mirrored' spec is the same as on CPC's site...
238 sector: It has to be an error. Because the "minimal" sector ID's length (12+3 byte sync + 7 byte id + 22 byte Gap II = 44byte) x 238 + postindex Gap 32 is more than 6500 byte (10504) ... and where is the data ?? 24 byte Gap III + (minimum) 128byte + 2byte crc per sector.
More than 12500 bpt rejected: Yes, because 3" DD disks can store only 6500 or slightly more data on a track, 5.25" HD disks can store 9000-10000 byte on a track, only 3.5" HD or ED disks 17000 or 34000 byte on a track... but i did not focus on HD an ED disks when wrote the code... BTW: the >12500 byte on a track is real? BTW: for Action Countdown this track size is real?
So "Offset-info" block overrides the "padding"... You may have a spec which include "offset-info"... (maybe this just an index with words of dwords...)
In fuse we check "extra length is a multiple of the nominal one." case (seclen % idlen) but only, if the "nominal" sector length is lower or equal than 32768 ..hmm where to came from this 32768???:
gdb, ddd, or just inject some fprintf(stderr,"") to the code... ;)
Last edit: Gergely Szasz 2019-04-06
Sorry:
to
BTW: for 007 Trans-Master (1988) this track size is real?
If it's really in the take like this, I don't know, but it's completely in the dsk file: the next track is at the expected offset. (Track 0x24 at file offset 0x2AD00 followed by track 0x25 at file offset 0x39B00, as can be expected for a 0xEE track length)
The track info block is like this:
Track Number 0x24
Side Number 0x00
Data Rate 0x01 (Single or double density)
Rec. Mode 0x00 (Unknown)
Sector size 0x01 (unlike all the other tracks, 0x02)
Num. sectors 0x0F (unlike all the other tracks 0x09)
Gap len. 0x52
Filler byte 0xE5
And then the (0x0F) weird sector infos:
(1st column should be 0x24, 2nd should be 0x00, 3rd should be sector ID, and 4th the nominal sector size. Last two columns are the LSB and MSB of "actual sector size")
0x77 0x77 0x77 0x77 0x20 0x20 0x00 0x20
0x3E 0x41 0x01 0x01 0x00 0x00 0x00 0x01
0x00 0xCD 0x4B 0x01 0x00 0x00 0x00 0x01
0x06 0x00 0x1E 0x00 0x20 0x20 0x80 0x00
0x21 0x00 0x06 0xCD 0x20 0x20 0x00 0x20
0x36 0x01 0x01 0x00 0x20 0x20 0x80 0x00
0x00 0x11 0x00 0x04 0x20 0x20 0x00 0x08
0x21 0x92 0x72 0xCD 0x20 0x20 0x00 0x20
0x12 0x01 0x21 0xE0 0x20 0x20 0x00 0x20
0x50 0x22 0x86 0x5C 0x20 0x20 0x00 0x20
0x3E 0x21 0x32 0x8A 0x20 0x20 0x00 0x20
0x5C 0xC3 0x92 0x72 0x20 0x20 0x00 0x20
0x00 0x00 0x00 0x00 0x20 0x20 0x80 0x00
0x00 0x00 0x00 0x00 0x20 0x20 0x80 0x00
0x00 0x00 0x00 0x00 0x20 0x20 0x80 0x00
Summing all the "actual sector sizes", rounding to the next 256 (128 bytes padding) and adding the track info block length gives the 0xEE00 bytes stored on the dsk file.
Now...is this a "protected" track? The data is mostly filler bytes, but it also has some data (that seems dummy). Maybe SAMDisk got confused by something and spit a lot of garbage...
Here is a detailed info from the track no. 36.
There are a lot of "weird" sector.. Yes maybe it is "weak" data, but rather "natural" than a copy protection, because (if i remember well) copy protections use only weak sector data (a hundred) with readable headers.
There are some "healthy" sectors (no. 2 and no.3), all others are with read errors (data checksum error).
Fuse emulate disks close to real. It converts all 'other' format to an UDI like "inner" format. Every track is a "byte stream" with "clock marks", "weak marks", etc... Every track is equal length (e.g. 6500 bpt) as in "real life". So the problem with this track in this EDSK that, the length is 60544byte. So i think, the program which created this image, several times wrap around when read. The three 8192 byte sector may include all the track data... So, the track looks like totally wrong.
We try to:
1. mark as unformatted track with no data
2. drop and/or truncate some "sector" to fit on a "regular" track (e.g. all except the first, which truncate to 6250 byte...)
to fuse "can read" the .dsk
btw, a attached a perl script (cpc.pl) and all the disk details.
{ "-bin" => "Track-Info\r\n\0...", "-fill" => 229, "-formatted" => "yes", "-glen" => 82, "-magic" => "Track-Info\r\n\0", "-nsec" => 15, "-padding" => 0, "-side" => 0, "-sinfo" => [ { "-C" => 119, "-H" => 119, "-N" => 119, "-R" => 119, "-ST1" => 32, "-ST2" => 32, "-bin" => "wwww \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 62, "-H" => 65, "-N" => 1, "-R" => 1, "-ST1" => 0, "-ST2" => 0, "-bin" => ">A\1\1\0\0\0\1", "-dlen" => 256, "-nlen" => 256, "-weakread" => 0 }, { "-C" => 0, "-H" => 205, "-N" => 1, "-R" => 75, "-ST1" => 0, "-ST2" => 0, "-bin" => "\0\315K\1\0\0\0\1", "-dlen" => 256, "-nlen" => 256, "-weakread" => 0 }, { "-C" => 6, "-H" => 0, "-N" => 0, "-R" => 30, "-ST1" => 32, "-ST2" => 32, "-bin" => "\6\0\36\0 \200\0", "-dlen" => 128, "-nlen" => 128, "-weakread" => 0 }, { "-C" => 33, "-H" => 0, "-N" => 205, "-R" => 6, "-ST1" => 32, "-ST2" => 32, "-bin" => "!\0\6\315 \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 54, "-H" => 1, "-N" => 0, "-R" => 1, "-ST1" => 32, "-ST2" => 32, "-bin" => "6\1\1\0 \200\0", "-dlen" => 128, "-nlen" => 128, "-weakread" => 0 }, { "-C" => 0, "-H" => 17, "-N" => 4, "-R" => 0, "-ST1" => 32, "-ST2" => 32, "-bin" => "\0\21\0\4 \0\b", "-dlen" => 2048, "-nlen" => 2048, "-weakread" => 0 }, { "-C" => 33, "-H" => 146, "-N" => 205, "-R" => 114, "-ST1" => 32, "-ST2" => 32, "-bin" => "!\222r\315 \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 18, "-H" => 1, "-N" => 224, "-R" => 33, "-ST1" => 32, "-ST2" => 32, "-bin" => "\22\1!\340 \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 80, "-H" => 34, "-N" => 92, "-R" => 134, "-ST1" => 32, "-ST2" => 32, "-bin" => "P\"\206\\ \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 62, "-H" => 33, "-N" => 138, "-R" => 50, "-ST1" => 32, "-ST2" => 32, "-bin" => ">!2\212 \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 92, "-H" => 195, "-N" => 114, "-R" => 146, "-ST1" => 32, "-ST2" => 32, "-bin" => "\\\303\222r \0 ", "-dlen" => 8192, "-nlen" => 0, "-weakread" => "N error" }, { "-C" => 0, "-H" => 0, "-N" => 0, "-R" => 0, "-ST1" => 32, "-ST2" => 32, "-bin" => "\0\0\0\0 \200\0", "-dlen" => 128, "-nlen" => 128, "-weakread" => 0 }, { "-C" => 0, "-H" => 0, "-N" => 0, "-R" => 0, "-ST1" => 32, "-ST2" => 32, "-bin" => "\0\0\0\0 \200\0", "-dlen" => 128, "-nlen" => 128, "-weakread" => 0 }, { "-C" => 0, "-H" => 0, "-N" => 0, "-R" => 0, "-ST1" => 32, "-ST2" => 32, "-bin" => "\0\0\0\0 \200\0", "-dlen" => 128, "-nlen" => 128, "-weakread" => 0 } ], "-slen" => 256, "-slencode" => 1, "-snum" => 0, "-tdlen" => 60544, "-tlen" => 60672, "-tnum" => 36, "-track" => 36 },Here we were talking about "007 Trans-Master" disk dump.
Other option is refuse to load the disk. The data looks too much weird to apply an heuristic to rebuild a low level track.
samdisk scan command reports this layout:
samdisk warns if the gap before the first visible sector is suspiciously large, and the wrapping gap is large enough to hide a 256-byte sector.
Probably the preservation team should read the whole track (raw) and do an analysis to validate the sectors.
The "Offset-Info" block is an extension to the EDSK specification by Simon Owen, generated mainly by samdisk:
https://simonowen.com/misc/extextdsk.txt
Fuse had some trouble reading samdisk disk images [bugs:#307] even when this info was unnecessary, so the fix at the time was to skip this block.
Of course, there might be disk images that require an interpretation of the "Offset-Info" block. And the saving routine should keep/update this info, too.
Related
Bugs:
#307Thanks... so offset info just for the physhical layout on the disk...
Now we can save only in DSK not EDSK...
I fixed a silly mistake in my patch and am now able to load NARC and CABAL in addition to Action Countdown (007 has another issue, as Gergely stated above).
...Sometimes its Tracks, other times its cylinders... messy.
BTW: stderr is not showing anything on my system, so I've used win32ui_confirm for debugging. ;-)
Changes to the 1.5.7 source (all within peripherals\disk\disk.c):
$ diff disk_157.c disk.c
Just for readability...
1617c1617
< if( i < 84 ) fix[i] = 0;
We should not reject sectors with extra data, just because it's not a multiple of the nominal length.
1633,1635c1633,1635
< if( idlen != 0 && idlen <= ( 0x80 << 0x08 ) && / idlen is o.k. /
< seclen > idlen && seclen % idlen ) / seclen != N * len /
< return d->status = DISK_OPEN;
Sector padding shouldn't be done in multiples of 128, sector_pad is now in bytes.
1665,1667c1665,1669
< if( seclen % 0x100 ) / every? 128/384/...byte length sector padded /
< sector_pad++;
< }
At the end of the track, sector_pad should just be added, not multiplied by 128.
1673c1675
< buffer->index += trlen + sector_pad * 128 + 256;
Clear and restart accumulating to find the padding at the end of each track.
1691c1693
<
Just for readability...
1709c1711
< if( i < 84 && fix[i] == 2 && j == 0 ) { / repositionate the dummy track /
And finally we data_add the correct amout of data and pad the exact amount instead of a multiple of 128.
1753,1767c1755,1785
< data_add( d, buffer, NULL, seclen > idlen ? idlen : seclen,
< hdrb[ 0x1d + 8 * j ] & 0x40 ? DDAM : NO_DDAM, gap,
< hdrb[ 0x1c + 8 * j ] & 0x20 && hdrb[ 0x1d + 8 * j ] & 0x20 ?
< CRC_ERROR : CRC_OK, 0x00, &idx );
< if( seclen > idlen ) { / weak sector with multiple copy /
< cpc_set_weak_range( d, idx, buffer, seclen / idlen, idlen );
< buffer->index += ( seclen / idlen - 1 ) * idlen;
< / ( ( N * len ) / len - 1 ) * len /
< }
< }
< if( seclen % 0x100 ) / every? 128/384/...byte length sector padded /
< sector_pad++;
< }
< gap4_add( d, gap );
< buffer->index += sector_pad * 0x80;
hmm... now we detect weak sectors in this way. The EDSK extension (2) says:
IMHO we cannot have other sector data length than: 128, 256, 512, 1024, 2048, 4096, 6144 (EDSK spec: For 8k Sectors (N="6"), only 1800h bytes is stored. ), 8192, 16384 (EDSK spec. extension 1) or NN x # (EDSK spec. ext. 2 )
If not weak sector, then what is the meaning of (sector data length > sector length from N)? How this data created? The disk controller does not read more data then determined by sector head information. So, if N=3 we got 2048 byte. No more (maybe less (?)) How we can have 582 byte data when N=2 (sector data length is 512 byte), ST1 and ST2 is 0??? IMHO this case the disk image is corrupt. :(
And what we can do with the extra data? Discard? ... (IMHO we need a "diskimagetool" util, which can repair, convert, etc defected disk images... instead of handling this in fuse ;-)
Sorry, if N=3 we got 1024 byte
So, you're saying that the only valid "stored data size" extension field usage is for "weak sectors"?
Then it is 2 bytes long (instead of storing only the MSB, like the old Track Info Block header), just to allow 128b sectors (as other sizes have LSB=0)?
And SAMDisk (the tool used to create these 5 images) should be fixed instead, not allowing such values in this field?
https://simonowen.com/blog/2009/03/21/further-edsk-extensions/
contains some more extensions, in the linked
https://simonowen.com/misc/extextdsk.txt file, already reffered by Sergio, above,
including the "Offset-Info" block and the point 4) : "special extensions for when the stored length exceeds sector length"
(Note that the "Offset-Info" header says that the header is "00-0D "Offset-Info\r\n" , which is not correct, it should be 00-0C instead or else include the extra 0x00 observed in the 5 dsk files referred in this Ticket, I can't match the remainder with the examples, though)
I also found this very informational:
https://simonowen.com/samdisk/sys_cpc/
Last edit: Luis Faria 2019-04-08
Thanks for the links ;-)
Just a note:
NARC track no. 4 has 10x 512 byte length sector (interleaved) but the last (not the first) has more data than 512 (585)...
Ahem... ;-) (btw I ask Simon about it...)
i use seclen for stored sector data length and idlen for sector data length calculated from N field of ID.
There are four case:
1. seclen == idlen -> 'normal'
2. seclen == idlen x N (where N is an integer) -> 'weak data'
3. seclen < idlen
4. seclen > idlen but not case 2.
Case 3.
- the 'original' DSK spec say: if N=6 we can store only 0x1800 (6144) byte data instead of 8192
- but for other cases, what we have to do with missing data? FDC wait eg 512 byte, but we have only 510... what we have to do? feed FDC with filler byte? or feed with random data, or what? and why? why we do not store these bytes?
Case 4.
- what we have to do with 'extra data' ? The FDC don't want to read after the 'official' length.. Why we store these bytes?
I just want to know "what's the point" :-)
I guess the FDC can be tricked into reading the data bytes followed by the CRC, GAP3 area and so on. Citing, as an example, the "Disc Manager by Goldmark Systems" protection:
Therefore, I assume we have to provide this "control" data as more normal data after the...normal data. ;)
Last edit: Luis Faria 2019-04-08
Luis, on Windows you can redirect stdout or stderr to a file (as Unix):
fuse.exe > stdout.txt 2> stderr.txtThanks for the tip, Sergio, it does work! 8-)
Hi Sergio,
Could you review related patches from [bugs:#451] [bugs:#452] [bugs:#454][bugs:#472] [bugs:#473] [bugs:#474] [bugs:#475] [bugs:#476] ... so we move forward a little bit :)
Related
Bugs: #451
Bugs: #452
Bugs: #454
Bugs: #472
Bugs: #473
Bugs: #474
Bugs: #475
Bugs: #476
Last edit: Gergely Szasz 2020-10-05