We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4295

Summary: [PATCH] music_mad.c must handle and skip tags
Product: SDL_mixer Reporter: Ozkan Sezer <sezeroz>
Component: miscAssignee: Ozkan Sezer <sezeroz>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: unspecified   
Hardware: All   
OS: All   
Attachments: patch to make libmad skip tags
updated patch for SDL-1.2
updated patch for SDL-2.0
patch for SDL_mixer-2.0
patch for SDL_mixer-1.2

Description Ozkan Sezer 2018-10-05 16:20:29 UTC 
Created attachment 3340 [details]
patch to make libmad skip tags

This file (https://github.com/WohlSoft/AudioCodecs/files/2448712/bad-mp3.zip)
plays back wrong with SDL_mixer if the library is built against libmad.  The
reason is that there is no tag handling and skipping in music_mad.c; see the
discussion led to this bugzilla entry here:
https://github.com/WohlSoft/AudioCodecs/issues/6#issuecomment-426941738

The lack of tag handling / skipping can even lead to crashes, as reported by
Vitaly Novichkov.

I cooked the attached patch, and it plays the mp3 referred to above normally.
The patch based mainly on sox, which in turn uses bits from libid3tag.  The
apetag parsing is based on a patch that was made for mpg123.

Can you see anything bad?  OK to apply?
Comment 1 Ozkan Sezer 2018-10-05 17:32:10 UTC 
The SDL-1.2 branch is worse that it certainly crashes:  valgrind
reports several invalid reads and invalid writes.  The patch fixes
the issue there too.
Comment 2 Ozkan Sezer 2018-10-05 18:16:33 UTC 
Created attachment 3343 [details]
updated patch for SDL-1.2
Comment 3 Ozkan Sezer 2018-10-05 18:17:09 UTC 
Created attachment 3344 [details]
updated patch for SDL-2.0
Comment 4 Sam Lantinga 2018-10-06 00:01:13 UTC 
We can't include GPL code unless MUSIC_MP3_MAD_GPL_DITHERING is defined, as package releases do not enable GPL code by default.

Do you want to do a clean implementation under the Zlib license, or add alternative code paths for non-GPL code?
Comment 5 Ozkan Sezer 2018-10-06 14:11:43 UTC 
Created attachment 3349 [details]
patch for SDL_mixer-2.0

The code was already half-way rephrased, but went back again, further
used id3.org docs and made the attached version. I don't think it can
be made more from scratch than this.  OK to apply?
Comment 6 Ozkan Sezer 2018-10-06 14:12:09 UTC 
Created attachment 3350 [details]
patch for SDL_mixer-1.2
Comment 7 Sam Lantinga 2018-10-07 00:58:20 UTC 
Yep, looks good, thanks!
Comment 8 Ozkan Sezer 2018-10-07 08:50:21 UTC 
SDL-1.2 branch: https://hg.libsdl.org/SDL_mixer/rev/ae9b46ccd5ab
default branch: https://hg.libsdl.org/SDL_mixer/rev/c39a11fa853e

Closing as fixed.