ZDoom

In this interest of combining bug reports: I wanted to cross-post this report I made a few months ago. The latest version of ZMusic also breaks support for .mptm files, which don't play correctly under libXMP. However, they do still play correctly with the old Dummy library. https://forum.zdoom.org/viewtopic.php?t=80249 This was also reported in another thread. https://forum.zdoom.org/viewtopic.php?t=79886

Even though these new libraries are technically upgrades, the switchover in 4.13 seems to have negatively impacted multiple audio formats. Although Graf has pointed out that it's possible to change what library is used to play specific songs in SNDINFO, this doesn't fix the problem for mods that have already been released when the default libraries still worked. I want to suggest defaulting to a previous libraries for both these formats until the new libraries fix their respective problems.

Still the same in 4.15pre303, in my case the only way to make all tracks load correctly was to convert them to Ogg Vorbis.

Yes, it does.

As a small update, it doesn't seem as though [c]MPG123_NO_FRANKENSTEIN[/c] is the cause of the failure with libsndfile, though the disparity between older and newer GZDoom versions is likely as said. Older releases have a ZMusic build that uses libmpg123 directly itself, which works (and is why it works for me with libmpg123 available), while newer releases rely on libsndfile alone which is less forgiving with poorly constructed mp3s. Though I am curious, at least for me libsndfile relies on libmpg123 as a shared lib. Does the Windows libsndfile build static link libmpg123 to avoid the extra DLL?

:laff:
Yeah, that makes sense.

I knew Chris was involved; I didn't know they were kcat.

[quote=Enjay post_id=1255830 time=1729245788 user_id=16]
Comment from the issue thread that I raised in GitHub:

[quote]Seems it might be due to mpeg_decoder_init explicitly setting the MPG123_NO_FRANKENSTEIN flag, making libmpg123 more strict with the bytestream and causing it to reject the files since they start with 0 data and junk. libmpg123 is otherwise capable of playing the files, but not through libsndfile, and that flag stands out as a possible reason.[/quote]
[/quote]

That's basically what Chris said above; which makes sense when you know that kcat on Github is Chris on here. :wink:

Comment from the issue thread that I raised in GitHub:

[quote]Seems it might be due to mpeg_decoder_init explicitly setting the MPG123_NO_FRANKENSTEIN flag, making libmpg123 more strict with the bytestream and causing it to reject the files since they start with 0 data and junk. libmpg123 is otherwise capable of playing the files, but not through libsndfile, and that flag stands out as a possible reason.[/quote]

zmusic still has direct libmpg123 support, but since libsndfile handles this case the DLL is no longer included. What does it do if you copy one from an older GZDoom into the folder?

I wonder what tool has written these files.

FWIW, it seems libsndfile isn't detecting either of the files for me either, meaning it's libmpg123 that manages to handle the files. I'd still call this an issue in libsndfile either way[s], if nothing else the ID3v2 tag in the first file should give it a confidence boost that there's supposed to be audio even if it may have to search deeper to get to it (the second file just has a lot of nothing at the start before reaching what appear to be dummy frames with text info before the audio data actually starts; libsndfile may just need to search a bit more by default to find it)[/s]. Looking at its source, it uses libmpg123 when compiled with mpeg audio support, and seems to explicitly set the [c]MPG123_NO_FRANKENSTEIN[/c] flag that makes it more strict about the mp3 bytestream. So I guess older GZDoom/ZMusic fell back to using libmpg123 directly without using that flag (or used an older libsndfile that used an older libmpg123 that didn't have that flag yet), allowing it to recognize the file, while newer versions rely on libsndfile with a newer libmpg123 with that flag, which is more strict and reject it.

Yep, no worries, and understood.
I only included the second file in case anyone was interested in seeing it, and I was busy typing my post when our posts crossed.

For what it's worth, I loaded up the files in a sound editor and saved them again. This cleared all the empty bytes and the resulting files work in GZDoom.

I have no idea if there are any projects in the wild with similar MP3s but I guess the possibility exists.
If libsndfile is failing to identify files like this, then they may be interested in looking at it.

I have raised an issue on libsndfile
https://github.com/libsndfile/libsndfile/issues/1039
I'm no expert on this, so if anyone has any corrections or anything to add, please be my guest. :)

You won't get anywhere with this here. Like I said, we rely on libsndfile for identification.

I can see all those zeroes in a Hex editor. I wonder how it ended up like that?

I don't suppose it makes much difference, but here's another sample.

https://www.aspectsweb.co.uk/enjay/TempStuff/MusNoWork2.pk3

(Same thing, load and it replaces d_runnin.)

The reason I'm uploading this one is that SLADE identifies it as an MP3 and plays it, but GZDoom does not.

Looking at it in a hex editor, it doesn't seem to have an ID3 header, but it does have lost of zeroes near the start, a few non-zero values and then a bunch more zeroes before the data starts properly. So, probably just a slightly different manifestation of the same cause.

If this happens in GZDoom you will have to report it to libsndfile. We do not attempt any MP3 detection ourselves, and I even think libsndfile just defers to libmpg123.

	// Check for empty wasted space at the beginning, since it's apparently
	// quite popular in MP3s to start with a useless blank frame.
	size_t s = 0;
	if (mc.size() > 0 && mc[0] == 0)
	{
		// Completely arbitrary limit to how long to seek for data.
		size_t limit = std::min<size_t>(1200, mc.size() / 16);
		while ((s < limit) && (mc[s] == 0))
			++s;
	}

[quote=Chris post_id=1255781 time=1729170862 user_id=61]
Looking at the file, it starts with an ID3 tag followed by a whole lot of nothing. The MP3 data doesn't start until the 3,370th byte, that makes me think perhaps the decoder gives up trying to figure out what it is before reaching the MP3 bytestream for some reason.
[/quote]

I can confirm this is the case at least for SLADE:
[code] // Check for empty wasted space at the beginning, since it's apparently
// quite popular in MP3s to start with a useless blank frame.
size_t s = 0;
if (mc.size() > 0 && mc[0] == 0)
{
// Completely arbitrary limit to how long to seek for data.
size_t limit = std::min<size_t>(1200, mc.size() / 16);
while ((s < limit) && (mc[s] == 0))
++s;
}

[/code]

Of course, in SLADE's case, there's also a performance requirements since it basically needs to identify every single lump when opening an archive. So it makes sense not to bother with trying to validate a reticent MP3 for too long.

gzdoom-x64-g4.14pre-16-g261881e0d gives me the same result as 4.13. i.e. the music does not play and there is the error message at the console.