format 'commodore1581' is broken

[markusC64]

td1581-optimized.flux.7z.zip

Please see the attached file (sorry, a zip is not compressing the data enough, so I needed to use 7zip. And in order to be able to upload that to github, I needed to package the 7z inside a zip)

The file "td1581-optimized.flux" contains a flux image of the real test-/demo 1581 disk. Only flux normalisation has been done: Flux times are changed to their optimum theorectical value - making the image highly compressible.

td1581.d81 contains the well known content of that disk. That's the expected result.

fluxengine read commodore1581 -s td1581-optimized.flux -o foo.d81

yields many errors like:

0.0: 500 ms in 145284 bytes 60 records, 30 sectors; 1.66us clock (602kHz); 10 distinct sectors; Required sector 0 missing; logical track 0.1; 15360 bytes decoded. 0.1: 500 ms in 145323 bytes 60 records, 30 sectors; 1.66us clock (603kHz); 10 distinct sectors; Required sector 0 missing; logical track 0.0; 15360 bytes decoded.

and the resulting image is malformed.

Also fluxengine write commodore1581 -i td1581.d81 -d foo.scp creates an scp file with malformed sector headers - an original commodore 1581 disk does not contain sector 0, but the generated .scp does.

[markusC64]

I have tested this again with the latest snapshot available for download.

Now the terminal output looks quite good:

0.0: 492 ms in 145284 bytes no more data; giving up 60 raw records, 30 raw sectors; 1.66us clock (602kHz) sectors: 0.0.0? 0.0.1? 0.0.2? 0.0.3? 0.0.4? 0.0.5? 0.0.6? 0.0.7? 0.0.8? 0.0.9? 0.1.1 0.1.2 0.1.3 0.1.4 0.1.5 0.1.6 0.1.7 0.1.8 0.1.9 0.1.10 5120 bytes decoded 0.1: 492 ms in 145323 bytes no more data; giving up 60 raw records, 30 raw sectors; 1.66us clock (602kHz) sectors: 0.0.1 0.0.2 0.0.3 0.0.4 0.0.5 0.0.6 0.0.7 0.0.8 0.0.9 0.0.10 0.1.0? 0.1.1? 0.1.2? 0.1.3? 0.1.4? 0.1.5? 0.1.6? 0.1.7? 0.1.8? 0.1.9? 5120 bytes decoded 1.0: 492 ms in 145242 bytes no more data; giving up 60 raw records, 30 raw sectors; 1.66us clock (602kHz) sectors: 1.0.0? 1.0.1? 1.0.2? 1.0.3? 1.0.4? 1.0.5? 1.0.6? 1.0.7? 1.0.8? 1.0.9? 1.1.1 1.1.2 1.1.3 1.1.4 1.1.5 1.1.6 1.1.7 1.1.8 1.1.9 1.1.10 5120 bytes decoded 1.1: 492 ms in 145284 bytes no more data; giving up 60 raw records, 30 raw sectors; 1.66us clock (602kHz) sectors: 1.0.1 1.0.2 1.0.3 1.0.4 1.0.5 1.0.6 1.0.7 1.0.8 1.0.9 1.0.10 1.1.0? 1.1.1? 1.1.2? 1.1.3? 1.1.4? 1.1.5? 1.1.6? 1.1.7? 1.1.8? 1.1.9? 5120 bytes decoded

but the resulting .d81 file is still malformed.

[davidgiven]

Hmm, interesting --- the '0.0.1?' entries are saying that it's looking for but not finding a sector on track 0, head 0, sector ID 1; the '0.1.1' is saying that it's seen a sector on track 0, head 1, sector ID 1. So it's somehow seen the wrong head. I notice that the format definition is inconsistent in its use of swap_sides, which is almost certainly a bug. You could try adding --decoder.ibm.trackdata.swap_sides=true --layout.layoutdata.physical.start_sector=1 and seeing if that helps. I'll test this myself when I get the chance.

I have actually just done a huge rework of the sector layout stuff, to make it less awful, so I'm unsurprised that there's fallout. Sorry about that!

[markusC64]

Thanks. I'll test that.

BTW: It might be a good idea to add the sample file from this issue to the fluxengine-testdata repo.

[markusC64]

Am I making something wrong?

I've used the sample data from the initial post of this issue. Then I ran this command:

fluxengine read commodore1581 -s td1581-optimized.flux -o foo.d81

After that I compared »foo.d81« with the expected image »td1581.d81« from the sample data (using »fc /b td1581.d81 foo.d81« in Windows). More or less completly different. The Software DirMaster from Style confirms this result: It can open td1581.d81, but not foo.d81.

[markusC64]

And

