SDL_mixer
SDL_mixer copied to clipboard
libsdl-org
[SDL3] Fixed detection of FLAC files with ID3 tag at begin and extending MP3 detection
Recently I found that some FLAC files won't play at all. I had checked, and I found it begins with an ID3 tag. Also, I checked around the stuff, and it's not a paranomrmal mess of Audacity: it's one of legacy forms of FLAC files that used ID3 tags at begin.
There is a comment where I explained about this a first time: https://github.com/WohlSoft/SDL-Mixer-X/commit/94e87503c80cc0311dfedb370e21d25573492e33
Example file (I exported it in 2019'th year by the old version of Audacity): Fox fantasy.mid-battle-chess-gmized.flac.zip
Also, I had to extended the MP3 detection code to ensure that more MP3 files will be properly detected:
- Veber_-_Rio_Rita.mp3.zip - contains an unexpected junk after ID3 tag, it WILL be misdetected by the given formula.
- Also, a lot of MP3 files that starts with a sequence of NULL bytes (possibly because of shitty tag remover processing)
- Some MP3 files may begin at the middle of a frame, so, need to detect the nearest valid frame.
Like I said in original commit: The idea of FLAC files with ID3 tags?? I shall vomit...
This may be one way of handling it. (Labeling a file starting with ID3 as mp3 is flaky to start with...) Any other ways to do this?
Maybe use my own implementation of detect_mp3()? https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/music.c#L1385-L1456
I mean, call it after skipping the ID3 tag (and inside the detect_mp3() itself, I should remove the check for the presense of ID3).
I polished it for many years on thousands of various MP3 files that I had at my personal collections.
Does this sounds better?
Also: put the detect_mp3 calling into the LOWEST position of the whole list to ensure all previous checks hadn't detected any known file format, like I did at MixerX.
Like I said in original commit: The idea of FLAC files with ID3 tags?? I shall vomit...
And I wonder: how much should we care about flac files with id3? I.e.: how common are they?
CC: @ktmf01 as libflac maintainer.
At the mainstream FLAC I also do see the code that skips ID3 tags: https://github.com/xiph/flac/blob/1619af5a36fc0343cdf6b3517bb78d8aee85fe59/src/libFLAC/metadata_iterators.c#L3143-L3176
And at the changelog I see, the ID3 tags were being supported in the past, but had been removed, and now, if any FLAC file contains these tags, they just gets skipped. Basically to keep compatibility to let any modern player keep opening these old files.
And at the changelog I see, the ID3 tags were being supported in the past
We're talking almost 19 years ago here. Support from plugins was removed in October 2004, and the command line programs never supported reading or writing them, only skipping them. There are a few programs around that are able to are able to transcode ID3v1 and ID3v2 to FLAC tags.
edit: the flac command line program gained the ability to re-encode files in 2006, where it just actively removed those tags from the file. So I'd say ID3 tags have been actively unsupported since then. Of course it is not my place to say anything about this, but I would discourage adding any support for them anywhere.
My patch doesn't adds support: it adds skipping as all current FLAC implementations. Without this patch, all these FLAC files gets recognised as MP3 files which obviously gets refused as invalid by MP3 decoders.
Also, recently at MixerX I slightly tweaked my code to also process the check for MP3 frames after ID3 tag to ensure that it's really an MP3 file and not something also. I could extend this patch by these changes I did.
The problem is that FLAC files with an ID3v2 block are not valid, according to the FLAC spec. Now, the flac command line tool does have quite some handling of other 'deviations' from the spec. For example, it'll also partially read corrupted files (if asked for), skip corrupted metadata chains, read files with no metadata at all and read files of which the coded number is incorrect.
But 'salvaging' broken files is something that is expected of the flac software, because it is expected to be the most capable piece of software concerning all things FLAC. Ironically, it isn't, because ffmpeg can do a lot of things that flac can't do. Anyway, just because the flac command line tool can read broken files doesn't mean all FLAC reading software should.
I talked about the library libflac, it still able to open these files. The problem at Mixer here is a necessary to properly detect these files, and then let libflac to deal with it by its own. The Mixer doesn't implements actual decode of FLAC, it only detects the file according small fragment of the file data, and then it lets the native library (or the official libflac, or the custom header-only drflac) to deal with these files.