keirf / greaseweazle

Tools for accessing a floppy drive at the raw flux level
The Unlicense
978 stars 97 forks source link

"gw convert" does not work as expected #149

Closed markusC64 closed 2 years ago

markusC64 commented 2 years ago

sampleimages 7z.rename-to-7z.zip

Please take the attached set of sample images. Some of them are an IBM 360k disk image, some of them are an IBM 180k image. Each taken with different settings: Single step or double step - and (in case of 180k image) single or double sided capture.

My first test was an attempt to convert them to kryoflux:

gw convert bsa-360k-single-step.scp 1\tr_.raw

The other images likewise. While this works for all single step images as expected, for double step images the resulting files are numbered wrong. They should name "tr_00.0.raw", "tr_00.1.raw", "tr_02.0.raw", "tr02.1.raw", and so on (the "tr??.1.raw" files are only for two sided images, of cause). Okay, renaming those files helps, but this is clearly a bug.

Ok, second attampt - convert all of them to an .img file:

gw convert --format ibm.360 --rpm 300 bsa-180k-double-step-double-sided.scp 1.img

(other images likewise) works for any double step images. This time singel step images fail. I tried options to correct it like

gw convert --format ibm.360 --rpm 300 --track c=0-79:step=2 bsa-360k-single-step.scp 1.img

but none of them was successfull. One last test: Use the same image

gw convert bsa-360k-single-step.scp 1\track_.raw

and let the kryoflux software decode it to the img file - done with:

dtc -m1 -f1\track_ -i0 -f1.img -k2 -i4

and the image has been converted without any problem.

markusC64 commented 2 years ago

And one more thing: Converting IBM 180k/360k to img from a kryoflux dump does not work, also. Convert the double sided single step image to kryoflux. For our case, the result is as goog as a real kryoflux dump. And in case of 180k you can also convert the single sided single step image to kryoflux, but I do not expect any difference, because in kryoflux streams, the differenmce is only that we have additional files.

markusC64 commented 2 years ago

One more test for convert: td1581-optimized.7z.rename-to-7z.zip

This is an optimized image of the commodore 1581 test demo disk. It's optimized because flux times are changed to their optimum theorectical value - making the image highly compressible.

MichaelHuth commented 2 years ago

As format conversion that doesn't break the image data is critical I suggest to add test cases for these.

keirf commented 2 years ago

Yes test cases now there is a convert function would be good. The first thing here however is that these are pretty much design deficiencies / missing items right now, rather than bugs. I'll follow up with proposed fixes and further comments in next few days.

MichaelHuth commented 2 years ago

@keirf thanks for the status update.

This is just unlucky in the sense that we expect now images from broken conversions to appear in the wild.

keirf commented 2 years ago

I think the generated images aren't subtly broken but obviously bogus. That's actually much less worrying.

markusC64 commented 2 years ago

With the possible exception of double step images converted to kryoflux. Yes, Michael and I can see that with just looking at the filenames. But others don't see that they're broken.

keirf commented 2 years ago

One would notice when they try and use their images? I mean, if you blindly convert stuff and post it directly online, what can you expect. Actually this is why there is so much crap online already 😅

keirf commented 2 years ago

Okay, first of all, there was a bug: --tracks step= sub-option was ignored. This made it impossible to import a single-stepped 40T image read using an 80T drive as the alternating tracks are not correctly skipped.

Secondly, I think that track-conversion sub-options (step,hswap,h[01].off) should be separately specifiable for input and output images. So I have added a new option --out-tracks which defaults to same as --tracks. This means, for example, that if the input is double-stepped (step=2) then by default the output will be too unless you specify --out-tracks=step=1. I'm not sure about this, do you think that by default the output steps and hswap should be forced to 1 and False respectively?

Anyway, here is a test build for you to try now, in light of the above. Please give more details about things which remain broken, or comments on the sanity of the current command-line options. (PS. You need to be logged into github for the link below to work). https://github.com/keirf/greaseweazle/suites/4870708460/artifacts/138955943

markusC64 commented 2 years ago

Much better. Must think of reasonable defaults. For the 1581 test demo disk, the required options are non obviuos:

gw convert td1581-optimized.scp td1581.d81 --track hswap --out-track hswap --rpm 300

Converting single step images (180k, 360k) to ibm requires: --track step=2

Converting double step images to kryoflux requires: --out-track step=2

Converting a kryoflux image with no "*_?[13579].?.raw" (i. e. an odd track number) to scp: --track step=2

It's not that easy - a good default must consider source and target type and track layout...

markusC64 commented 2 years ago

.scp to .adf seems to work without any issues.

keirf commented 2 years ago

D81 image format (and Commodore 1581) is interesting. My understanding was that the sides are logically swapped but not physically swapped. By which I mean: The sector headers on conventional physical side N of the disk still contain H=N, it's just that the host considers this side 1-N (and hence, simply, physical sides are swapped in a D81 image file). That's what I implement in FlashFloppy, and noone has complained. However, if H doesn't get checked then I could be wrong and everything still appears to work anyway.