fluxengine write commodore1581 -i td1581.d81 -d foo.scp --drive.rotational_period_ms 500

yields in the wrong result, too. To check that image, I've used

gw convert foo.scp foo.scp.d81

which results in

Converting c=0-79:h=0-1 -> c=0-79:h=0-1 Format commodore.1581 SCP: Imported legacy single-sided image T0.0: IBM MFM (10/10 sectors) from Raw Flux (48210 flux in 498.77ms) T1.0: Ignoring unexpected sector C:0 H:0 R:8 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:5 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:6 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:3 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:1 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:10 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:4 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:2 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:9 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:7 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:8 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:5 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:6 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:3 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:1 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:10 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:4 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:2 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:9 N:2 T1.0: Ignoring unexpected sector C:0 H:0 R:7 N:2 T1.0: IBM MFM (0/10 sectors) from Raw Flux (48220 flux in 498.77ms) T2.0: Ignoring unexpected sector C:1 H:1 R:1 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:10 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:2 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:7 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:5 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:3 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:8 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:6 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:4 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:9 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:1 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:10 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:2 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:7 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:5 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:3 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:8 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:6 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:4 N:2 T2.0: Ignoring unexpected sector C:1 H:1 R:9 N:2 T2.0: IBM MFM (0/10 sectors) from Raw Flux (48201 flux in 498.77ms)

while it works with the refence flux (scp format) [created with »fluxengine rawread -s td1581-optimized.flux -d bar.scp«].

[markusC64]

Flux to d81: It turns out if I swap the sides in the resulting d81 file, then it is good.

d81 to flux: I had to repair the header: Endtrack should not be 00. Number of sides should be both. After that the image has been nearly ok, only the sector data of the sides has tio be swapped (the sector headers are ok).

[davidgiven]

Hmm.

I can actually repeat that with your flux file. But I also have a Kryoflux stream of the same disk (I was really confused until I realised this... I wasn't expecting the resulting data to all be the same!) and it works there. My initial thought is that the flux file is bad, i.e. the tracks are labelled backwards. That might be possible depending when you made it. But I tried actually writing a C64 image to disk, and that's failing because due to verification mismatches --- it's clearly seeing sectors with the wrong label.

I haven't found a reference yet for how the 1581 actually stores its data; i.e. if the data itself is on the opposite side, or whether the data is on the usual side (side 0 on the bottom, side 1 on the top) and it's just the side bytes in the sector headers which are inverted.

[markusC64]

Oh well, I think that is easy to explain:

The 1581 (and the CMD FD2000 and CMD FD4000) all swap the sides together with the data. But when reading, the controller ignores the side byte inside the sector header . It just accesses the physical "right" side. So yes, with wrong sector headers, the disk will work on a real 1581.

So you can get different images of flux data. Mine is from an original disk I got from ebay. If you write the disk with a pc (there is ms dos software for that available), then sector headers will probably be pc like regarding the side byte).

Yes, 1581 info's hard to find, that's why I bought the 1581 test demo disk from ebay.

[markusC64]

BTW: The flux file has been created from valid .scp by converting it with an older version of fluxengine. The mentined greaseweazle issue has the original .scp attached. It really has the same test results.

[davidgiven]

Okay, let's try this one. It now roundtrips correctly, and will decode both the Kryoflux stream and the flux file you sent me into images which match your d81 file.

The secret, BTW, was to invert the side byte in the header when writing, ignore it on reading, but not to swap the actual data...

[hpingel]

I'm just testing the latest state and 1581 is broken. It needs swap_sides. I come back every year to fight for swap_sides. But let me try to get it working somehow so that I can provide a PR.

[markusC64]

That's right. Without swapping the sides, the roundtrip may work, but the generated d81 file cannot match the expected one. In case of Kryoflux, the option "-oo4" is needed, which means "side 1 then side 0" for each track of the image file. That is what e mean by swapping sides.

[markusC64]

I can confirm that these are the only problems from Fluxengine with d81:

• Sides need to be swapped.

• Header of generated .scp defective: end track (byte $7) and sides (byte $A) must be corrected.

If one provides a bad workaround by swapping the pages in the D81 after read of fluxengine resp. before write of fluxengine, then when reading the generated repaired D81 is okay and when writing the generated .scp file (header corrected by hex editor) is good.

[davidgiven]

Aaargh.

[davidgiven]

facepalm

I managed to overwrite the td1581.img file I got from @markusC64 and so have been testing all along against the wrong image...

[davidgiven]

Okay, a fix is in place.

I'm not going to tempt fate by closing the bug again. PTAL?

[markusC64]

I'm going to test that version today in the evening ( gmt+2, for other time zones it might not be the evening ).

