Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(very minor) Crash with certain extended ADF file #788

Closed
mras0 opened this issue Mar 25, 2023 · 13 comments
Closed

(very minor) Crash with certain extended ADF file #788

mras0 opened this issue Mar 25, 2023 · 13 comments
Labels
Bug Something isn't working v2.4

Comments

@mras0
Copy link

mras0 commented Mar 25, 2023

Testing some stuff where I create the EADF myself, and noticed that it can in certain situations (like this:
beemoved_altmfm.zip) crash vAmiga (at least ports). The attached extended ADF works in latest WinUAE (4.9.10b2).

EDIT: Sample it's supposed to be playing is this freely available one: http://helpguide.sony.net/high-res/sample1/v1/data/Sample_BeeMoved_96kHz24bit.flac.zip

@dirkwhoffmann
Copy link
Owner

dirkwhoffmann commented Mar 26, 2023

I have trouble reproducing the crash. In the Mac version, it seems to play correctly:

Bildschirm­foto 2023-03-26 um 09 52 50

vAmigaWeb (@mithrendal's port) with Aros shows this (which is probably wrong):

Bildschirm­foto 2023-03-26 um 09 53 03

For some reason I haven't been able to install Kick 1.3. in vAmigaWeb.

vAmiga.net does not accept the disk, which is the expected behavior at the moment.

UPDATE: With Kick 1.3. it plays the music in vAmigaWeb, too:

Bildschirm­foto 2023-03-26 um 10 23 58

@mras0
Copy link
Author

mras0 commented Mar 26, 2023

Sorry, maybe false positive then, seems to work in vAmigaWeb (I'd forgotten it supported extended ADFs with just the normal extension). It's probably correct behavior in Aros (bootblock is lazy and does that if it can't AllocAbs specific chip memory areas).

Had a closer look, and for me it crashes in EXTFile::encodeStandardTrack for track 161 because adf in EXTFile::encodeDisk only holds the standard 160 tracks. The file can store 164 tracks (even though not that many are used) and unused ones are just left as plain amiga dos ones.

@dirkwhoffmann dirkwhoffmann added the Bug Something isn't working label Mar 26, 2023
@dirkwhoffmann
Copy link
Owner

for track 161 because adf in EXTFile::encodeDisk only holds the standard 160 tracks

Thanks for pointing this out! Definitely a bug.

@dirkwhoffmann
Copy link
Owner

dirkwhoffmann commented Mar 26, 2023

I think I've fixed it. The culprit was this line in EXTFile::encodeDisk:

   // Create an empty ADF
    ADFFile adf(getDiameter(), getDensity()); 

As @mras0 has already pointed out, this statement always creates an ADF with 160 tracks.

The new code looks like this:

    // Create an empty ADF
    ADFFile adf(*this);

It calles a new constructor that creates an ADF with the layout of another FloppyFile:

ADFFile(const class FloppyFile &file) throws { init(file); }

Hmm, while writing this post, I realize that this might not be a good solution, because most people will probably expect that this constructor also copies the data from file. A better solution will be to add a constructor that takes some layout descriptor... I need to think about it again...

Also, I should probably change the name EXTFile to EADFFile. I like the name EADF.

@dirkwhoffmann
Copy link
Owner

FloppyDiskDescriptor has been added. The new code looks like this:

   // Create an empty ADF
    ADFFile adf(getDescriptor());

This ensures that the created ADF uses the same layout parameters as the EADF.

@mras0
Copy link
Author

mras0 commented Mar 26, 2023

Thanks, that fixed it.

The sample seems play slightly different (worse, in my subjective opinion) in vAmiga(Web) compared to WinUAE. That would be a different "issue", and maybe vAmgia is more accurate, but if you want I can open another ticket with info.

@dirkwhoffmann
Copy link
Owner

The sample seems play slightly different (worse, in my subjective opinion) in vAmiga(Web) compared to WinUAE.

I've compared FSUAE with vAmiga Mac. To my (untrained) ears, they sound very much alike. However, they use different filters internally, so somebody with a better trained ear might still hear a difference.

vAmigaWeb sounds a bit different to me and is much louder. Maybe @mithrendal uses a too large scaling factor with the result that some amplitudes get chopped off.

@mras0
Copy link
Author

mras0 commented Mar 27, 2023

I've compared FSUAE with vAmiga Mac. To my (untrained) ears, they sound very much alike. However, they use different filters internally, so somebody with a better trained ear might still hear a difference.

vAmigaWeb sounds a bit different to me and is much louder. Maybe @mithrendal uses a too large scaling factor with the result that some amplitudes get chopped off.

I usually can't make out subtle difference like this, but in this case I think there's an audible difference in the beginning (first couple of seconds) where it shouldn't be close to clipping, though my mind may be playing tricks on me.

Now that you mention filtering, one thing I noticed is there's no filtering applied when the LED is off in vAmiga (which is the case for this test). According to https://eab.abime.net/showthread.php?t=112931 (and UAE source code) there are always low- and high-pass filter applied on A500. WinUAE doesn't seem to implement the high pass filter, but pt2-clone does (and has more information).

This is just info for you consideration, not something I expect you to do anything about :)

For reference the first couple of seconds form WinUAE & vAmigaSDL. For the latter I set the samplerate to 48000 and saved the audio output to a raw file containing (32-bit float stereo). In both cases I've normalized the output with audacity to make sure volume is the same.
BeeMoved_samples.zip

@dirkwhoffmann
Copy link
Owner

This is just info for you consideration, not something I expect you to do anything about :)

Sounds like a nice project to me and I am eager to do it. I think vAmiga would really benefit from having realistic audio filters.

The relevant parts of the pt2-clone source code are crystal clear, so it should be easy to adapt the code. I'll open up another issue for the filter project...

@mithrendal
Copy link
Contributor

mithrendal commented Apr 12, 2023

Hi @mras0 and @dirkwhoffmann

I have found another ADF which fails to insert ...

https://www.pouet.net/prod.php?which=94129

it works in SAE but not in latest vAmiga Mac v2.4.b1 and also not in v2.3

is it maybe also because of the extended format?

@mras0
Copy link
Author

mras0 commented Apr 12, 2023

It's a normal ADF, but it only contains 79 cylinders worth of data (compared to the standard 80). I bet if you pad it with 11264 zero bytes it'll work.

@dirkwhoffmann
Copy link
Owner

truncate -s +11264 demo.adf

Bildschirmfoto 2023-04-12 um 21 44 48

@dirkwhoffmann
Copy link
Owner

Part of v2.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working v2.4
Projects
None yet
Development

No branches or pull requests

3 participants