Perhaps someone who knows about 1581 and D81 images can confirm if FlashFloppy is doing the right thing as currently implemented?

EDIT: It's looking possible that sides are physically swapped. D81 image contains interleaved sides as usual, 0 then 1. But conventional physical side 0 of the 1581 disk contains side 1 (side 1 data, and header H bytes contain '1'). I can try out a patch for this....

markusC64 commented 2 years ago

Well, I actually do have an original 1581 test demo disk and can create all images you wish with a SuperCard Pro and Kryoflux. This image contains the expected result:

http://www.zimmers.net/anonftp/pub/cbm/demodisks/drives/1581demo.d81.gz

I can send you an actual dump of the test demodisk (it happens that I already have one kryoflux dump on my harddisk), but the version in my 3rd post here is actually nearby, it only has been parsed as mfm raw bitstream (with clock & databytes) and converted back. So no mfm data and metadata changed.

keirf commented 2 years ago

Thanks, that confirms that FlashFloppy is correct except that the sector H byte should be swapped too. So I will look at a fix in Greaseweazle to start with.

Incidentally, do you happen to use FluxEngine, and if so can you confirm that its Commdore 1581 handling is broken (it expects sector ids 0..9 rather than 1..10). eg fluxengine write commodore1581 -s td1581-optimized.scp -o td1581.d81

markusC64 commented 2 years ago

I just checked decoded mfm data from my real kryoflux dump (decoded with my g64conv 4.0, which can decode mfm to text now):

Track 0, Side 0:

mfmsnyc-a1 mfmsnyc-a1 mfmsnyc-a1 begin-checksum ; Header mfm-bytes fe ; Track mfm-bytes 00 ; Side mfm-bytes 01 ; Sector mfm-bytes 01 ; Sectorsize 512 mfm-bytes 02 ; Trk 0 Sec 1 Side 1 ; CRC mfm-bytes fd 5f end-checksum ; Gap mfm-bytes 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 00 00 00 00 00 00 00 00 00 00 00 00 mfm-bits 0000000 [and so on]

Track 0 Side 1:

mfmsnyc-a1 mfmsnyc-a1 mfmsnyc-a1 begin-checksum ; Header mfm-bytes fe ; Track mfm-bytes 00 ; Side mfm-bytes 00 ; Sector mfm-bytes 01 ; Sectorsize 512 mfm-bytes 02 ; Trk 0 Sec 1 Side 0 ; CRC mfm-bytes ca 6f end-checksum ; Gap mfm-bytes 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 4e 00 00 00 00 00 00 00 00 00 00 00 00 mfm-bits 0000000

keirf commented 2 years ago

Okay, here is a new d81-fixed version to try. You should find this now works: gw convert td1581-optimized.scp td1581.d81 --rpm 300

So where does that leave us? There's some need for step=2 specified here and there if doing 40T disks in 80T drives.

https://github.com/keirf/greaseweazle/suites/4871505201/artifacts/139012938

markusC64 commented 2 years ago

Right. And note Kryoflux hardware does not support 40 track floppy drives, so with kryoflux format you'll need step=2 for any 40T disk.

I must think of it.

In g64conv, I have the condition

if (largest track numer < 43) then assume double step else assume single step

You see, a single step image on a 80 track drive with just 42 tracks, that is most likely wrong to assume. So that corrects scp images so that they all are from the view of a 80 track drive. Any parsing algorithm can therefore assume the 80 track view.

And for saving into scp it's checked if "halftracks" (the 1541 word is quite fitting here) exists and if not, track numbers are condensed to 40 drive numebrs for saving into scp.

Such things might make things easier - you'll just need an ibm.singlestep and an ibm.doublestep for 5,25" drives. For 3.5" drives, ibm.singelstep can be used. For hd floppies, either the ibm.singlestep could be used, or further hd formats might be needed.

PS: There are two 360k formats: One with 40 Tracks, 2 sides and 9 sectors. The other with 80 Tracks, 1 side and 9 sectors.

markusC64 commented 2 years ago

I cannot make the current version of fluxengine to work under windows, and the newest but one build does not support commodore 1581.

The newest fluxengine build for windows is producing no output at all, no even no help message... So no test result for that. But if you happen to have it working under another operating system, I can analyze the resulting scp file.

markusC64 commented 2 years ago

scp to d81 works now as promised.

markusC64 commented 2 years ago

Thanks, that confirms that FlashFloppy is correct except that the sector H byte should be swapped too. So I will look at a fix in Greaseweazle to start with.

Incidentally, do you happen to use FluxEngine, and if so can you confirm that its Commdore 1581 handling is broken (it expects sector ids 0..9 rather than 1..10). eg fluxengine write commodore1581 -s td1581-optimized.scp -o td1581.d81

I can now confirm that - these days a new version of FluxEngine's been released that starts again with windows, but Commodore 1581 handling is broken.

keirf commented 2 years ago

Could you let David know on his project GitHub?

markusC64 commented 2 years ago

Here we are: https://github.com/davidgiven/fluxengine/issues/413