[hpingel]

It is still broken. SwapSides is not taken into account if set. I commented in the latest PR #588 instead of here yesterday.

I moved option swapSides back into the layout yesterday night in my local code but was still unsuccessful in getting it to write d81 to disk. Still investigating.

[markusC64]

Well, that's interesting:

I have converted my .flux file to .d81. That works correctly.

Then I have converted the td1581.d81 to .scp with the command I mentioned some days ago:

fluxengine write commodore1581 -i td1581.d81 -d foo.scp --drive.rotational_period_ms 500

Well, that also worked - but the resulting file still have the two bytes wrong in the header. Which breaks some tools working on that .scp file. After fixing those two bytes greaseweazle can decode that .scp file to a d81 file which equals td1581.d81. Which is great, because greaseweazle checks track, side and sector on each block header to be correctly set - it does not ignore the size.

Well, @hpingel reports writing back goes wrong. So there must be something that makes a difference when writing to a real floppy (disk). Perhaps (just a guess!) related to the wrong header values of the .scp file.

Anyway, even if my testing described above in this post is quite good, it's not perfect. I'll check the length of the gaps right now.

[markusC64]

Well, the gap lengths look fine, the same as on my sample file. The gap content differs. But you generated gap content looks valid. The whole track is 1 byte too short, which is no problem at all because its clearly in rotation tolerance.

So if writing to real floppy (disk) fails, it's probably something with metadata, because the flux is correctly generated. Remeber I have to correct two bytes in the header of the .scp file before it is good.

[markusC64]

Hm, I cannot write back to real floppy for some reason with my GreaseWeazle V4.

With the GreaseWeazle Software, writing works. With fkuxengine software, I get a verify error and abortion after the very first track.

[davidgiven]

I rewrote the entire layout engine. It now round-trips correctly and reads the demo disk flux files correctly and will write to disk and the verify works. PTAL?

[hpingel]

You da man! Great news! Thank you for your work and this great open source project!

I have good news: I have taken two d81-Image-Files from the CSDb.org and written them to real disks using your latest code and the Cypress PSOC dongle. The disks both work on my C64 with a real 1581 disk drive.

EDIT: Have also tested reading a real disk to d81 image and then did a hash comparison and it has the same content as the original image.

I haven't tested any raw flux file reading or writing yet but I gather that should work as that was what you have tested. I also haven't tested any other format than 1581.

EDIT2: Have just successfully tested the writing of commodore1541_35 to real disk. Disk works on real C64.

Thanks again and please never take my statements like "still broken" as criticism! This project is so much fun, to see it work and to somehow help to improve it.

[hpingel]

Oh, I forgot one thing: Tonight I did a scratch make as an incremental make seemed to fail. But I was also switching git branches so something got mixed up. Build speed is "slower" than one year ago, a lot must have changed. But with -j 16 my Ryzen from 2017 has something to do and it doesn't take long.

[davidgiven]

Oh, thank goodness. I thought this would never get fixed...

So, yeah, this one all boiled down to the whole layout stuff being essentially bodged together over time, and the 1581 with its weird side-swapping behaviour caused it to fall down completely. The new approach where all the mapping management is in a single place which is used everywhere is so much cleaner and easier to work with, and I'm much happier with it, so thanks for keeping poking me to do this. It'll actually let me do more cleanup, which is nice.

Yes, the makefile isn't quite right --- I haven't figured out why yet. I'd love to use a better build system but I've been searching for a suitable one for years... and yeah, it's slower now, but mostly because there are more and more complex files (the GUI stuff takes an age to build). Also, I've got a 2012 i7-3770k.

Closing now!

(Also, @markusC64: could you please file a bug for the SCP header issue?)

[markusC64]

(Also, @markusC64: could you please file a bug for the SCP header issue?)

Here you are: #593

[markusC64]

BTW: Are you interested in sample flux images from CMD FD2000 and CMD FD4000 (both C64)? Both together with the expected sector based image?

[davidgiven]

@markusC64 Yes, I am! Preferably blank ones so I don't have to worry about copyright and I can add them to the test corpus, but anything's good.

[markusC64]

I cannot provide a blank image, because I do not have that floppies. But I can provide an Image containing just one partition with just one file that has just random data.

Surely no copyright on random data.

In the »Forum 64« weÄve created that in order to be used for testing, feature requests, bug reports and likewise. I normalized the flux for extrem,e compression supporting the mentioned use cases.

It will probably be the best to open a new issue for that, asking for support of that.

[markusC64]

Ah, I forgot to mention: The idea of having that image filled with one random file is that sector missmappings in the image file are detectable.

[davidgiven]

Yup, that's absolutely fine.