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

Query: should model=CD32 work with mp3 audio playback? #1611

Closed
giantclambake opened this issue Jan 25, 2025 · 10 comments
Closed

Query: should model=CD32 work with mp3 audio playback? #1611

giantclambake opened this issue Jan 25, 2025 · 10 comments
Assignees

Comments

@giantclambake
Copy link

Example file .... https://mega.nz/file/8uxyTZSA#Fh4xt9ixxQ_Mh4VJS9Z-VEG0hqR6RFa-BsoLXR_jV40

Reference -> https://eab.abime.net/showthread.php?t=119546

Result: direct loading the starwars_holiday.cue starts the emulation and title as expected, but none of the ingame music is heard, only SFX/samples work.

Expected: The music .mp3 files to be play/heard?

Note: the .cue file does contain capitalization errors ...ie; .MP3 vs .mp3 but correcting this has no effect - mp3 files not played.

Unimplemented?....

TIA

@giantclambake
Copy link
Author

Additional:

gcb@gallah:~/Amiberry/cdroms/superholiday$ ls
cdaudio_track_10.mp3  cdaudio_track_2.mp3  cdaudio_track_4.mp3  cdaudio_track_6.mp3  cdaudio_track_8.mp3  starwars_holiday.cue
cdaudio_track_11.mp3  cdaudio_track_3.mp3  cdaudio_track_5.mp3  cdaudio_track_7.mp3  cdaudio_track_9.mp3  starwars_holiday.iso

...is it only detecting filename.MP3 (irrespective of .cue content) ?

@giantclambake
Copy link
Author

giantclambake commented Jan 25, 2025

Confirmed: ..recall that I said last year, this capitalization thang was going to keep biting us? .... ;)

I think I saw it at...

src/blkdev_cdimage.cpp: if (!_tcsicmp (fnametype, _T("MP3")) || (ext && !_tcsicmp(ext, _T("MP3"))))

Test:

cd ~/Amiberry/cdroms/superholiday

//Rename all *.mp3 files to *.MP3

for file in *.mp3; do
    mv -- "$file" "${file%.mp3}.MP3"
done

//now filename extension capitalization matches that contained in starwars_holiday.cue

  • direct load the starwars_holiday.cue file, emulation starts immediately, and now music replay works as expected

...looking at the code, it may be that .WAV vs .wav ...and .FLAC vs .flac ....would be similarly effected?

If we look back at #1605 (towards the bottom), the better fix would have been...

mv 'Xenon 2 - Megablast (1992)(Mirrorsoft)(M5)[CDTV-CD32].bin' 'Xenon 2 - Megablast (1992)(Mirrorsoft)(M5)[CDTV-CD32].BIN'

....because the problem here is really the filename extensions, not the contents of the .cue file (...as this example goes on to prove ~ editing the .cue is not guaranteed to work ;)

This is a small pita, because the files themselves came from AmigaOS, or within emulation running on WinUAE using Windows, which are both insensitive to filename.ext capitalization ...grrr....

Anything you can do here to ease the users' pain? ;)

TIA

@midwan
Copy link
Collaborator

midwan commented Jan 25, 2025

Alright, let's see if we can make this better.

@midwan midwan self-assigned this Jan 25, 2025
midwan added a commit that referenced this issue Jan 25, 2025
…ensions (#1611)

When opening files with zfile, try to check for different capitalization cases in the file extension.
Hopefully this should cover incorrect filenames in CD images, where the .CUE file refers to one filename but what actually exists has a different capitalization (e.g. mp3 vs MP3)
@midwan
Copy link
Collaborator

midwan commented Jan 25, 2025

Check again with the latest commit please.

@giantclambake
Copy link
Author

Thanks ~ that works with .mp3 filenames now, and should work for other lower/upper case scenarios at a guess....

....it's damn 'catch-22' at the same time though....ie; if the .cue contains .MP3 and all the files are .mp3, it still won't work (no filename**.ext** match) ....so one still needs to edit the .cue file in these scenarios...

However, it is 'better' now, in as much as the user only has to edit the .cue file, so the filenames extensions match (as opposed to having to rename all the media files) ... and I think that's a catch-all fix for cases like this... edit the .cue

If like you say, nothing can be done wrt parsing the .cue file, and manually editing the .cue is the only reasonable way to work around this, then feel free to close this one ;)

Kudos

@giantclambake
Copy link
Author

Oh... if it's left like this, it should be documented somewhere ;)

@midwan
Copy link
Collaborator

midwan commented Jan 25, 2025

It should work in both scenarios:

  1. Extension is lowercase but referred to capitalized
  2. Extension is uppercase but referred to in lowercase

Did you try both of these, and did it fail in one of them?

The logic was added in the zfile code, so it will affect other places using that mechanism to open files as well.

@giantclambake
Copy link
Author

Did you try both of these, and did it fail in one of them?

Yes, Extension is lowercase but referred to capitalized failed.

midwan added a commit that referenced this issue Jan 30, 2025
…sions (#1611)

The previous implementation was not fully working, fixed it and modernized it using strings
@midwan
Copy link
Collaborator

midwan commented Jan 30, 2025

Should be fixed now, tested here with the sample you provided, and it seems that way. :)

@midwan midwan closed this as completed Jan 30, 2025
@giantclambake
Copy link
Author

Seems to work correctly now, kudos ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants