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

Unable to play strange.uni in libmikmod 3.3.11 but was able to play it in libmikmod 3.3.10 #68

Closed
Baguettedood opened this issue Sep 22, 2022 · 7 comments

Comments

@Baguettedood
Copy link

strange.uni is a music track included in Onlink (the mod to the hacking game Uplink, but the track doesn't seem to be included in the original game or its bonus CD). I've attached a zip of the specific file, which is hopefully permitted. If not then it could be extracted from Onlink's data.dat yourself.
strange.zip

This track used to play on earlier versions of libmikmod but broke in later versions. The strange_loader.mod available on modarchive is still playable in newer versions of libmikmod.

The breaking commit appears to be 3a8d364 and the track worked in the previous commit 1c8dfd0

@sezero
Copy link
Owner

sezero commented Sep 22, 2022

Well, the file hits end-of-file during in the middle of sample reading
in SL_LoadInternal() at
https://github.com/sezero/mikmod/blob/master/libmikmod/playercode/sloader.c#L297

I don't know whether I can do anything about it. @AliceLR: Can you think
of any better solution than the existing one?

@sezero
Copy link
Owner

sezero commented Sep 22, 2022

FWIW, here are the debug lines I added:

diff --git a/libmikmod/playercode/sloader.c b/libmikmod/playercode/sloader.c
index 4c52ab0..e135403 100644
--- a/libmikmod/playercode/sloader.c
+++ b/libmikmod/playercode/sloader.c
@@ -234,7 +234,7 @@ static int SL_LoadInternal(void *buffer,UWORD infmt,UWORD outfmt,int scalefactor
 	SBYTE compressionTable[16];
 	SWORD adpcmDelta = 0;
 	BOOL hasTable = 0;
-
+fprintf(stderr, "SL_LoadInternal: length==%lu (at %ld)\n",length,reader->Tell(reader));
 	status.buf = 0;
 	status.last = 0;
 	status.bufbits = 0;
@@ -282,7 +282,7 @@ static int SL_LoadInternal(void *buffer,UWORD infmt,UWORD outfmt,int scalefactor
 			}
 		} else {
 			if(infmt&SF_16BITS) {
-				if(_mm_eof(reader)) {
+				if(_mm_eof(reader)) {fprintf(stderr, "SL_LoadInternal(%d): EOF, length==%lu at %ld\n",__LINE__,length,reader->Tell(reader));
 					_mm_errno=MMERR_NOT_A_STREAM;/* better error? */
 					return 1;
 				}
@@ -294,7 +294,7 @@ static int SL_LoadInternal(void *buffer,UWORD infmt,UWORD outfmt,int scalefactor
 				SBYTE *src;
 				SWORD *dest;
 
-				if(_mm_eof(reader)) {
+				if(_mm_eof(reader)) {fprintf(stderr, "SL_LoadInternal(%d): EOF, length==%lu at %ld\n",__LINE__,length,reader->Tell(reader));
 					_mm_errno=MMERR_NOT_A_STREAM;/* better error? */
 					return 1;
 				}

... and it reports:

SL_LoadInternal: length==2476 (at 13001)
SL_LoadInternal: length==1060 (at 15477)
SL_LoadInternal: length==2434 (at 16537)
SL_LoadInternal: length==5346 (at 18971)
SL_LoadInternal: length==3470 (at 24317)
SL_LoadInternal: length==4024 (at 27787)
SL_LoadInternal: length==2764 (at 31811)
SL_LoadInternal: length==800 (at 34575)
SL_LoadInternal: length==8906 (at 35375)
SL_LoadInternal(297): EOF, length==714 at 43375

(Worthy of note is that commit 3a8d364 failed to add the same EOF checks
to the SF_ADPCM4 case at line 267.)

@sezero
Copy link
Owner

sezero commented Sep 27, 2022

@AliceLR: Do you have any possible solutions? Or should I close this as wontfix?

@Baguettedood
Copy link
Author

Ultimately it seems like the file only played because two bugs cancelled each other out 😅

The Onlink devs are fixing the file in the next version so I guess it won't really be a problem for much longer though.

Thanks for looking into it.

@sezero
Copy link
Owner

sezero commented Sep 27, 2022

The Onlink devs are fixing the file in the next version so I guess it won't really be a problem for much longer though.

OK then, closing.

Thanks for looking into it.

You're welcome!

@sezero sezero closed this as completed Sep 27, 2022
@AliceLR
Copy link
Collaborator

AliceLR commented Sep 27, 2022

Sorry I missed this. IMO failed sample loads for these very old formats shouldn't necessarily cause MikMod to reject the file, but I'm not familiar with Unimod. Failed sample loads are one of the most recoverable issues (just disable the sample).

@sezero
Copy link
Owner

sezero commented Sep 27, 2022

Hearing strange tones from broken files would be preferred?
Well, that's a choice too.

I might mess with it, but not a high priority.. Do you have a patch?

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

3 participants