Merge SDL_mixer with SDL Mixer X (my extended fork of SDL_mixer)

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• 1139.patch (1139.patch, text/plain, 2019-12-21 12:19:59 +0000, 716 bytes)
• 1141.patch (1141.patch, text/plain, 2019-12-21 12:20:34 +0000, 13908 bytes)
• 1142.patch (1142.patch, text/plain, 2019-12-21 12:21:01 +0000, 10728 bytes)
• 1144.patch (1144.patch, text/plain, 2019-12-21 12:21:36 +0000, 10793 bytes)
• 1145.patch (1145.patch, text/plain, 2019-12-21 12:22:08 +0000, 25782 bytes)
• 1147.patch (1147.patch, text/plain, 2019-12-21 12:22:41 +0000, 793 bytes)
• 1152.patch (1152.patch, text/plain, 2019-12-21 12:23:08 +0000, 2634 bytes)
• 1142.patch (cleaned-up) (1142.patch, text/plain, 2019-12-21 17:10:33 +0000, 10530 bytes)
• 1145a.patch (clean-ups for 1145.patch) (1145a.patch, text/plain, 2019-12-21 17:13:22 +0000, 3866 bytes)
• An example MP3 file with the sequence of zeroes at begin (moi_drug_luchshe_vseh_igraet_blu.mp3.7z, application/octet-stream, 2021-02-05 19:13:50 +0000, 9878829 bytes)

Reported in version: unspecified
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2017-10-21 20:07:04 +0000, Vitaly Novichkov wrote:

Hello!
I'm a creator of my own fork of SDL Mixer library I began by many purposes and a wish to extend it:

https://github.com/WohlSoft/SDL-Mixer-X

What I implemented into this fork:

• Added new codecs: libGME (Game music Emulators), libADLMIDI (Emulated FM synth MIDI player based on OPL3 chip), libOPNMIDI (another emulated FM synth MIDI player based on OPN2 chip). (The reason I added libADLMIDI and libOPNMIDI - there are requires no extra banks and there are able to play MIDI everywhere, just everything is generated from scratch by formulas. Also, I'm a fan of FM synthesis and I like to use FM chips to create/listen music :-) ).
• Added ability to toggle MIDI sequencer in real time (unlike of internal auto-choosing, you can manually choose which MIDI sequencer you wish to use).
• Added ability to read meta-tags from some files (song title, author, copyright, album title)
• Because was "seek" the only command, I have added "tell" to retrieve a current position of a song (I have used that in my PGE MusPlay tool) and I also added support to retrieve loop tags positions too (which are in OGG file).
• Added a workaround - simple internal resampler while in that time SDL had it very bad and glitchy.
• libMAD playing code where I fixed various bugs and I added usage of the libid3tag to recognize start position of the song data (some MP3 with ID3 tags are causing CRASH of library when libMAD tries to play it, and it plays, but result is glitchy until crash will appear)
• Wavestream renamed into music_wave, where I changed strategy to make it simpler and less glitchy. I added the ability to support multiple streams with the same codec. I want to make "Multi-Music" API that will play multiple music streams by the same strategy as music.c (for example, very long atmosphere SFX or dynamical music tracks where you can turn on/off some musical instruments, I heard that Half-Life had that). (This also required to add support of cross-fade)
• I have added ability to redefine timidity config patch to get ability have timidity patches everywhere
• Extra arguments are can be given together with file path (to choose ADLMIDI bank or choose track in the multi-track NSF/HES file)
• Rewritten music.c to have friendly interface instead of heavy and scary switch-cases. (I did that before you recently did the same).
• Reorganized folders and files (instead of a flood of the root folder, I made a beautiful structure where I put codecs modules independently from common source code, header into "include" and all internal files into "src").
• Nuked Autotools and used QMake and CMake

I want to synchronize our works to support same. For now, I graduated applying your latest changes to my fork to support same new things as you are introduced recently.

P.S. My code style in most is "Allman", therefore our codes are having differences (I did use "Artistic Style" beautifier to beautify some of the source files) and I'm a fan of spaces rather tabs. So, If you wish to keep your style as you like, can you describe style you following strictly (brackets, intending, spaces/tabs/ etc.) and which are doesn't matter and I'm free to use that style I using myself?

P.P.S. Myself I using QMake and CMake to build stuff and I did remove Autotools from me as hard to maintain it. Also, I made usage of Qmake and CMake for each dependency (except SDL2 which already has CMake). Therefore I would need your help to keep the correct support of Autotools without breaking of them.

On 2018-01-21 00:06:45 +0000, Vitaly Novichkov wrote:

Hello!

Recently I have completed my merge job of your changes with mine and I have extended functionality by adding of my stuff.

Especially for your convenience I made the Mercurial repository on Bitbucket you can easily review my changes and pull and merge if that needed:
https://bitbucket.org/Wohlstand/sdl_mixer/commits/all

Here is written a full changelog of stuff I made:
https://bitbucket.org/Wohlstand/sdl_mixer/commits/524433869ae11d0fb56a1464dfb75b498911e39f

My next part of the job is completing the "What NOT added from SDL Mixer X yet" list:

• Add all those libraries into "external" folder and make them be buildable through Automake and through VisualStudio projects. Then I can freely add the GME, ADLMIDI, and OPNMIDI libraries.
• Implement complete CMake build support that does not exist yet in SDL Mixer

On 2019-11-17 17:36:22 +0000, Vitaly Novichkov wrote:

Note that bitbucket repository is now very outdated, and soon, Bitbucket will nuke all Mercurial repositories due to stopping to support Mercurial VCS at all.

On 2019-11-17 18:55:33 +0000, Sam Lantinga wrote:

In general, these look awesome! I'd like to finish merging your changes, and promote this to SDL_mixer 2.0.

Do you have a clean set of changes that apply to the current SDL_mixer on hg.libsdl.org?

On 2019-11-17 19:10:56 +0000, Vitaly Novichkov wrote:

Do you have a clean set of changes that apply to the current SDL_mixer on hg.libsdl.org?

I had, however, they got to be very outdated. So, I'll rebuild them from scratch again. Looks like I need to begin a new fork of the repository from the scratch.

With the set of changes are:

• relocating files into nice "include", "src", "src/codecs". This will break all hand-made VS and XCode projects which will require to apply these locations to them. This is optional to make the look of repository better.
• adding some new files and modules that will require modifying of VS and XCode projects to attach them.
• adding some new libraries to link, which will also require modifying of VS and XCode projects.
• CMake build is going to be default one. On my side, I should improve the look of dependencies management as now it looks not good (repeating parts of code to check the existence of certain libraries and using pre-build from AudioCodecs set by optional choice).

However, there are changes that can be applied without adding new files or libraries which can be put over current stuff now.

Alternatively, I'll give the backport of most of my changes include new files and libraries, and can you begin a new branch to work on them separately?

On 2019-11-17 19:12:49 +0000, Vitaly Novichkov wrote:

to work on them separately?

I meant fixing of VS and XCode projects are will be broken after adding a set of new dependencies and files. And, posssibly, because of source files relocations (which is optionally and can be skipped to leaving files in their current places).

On 2019-11-17 19:16:48 +0000, Vitaly Novichkov wrote:

So, my question is: do you prefer to keep current locations of source files or I can freely relocate them into "src", "src/codec", "include"?

On 2019-11-17 19:32:59 +0000, Ozkan Sezer wrote:

Speaking for myself,

(In reply to Vitaly Novichkov from comment # 4)

• CMake build is going to be default one.

If autotools, VisualC and Xcode projects aren't touched/broken
I have no problem with that.

(In reply to Vitaly Novichkov from comment # 6)

So, my question is: do you prefer to keep current locations of source files
or I can freely relocate them into "src", "src/codec", "include"?

I'd rather not change the tree layout for the time being.

On 2019-11-17 20:01:51 +0000, Vitaly Novichkov wrote:

If autotools, VisualC and Xcode projects aren't touched/broken
I have no problem with that.

Okay, however, for some files I'll try to add new files into these projects also. Regarding new dependencies, these projects will still be buildable, however, these new dependencies and codecs are based on them, will not work until support will be implemented. There are GME, libXMP, libADLMIDI, libOPNMIDI, and libID3TAG (for MP3 tags support).

I'd rather not change the tree layout for the time being.

Ok, will keep the tree layout the same as the original while backporting mine stuff.

On 2019-11-18 01:38:48 +0000, Sam Lantinga wrote:

Actually, with a change of this magnitude, moving files around is fine, we'll rebuild the hand-crafted projects, no problem.

One thing to be aware of is that all third party dependencies must be optional, and dynamically loadable. Any code that is loaded by default must be compatible with the zlib license. If there is any LPGL or similar code, it must default to off.

On 2019-11-18 08:21:27 +0000, Vitaly Novichkov wrote:

Actually, with a change of this magnitude, moving files around is fine, we'll rebuild the hand-crafted projects, no problem.

Ok, then will restructure a tree for nicer look.

One thing to be aware of is that all third party dependencies must be optional, and dynamically loadable. Any code that is loaded by default must be compatible with the zlib license. If there is any LPGL or similar code, it must default to off.

All new dependencies I have introduced are optional, therefore yeah, I will keep the note to make them be disabled by default. I'll also implement dynamic loading of libADLMIDI, libOPNMIDI and libGME are didn't make yet. libXMP has two variants of build: MIT and LGPL, however, I'll implement the dynamic loader for it too.

BTW, I looking for an alternative to modified libID3TAG made by a team who made MAD codec (I modded it to equip it with SDL_RWops support) library which is GPL, so, I will give the dummy only, and it needs to:

• keep as-is and don't parse ID3 tags at all
• make some minor ID3 tag parsing

On 2019-11-19 06:23:24 +0000, Sam Lantinga wrote:

Ozkan, can you hold off further SDL_mixer cleanup while Vitaly prepares his merge, and then verify it once he's created his merge patchset?

On 2019-11-19 06:34:16 +0000, Ozkan Sezer wrote:

(In reply to Sam Lantinga from comment # 11)

Ozkan, can you hold off further SDL_mixer cleanup while Vitaly prepares his
merge, and then verify it once he's created his merge patchset?

Of course. (The cleanups I did yesterday was after the merge for
bug # 4865, anyway.)

I hope the merge for this one happens with multiple / broken-up
which do one thing at a time.

On 2019-11-19 06:50:43 +0000, Vitaly Novichkov wrote:

Ozkan, while I working on a patchset, I would grant you a write access to my repos where you would be able to make various fixes in parallel with my works.
Ooay, my workflow is:

• polish CMake build and make dynamical APIs work again (they are never used right now, and I should make them working by cmake build rework)
• make dynamical loading of libADLMIDI, libOPNMIDI, GME and libXMP
• replace GPL-licensed libID3TAG with a ZLib compatible alternative and mimimalistic implementation
  And then:
• make a new branch on SDL_mixer repo (at my bitbucket)
• put my code into it
• update autotools build to make it build my update
• polish some other cases
  And when works will be completed, feel free to pull that branch and merge.

On 2019-11-19 09:41:51 +0000, Vitaly Novichkov wrote:

Okay, I think, to simplify future works, I made on a quick hand restructure of the tree and I have put it here: https://bitbucket.org/Wohlstand/sdl_mixer/commits/608bacaa2d582870df9824c14fa7fc50772adfd5
(feel free to pull https://Wohlstand@bitbucket.org/Wohlstand/sdl_mixer and verify the stuff)
I have ported and tested build and install of Autotools build which now working with a new tree, CMake and Android also ported but didn't test yet. Android builds need a fix of include directories I guess (You should add include, src, and src/codecs as global include paths).

On 2019-11-20 15:44:01 +0000, Ozkan Sezer wrote:

I merged the tree layout changes of Vitaly Novichkov
to help with his preparing his changesets.

Build status: Autotools, Xcode, and VicualC works for me.
Xcode-iOS and VisualC-WinRT projects are updated but NOT
tested at all: please test/update them as needed (I can
not do that myself.)

On 2019-12-17 08:03:51 +0000, Ozkan Sezer wrote:

Vitaly: Can you give an ETA about your patch-sets for this?

On 2019-12-21 12:19:28 +0000, Ozkan Sezer wrote:

Here are the first set of patches from Vitaly, currently residing at:
https://bitbucket.org/Wohlstand/sdl_mixer/branch/music-interfaces-ex2

Comments, concerns, accepts, rejects, changes requests, etc. wanted:

• Are there any of them we do not want in mainstream?
• Do any of them require changing in some way before accept?

If no comments until Mon. Dec. 23, 14:00 (GMT+03), I'll most probably
apply them mainstream SDL_mixer.

On 2019-12-21 12:19:59 +0000, Ozkan Sezer wrote:

Created attachment 4123
1139.patch

music_mad.c (MAD_Seek): Avoid a junk chunk be played after seek
https://bitbucket.org/Wohlstand/sdl_mixer/commits/abc0101a09bb2aaa6a390b81e19f7368719d68b6?at=music-interfaces-ex2

On 2019-12-21 12:20:34 +0000, Ozkan Sezer wrote:

Created attachment 4124
1141.patch

Add Mix_GetVolume() call to retrieve the volume of a music
https://bitbucket.org/Wohlstand/sdl_mixer/commits/21eaf90a7bd22420618205ae690f81b87b876afb?at=music-interfaces-ex2

The 'Stream' suffix is there to prepare for multi-music functionality
which includes a set of new functions are equal to old Music functions
but unlike them, requiring a music pointer.

multi-music: it's an api that should allow to play multiple parallel
streams (for example, various environment SFX that is hard to play via
existing chunks api due the fact they are loading an entire song into
memory). Also the Multi-Music API should allow a cross-fade of songs
and should allow making a dynamical songs - i.e. playing separated
tracks of the same song as different streams, and controlling a volume
of every stream to mute/unmite certain stream. All multi-music related
functions will have 'Stream' suffix.

multi-music isn't ready yet on MixerX side, however, I prepared some
works for it.

On 2019-12-21 12:21:01 +0000, Ozkan Sezer wrote:

Created attachment 4125
1142.patch

Add Mix_GetMusicPosition() call to retrieve a current music playing position
https://bitbucket.org/Wohlstand/sdl_mixer/commits/c639c940542cffb0a629b3d4910b0358bebe8bcb?at=music-interfaces-ex2

On 2019-12-21 12:21:36 +0000, Ozkan Sezer wrote:

Created attachment 4126
1144.patch

Add Loop points information calls
https://bitbucket.org/Wohlstand/sdl_mixer/commits/ee1a67f25eae199a0b2790548d4f1d1e57e85f5f?at=music-interfaces-ex2

• Mix_GetMusicLoopStartTime() to retrieve loop start time position
• Mix_GetMusicLoopEndTime() to retrieve loop end time position
• Mix_GetMusicLoopLengthTime() to retrieve a length of looping area

On 2019-12-21 12:22:08 +0000, Ozkan Sezer wrote:

Created attachment 4127
1145.patch

Introduce MetaTags api
https://bitbucket.org/Wohlstand/sdl_mixer/commits/ae9a63148ec2950b22dabf501c9f1236e31331b2?at=music-interfaces-ex2

In some games or players it's would be possible to print a title and
some minor tags about playing music.

There are 5 added functions which can give meta-tags from music if they
are available or supported by a codec:

• Mix_GetMusicTitle() gives a song title.
  Unlike Mix_GetMusicTitleTag, returns a filename if tag is blank.
• Mix_GetMusicTitleTag() gives a song title
• Mix_GetMusicArtistTag() gives an artist name
• Mix_GetMusicAlbumTag() gives an album name
• Mix_GetMusicCopyrightTag() gives a copyright tag

On 2019-12-21 12:22:34 +0000, Vitaly Novichkov wrote:

P.S. A note about Multi-Music: WohlSoft/SDL-Mixer-X#2

On 2019-12-21 12:22:41 +0000, Ozkan Sezer wrote:

Created attachment 4128
1147.patch

Change default sample rate to 44100
https://bitbucket.org/Wohlstand/sdl_mixer/commits/47ce1bb96fefb20fc94dfc90e30967f9daea4a55?at=music-interfaces-ex2

22050 for a very long time ago, is no more widely used. The 44100 is the most
popular for ten and more years. I don't think here is any reason to keep this
macro to have 22050 sample rate.

On 2019-12-21 12:23:08 +0000, Ozkan Sezer wrote:

Created attachment 4129
1152.patch

playmus.c: Add more information printing
https://bitbucket.org/Wohlstand/sdl_mixer/commits/8d269d3ce5b093060e9f335fab4784e16c99aac9?at=music-interfaces-ex2

• Meta-tags if available
• Loop points if available
• Current position streaming if possible

This one obviously depends on functionality added by previous patches.

On 2019-12-21 12:28:26 +0000, Vitaly Novichkov wrote:

music_mad.c (MAD_Seek): Avoid a junk chunk be played after seek

This is a small bugfix that would go out of this workflow

On 2019-12-21 12:32:25 +0000, Vitaly Novichkov wrote:

BTW, I looking for an alternative to modified libID3TAG made by a team who made > MAD codec (I modded it to equip it with SDL_RWops support) library which is
GPL, so, I will give the dummy only, and it needs to:

• keep as-is and don't parse ID3 tags at all
• make some minor ID3 tag parsing

Also, instead of this library, I built my own tag parsing, so, this dependency is no more dependency.

On 2019-12-21 17:10:33 +0000, Ozkan Sezer wrote:

Created attachment 4132
1142.patch (cleaned-up)

(In reply to Ozkan Sezer from comment # 20)

Created attachment 4125 [details]
1142.patch

Add Mix_GetMusicPosition() call to retrieve a current music playing position
https://bitbucket.org/Wohlstand/sdl_mixer/commits/
c639c940542cffb0a629b3d4910b0358bebe8bcb?at=music-interfaces-ex2

Cleaned-up version of the patch, which removes an unnecessary NULL
check from FLAC_Tell().

On 2019-12-21 17:13:22 +0000, Ozkan Sezer wrote:

Created attachment 4133
1145a.patch (clean-ups for 1145.patch)

(In reply to Ozkan Sezer from comment # 22)

Created attachment 4127 [details]
1145.patch

Introduce MetaTags api
https://bitbucket.org/Wohlstand/sdl_mixer/commits/
ae9a63148ec2950b22dabf501c9f1236e31331b2?at=music-interfaces-ex2

In some games or players it's would be possible to print a title and
some minor tags about playing music.

There are 5 added functions which can give meta-tags from music if they
are available or supported by a codec:

• Mix_GetMusicTitle() gives a song title.
  Unlike Mix_GetMusicTitleTag, returns a filename if tag is blank.
• Mix_GetMusicTitleTag() gives a song title
• Mix_GetMusicArtistTag() gives an artist name
• Mix_GetMusicAlbumTag() gives an album name
• Mix_GetMusicCopyrightTag() gives a copyright tag

Cleanup after MetaTags patch: (my review)

• music_flac.c (flac_metadata_music_cb): remove unnecessary style change
• music.c: fix typo utiltiy -> utility
• music.c (meta_tags_set): remove unnecessary memset(0)
• music.c (struct _Mix_Music): rename music_filename to filename
• music.c: add get_last_dirsep() inline to OS-specifically get the
  last directory separator.
• music.c (Mix_LoadMUS): use get_last_dirsep() fon music->filename

If OK, and if the original patch is OK'ed,
can apply the two combined.

On 2019-12-21 20:35:43 +0000, Vitaly Novichkov wrote:

• music.c (meta_tags_set): remove unnecessary memset(0)

That memset(0) I did by historical reasons where I had a non-terminated strings caused a leading junk. memset(0) should guarantee the termination will always be. Probably, I had a deal with some buggy implementations of strcpy() in a past. Should be fine if SDL_strlcpy will guarantee a valid null termination.

On 2019-12-21 20:52:07 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 30)

[...] Should be fine if SDL_strlcpy will guarantee a valid
null termination.

Yes it does.

On 2019-12-22 06:14:00 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 26)

music_mad.c (MAD_Seek): Avoid a junk chunk be played after seek

This is a small bugfix that would go out of this workflow

Why precisely is this needed?

On 2019-12-22 07:39:56 +0000, Vitaly Novichkov wrote:

(In reply to Ozkan Sezer from comment # 32)

(In reply to Vitaly Novichkov from comment # 26)

music_mad.c (MAD_Seek): Avoid a junk chunk be played after seek

This is a small bugfix that would go out of this workflow

Why precisely is this needed?

If you'll try to seek, if you'll don't clear the buffer, the garbage left after previously played frame will be played afger seek until play a frame on a target position.

On 2019-12-22 12:45:11 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 33)

(In reply to Ozkan Sezer from comment # 32)

(In reply to Vitaly Novichkov from comment # 26)

music_mad.c (MAD_Seek): Avoid a junk chunk be played after seek

This is a small bugfix that would go out of this workflow

Why precisely is this needed?

If you'll try to seek, if you'll don't clear the buffer, the garbage left
after previously played frame will be played afger seek until play a frame
on a target position.

I really don't see how, what am I missing? (Maybe bad day for me.)

After seek-to-start we start doing read_next_frame(). Whether you
clear the buffer or not, it writes to input_buffer from the start.
How is this helping with what?

On 2019-12-22 13:23:53 +0000, Vitaly Novichkov wrote:

(In reply to Ozkan Sezer from comment # 34)

(In reply to Vitaly Novichkov from comment # 33)
I really don't see how, what am I missing? (Maybe bad day for me.)

After seek-to-start we start doing read_next_frame(). Whether you
clear the buffer or not, it writes to input_buffer from the start.
How is this helping with what?

I made the demo:

https://www.youtube.com/watch?v=nCut01tnNw4

• seek causes previous frame be played, it's hearable when seeking into begin of the song
• I uncommenting my fixing line and seek doesn't cause the previous fragment of a frame to be played anymore.

On 2019-12-22 14:26:11 +0000, Ozkan Sezer wrote:

(In reply to Ozkan Sezer from comment # 18)

Created attachment 4123 [details]
1139.patch

music_mad.c (MAD_Seek): Avoid a junk chunk be played after seek

Applied this one after minor comment editing:
https://hg.libsdl.org/SDL_mixer/rev/760a8cb05f97

The issue itself can be reproduced by applying the following
to playmus.c, running "./playmus -i some.mp3" and hitting 0,
1, 2, 3 or 4 (followed by Enter) during playback.

diff --git a/playmus.c b/playmus.c
--- a/playmus.c
+++ b/playmus.c
@@ -75,6 +75,11 @@ void Menu(void)
fflush(stdin);
if (scanf("%s",buf) == 1) {
switch(buf[0]){

•    case '0': Mix_SetMusicPosition(0);break;
  

•    case '1': Mix_SetMusicPosition(10);break;
  

•    case '2': Mix_SetMusicPosition(20);break;
  

•    case '3': Mix_SetMusicPosition(30);break;
  

•    case '4': Mix_SetMusicPosition(40);break;
     case 'p': case 'P':
         Mix_PauseMusic();
         break;
  

On 2019-12-23 11:15:20 +0000, Ozkan Sezer wrote:

Got no reaction to any of the patches, so I applied the
first patchset:

https://hg.libsdl.org/SDL_mixer/rev/9835d67a27f9
https://hg.libsdl.org/SDL_mixer/rev/70412138c859
https://hg.libsdl.org/SDL_mixer/rev/d711a86866aa
https://hg.libsdl.org/SDL_mixer/rev/24ca9a03d51c
https://hg.libsdl.org/SDL_mixer/rev/57a746548d9e
https://hg.libsdl.org/SDL_mixer/rev/71510d620652
https://hg.libsdl.org/SDL_mixer/rev/6bf0bc6d510c

Keeping this open for future patchsets.

On 2019-12-23 15:38:45 +0000, Vitaly Novichkov wrote:

Great!
Approximately at this evening, I'll try to backport two next things:

• Ability to initialize and use the mixer without it taking over the SDL audio callback, made by krcroft Add ability to initialize and use the mixer without it taking over the SDL audio callback WohlSoft/SDL-Mixer-X#20
• MIDI switch API (you'll be able to switch MIDI library between songs, that is also used at me to use a different MIDI library with different songs to define the sounding).
• Extra useful music and SFX playing calls
• Take my version of mp3utils.c to allow MP3 and WAV/AIFF-side ID3v2 be parsed

@ozkan Sezer, can you check out my mp3utils.c and fix anything you'll find? A bit later I'll equip it with a length value I should take from ID3v2 tag and give the one another alternative way to recognize length of MP3s with using of ID3v2-stored data.

On 2021-02-05 12:16:44 +0000, Ozkan Sezer wrote:

(In reply to Ozkan Sezer from comment # 37)

Got no reaction to any of the patches, so I applied the
first patchset:

https://hg.libsdl.org/SDL_mixer/rev/9835d67a27f9
https://hg.libsdl.org/SDL_mixer/rev/70412138c859
https://hg.libsdl.org/SDL_mixer/rev/d711a86866aa
https://hg.libsdl.org/SDL_mixer/rev/24ca9a03d51c
https://hg.libsdl.org/SDL_mixer/rev/57a746548d9e
https://hg.libsdl.org/SDL_mixer/rev/71510d620652
https://hg.libsdl.org/SDL_mixer/rev/6bf0bc6d510c

Keeping this open for future patchsets.

After my recent addition of libxmp support, I want to wrap up this bug,
and then close it. As far as I can see, the rest of the MixerX changes
are too specialized and need not go into mainstream SDL_mixer -- so the
feature-set for this bug entry is done, as far as I am concerned.

My comments about current state:

• I am not overly happy with the metatags patchset, i.e.:
  https://hg.libsdl.org/SDL_mixer/rev/24ca9a03d51c
  The mp3 side of it is absent, and home-brewing a mp3 tag parser seems
  time consuming and error-prone.
  My suggestion would be removing the metatags patchset.

Sam, Ryan: What are your opinions? Anything to add? Anything to remove?

On 2021-02-05 13:05:34 +0000, Vitaly Novichkov wrote:

(In reply to Ozkan Sezer from comment # 39)
Thanks for bringing this up, I gonna make my own comments on this:

the rest of the MixerX changes are too specialized and need not go into mainstream SDL_mixer

It's about changes that were added for libADLMIDI, libOPNMIDI, and libGME libraries I had to add for the purposes of my projects, especially the system of path arguments and track selection specific calls (mainly needed for the libGME library). Also, changes with the option of adding the __stdcall call conversion to build the VB6 binding library I made by request of some Chinese folks who developed games on this and using the VB6. Also, my recent alternative NativeMIDI player allows the seek-ability/loop tags/tempo changes/etc, and also, has the MSGS volume workaround that avoids the Windows-side bug. does use the C++ code piece that I don't backport into the mainstream until I'll recode it into the pure-C.

I can try to list changes that can be freely ported into the mainstream (at the bottom of this post)

• I am not overly happy with the metatags patchset, i.e.:
  https://hg.libsdl.org/SDL_mixer/rev/24ca9a03d51c
  The mp3 side of it is absent, and home-brewing a mp3 tag parser seems
  time consuming and error-prone.
  My suggestion would be removing the metatags patchset.

Metatags API is still useful for OGG/Opus/FLAG/Trackers/WAV music formats. If you want to avoid the dangerous MP3 tag parsing code, just don't support meta-tags for MP3 formats at the mainstream. At least, the MP3 format is a pain because of its ineffective nature and the complicated case of tags for this format.

What features of MixerX can fit into the mainstream (may be incomplete):

• Run-time side MIDI change API: you can choose the library to play MIDI on runtime, and the next opened MIDI file will use the selected MIDI library. There are Timidity, FluidSynth, and Native MIDI. On the MixerX side, there are also libADLMIDI and libOPNMIDI. If you don't want to support these libraries, just don't support them, make the only MIDI select API.
• Ability to initialize the Mixer library without of SDL_Audio taking, I.e. Allow the Mixer to re-use an existing SDL_Audio initialized separately by the side, see the pull-request and the discussion Add ability to initialize and use the mixer without it taking over the SDL audio callback WohlSoft/SDL-Mixer-X#20
• Tempo change which is useful for the XMP right now, at MixerX it's also useful for libADLMIDI, libOPNMIDI, and libGME libraries. I keep some plans to add some PCM algorithms to make PCM-based streams also can change their tempo and tone. (Just making them play faster/slower is easy at least, but not good as people told me in the discussion).
• My recent clean-up where I moved all Vorbis/FLAC/Opus loop tags into the utils.c (https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/utils.c) (this file also contains other utility calls you don't need)
• Support the linking with both FluidSynth 2 and 1, the time is going and there is FluidSynth 2 getting widely. Right now at me on Linux Mint 20.1 the FluidSynth 2.1.1 is used. The difference is the return type of the delete_fluid_player() and the delete_fluid_synth() calls: https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/codecs/music_fluidsynth.c#L34-L38. Maybe there would be a better solution that allows dynamic use of each FluidSynth 1 and FluidSynth 2 by making some of the calls be optional and be a replacement to each other.
• At the libMAD I made the support for the Tell and the Duration which is still not backported into the mainstream.
• More accurate MP3 detector for the magic-number scanner (see https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/music.c#L736-L816)
• Stopping to trust the filename extension to recognize the file type (I saw several problems because some "so smart" users had to try "convert" the format by just renaming. That caused the file won't play because of an error. The file itself is valid but has the lying filename extension. I avoided this by making the format detection by the magic number only except the set of formats that is hard to recognize by the content).
• Ability to play chunks with the given volume value (calls such as "int Mix_PlayChannelTimedVolume(int which, Mix_Chunk *chunk, int loops, int ticks, int volume)"). I added this API because it's a pain when playing voices with different volumes and using the -1-random free channel that doesn't set/reset the volume, which causing the unexpected chunk volume change. I had to add those calls without ABI breaking, so, the existing Mix_FadeInChannelTimed() is a function, not a macro, but inside it calls the new Mix_FadeInChannelTimedVolume() with an ignorance of the initial volume set.

I gonna resume my work on making the mainstream patches as soon as possible. I had to pause my work because of business with the more important tasks at that moment.

On 2021-02-05 13:10:44 +0000, Vitaly Novichkov wrote:

The small note about the MP3 detector I made: I have several files that won't be detected by the current MP3 detector code at the mainstream Mixer, especially the case when file had an ID3 tag that was filled by zeroes, and therefore, the result is always failed.

On 2021-02-05 14:49:28 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 40)

• More accurate MP3 detector for the magic-number scanner (see
  https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/music.c#L736-L816)

(In reply to Vitaly Novichkov from comment # 41)

The small note about the MP3 detector I made: I have several files that
won't be detected by the current MP3 detector code at the mainstream Mixer,
especially the case when file had an ID3 tag that was filled by zeroes, and
therefore, the result is always failed.

If ID3 is the only problem with detection:

• Is the ID3 tag broken? I.e.: what does it mean "an ID3 tag that
  was filled by zeroes" ?

• We do not have any sample files here which fail detection. Care
  to send or give links?

• Does detection work if ID3 is skipped?

• Your change seems to do a lot of search, can it not lead to any
  mis-detections?

On 2021-02-05 14:52:00 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 40)

• Support the linking with both FluidSynth 2 and 1, the time is going and
  there is FluidSynth 2 getting widely. Right now at me on Linux Mint 20.1 the
  FluidSynth 2.1.1 is used. The difference is the return type of the
  delete_fluid_player() and the delete_fluid_synth() calls:
  https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/codecs/
  music_fluidsynth.c#L34-L38. Maybe there would be a better solution that
  allows dynamic use of each FluidSynth 1 and FluidSynth 2 by making some of
  the calls be optional and be a replacement to each other.

This one is a simple change that can be added independent of this bug.

On 2021-02-05 18:41:59 +0000, Ozkan Sezer wrote:

(In reply to Ozkan Sezer from comment # 43)

(In reply to Vitaly Novichkov from comment # 40)

• Support the linking with both FluidSynth 2 and 1, the time is going and
  there is FluidSynth 2 getting widely. Right now at me on Linux Mint 20.1 the
  FluidSynth 2.1.1 is used. The difference is the return type of the

This one is a simple change that can be added independent of this bug.

Fixed in both SDL-1.2 and SDL2.0 (default) branches.

On 2021-02-05 19:11:12 +0000, Vitaly Novichkov wrote:

(In reply to Ozkan Sezer from comment # 42)

If ID3 is the only problem with detection:

• Is the ID3 tag broken? I.e.: what does it mean "an ID3 tag that
  was filled by zeroes" ?

This means at beginning of the file is the just zeroes sequence until the actual MP3 data begins. I meant that in the file sample I had, was an attempt to remove the ID3 tag by just filling it with zeroes. Because of that, the MP3 file gets miss detected when using a magic-number way.

• We do not have any sample files here which fail detection. Care
  to send or give links?

Lemme check my archives, I have at least one of such broken files, I still remind the song name.

• Does detection work if ID3 is skipped?
  It can't skip ID3 because the file has no ID3, it has just a sequence of zeroes at begin.

• Your change seems to do a lot of search, can it not lead to any
  miss-detections?
  I polished this function on many files I had around, and it doesn't miss-detect non-MP3 files in the rest. I do search the MP3 header in the first 10240 bytes. In the past, I had another variant of this detector that was less accurate, and it had some miss-detections on random files.

How it works:

• if ID3 at the beginning of a file as usually
• gets the end of the file
• checks if the first 4 bytes are zeroes, then it skips the reading loop and goes to check the MPEG header that fails and starts the scanner (I don't remind why I made this, I may guess this part of code is junk)
• does the search loop that finds any non-zero bytes in the stream until condition:
  -- found a non-zero byte
  -- bytes left less than 4 to end
  -- seek position is more than 10240 bytes
  -- end of file has reached
• Does the well-known check

Anyway, to confirm this algorithm works fine, I gonna make the unit test that will scan thousands of files I have in my collection and give me any possible miss-detections such as:

• MP3 file got detected as not-MP3
• not-MP3 file got detected as MP3

On 2021-02-05 19:13:50 +0000, Vitaly Novichkov wrote:

Created attachment 4764
An example MP3 file with the sequence of zeroes at begin

This is one of the MP3 files that won't be detected properly with the older MP3 detector.

On 2021-02-05 19:26:54 +0000, Ozkan Sezer wrote:

If ID3 is the only problem with detection:

• Is the ID3 tag broken? I.e.: what does it mean "an ID3 tag that
  was filled by zeroes" ?

This means at beginning of the file is the just zeroes sequence until the
actual MP3 data begins. I meant that in the file sample I had, was an
attempt to remove the ID3 tag by just filling it with zeroes. Because of
that, the MP3 file gets miss detected when using a magic-number way.

Unless I'm missing something, this sounds like a user-shoots-foot
situation..

On 2021-02-05 19:28:42 +0000, Vitaly Novichkov wrote:

(In reply to Ozkan Sezer from comment # 47)

Unless I'm missing something, this sounds like a user-shoots-foot
situation..

This file I had to attach, perfectly opens in any other players include VLC, Windows Media Player, etc. etc.

On 2021-02-05 19:31:49 +0000, Vitaly Novichkov wrote:

I got this file in the wild many years ago, and together with this, I have a dozen of others. I also found some examples where the file got chopped in a middle, and the MPEG frame got offset. Even that, those files also getting be played fine anywhere.

On 2021-02-05 19:43:25 +0000, Ozkan Sezer wrote:

(In reply to Vitaly Novichkov from comment # 48)

(In reply to Ozkan Sezer from comment # 47)

Unless I'm missing something, this sounds like a user-shoots-foot
situation..

This file I had to attach, perfectly opens in any other players include VLC,
Windows Media Player, etc. etc.

$ file moi_drug_luchshe_vseh_igraet_blu.mp3
moi_drug_luchshe_vseh_igraet_blu.mp3: data

I'll let Sam and/or Ryan to decide.

[sezero]

@slouken: Waiting your comments.
See comments from 2021-02-05 12:16:44 - and later.
(I miss Bugzilla...)

[sezero]

Adding Vitaly's mad duration patch here, so I don't lose it
(and others may possibly review it, too.)

mad-tell-duration-patch.txt

[Wohlstand]

Adding Vitaly's mad duration patch here, so I don't lose it
(and others may possibly review it, too.)

mad-tell-duration-patch.txt

Maybe post it as a pull-request?

P.S. I'll be available tomorrow (UTC+3), now I gonna sleep.

[slouken]

@Wohlstand, let's revisit this for SDL_mixer 3.0?

It would be nice to put our heads together on features available in each library and see if it makes sense to create a merged version.

[Wohlstand]

Hello!
A good note, I can help.
At first, I'll write the full list of MixerX's features that were already being implemented and supported, and then you can give some about that. Just a note, we can perform the thing FASTER if we reduce the amount of buerocraty against every feature. There is a serious problem that many features at MixerX had been developed a while ago, and several of them depends on each other, and it's pretty hard to port each of them separately, but its possible to port as a "biggie" that can be integrated much easier.

P.S. everything was done over Mixer 2, and I took an effort to keep the full forward compatibility (including ABI compatibility to apps linked to original Mixer).

What about stability: the whole thing gets tested and polished while it used in several projects (mine as well, as other's people whom I don't know ever, and they reports me my bugs sometimes).

[Wohlstand]

@slouken

Okay, I going to share my set of features that MixerX has right now which original Mixer doesn't:

• Multi-Music - an ability to run multiple parallel streams of Mix_Music* things. I made a set of functions with the *Stream suffix which actually done to use this this functionality compare to the single-stream music thing. No-no, this is NOT a replacement of chunks for the reason, chunks do play instantly, and the same chunk can be played multiple times in parallel (the same chunk in multiple channels). Music can't that without copying an instance (i.e. opening the same music file as a second Mix_Music instance). This API allows the cross-fade effect that was impossible before I implemented this API.
• Advanced MIDI support - I implemented a pile of functions for taking advantages of MIDI format:
  • Ability to switch MIDI playback during runtime, and even run different songs with different MIDI synthesizer libraries and different setup of them. (The Timidity is only can't do that because it runs everything without of instances at all).
  • About Timidity, I have the ability to switch its config file during runtime, I can test different hand-made banks easily. I am one of these people who crafter several custom banks for Timidity (Using very old Awave program that is able to open and save .PAT files).
  • I added my own MIDI synthesizer libraries such as libADLMIDI and libOPNMIDI, they are MIDI playing libraries that use emulated YMF262 and YM2612 sound generator chips. Their main purpose to play MIDI files like they sound in DOS games with AdLib or SoundBlaster cards, or like on the Sega Megadrive. HOWEVER, both libraries licensed with GPLv3 because of the code pieces used by me initially, and I putting an effort to gradually get rid of them and turn the license to at least the LGPL, here is a reason: there are several projects that wants to use libADLMIDI, but they can't, for example, a source port of the Descent game, it has a GPL-incompatible non-free license, so, they can't use my libraries even they do want that.
  • I applied my custom MIDI player module (it's C++ coded right now, but I want to re-code it into Pure-C eventually) that I use with FluidLite and with Windows's Native MIDI as it can play XMI and MUS files, and also, it can play MIDI files with loops, can easily seek/tell the position with seconds time, has an ability to change the tempo on the fly, etc.
  • Additionally, I added a pile of ADLMIDI/OPNMIDI related functions to switch these options.
• I implemented the concept of music arguments - there are special appendix lines written at end of the path to the music file (or given separately via function argument), and they can be used to alternate sounding and behaviour of music files which supports (path arguments do exist for MIDI musics and for GME-based chiptunes). The main goal of this feature is allowing users of applications that use MixerX to supply any extra music setup arguments even without direct manipulation to the library from the code. Also, these music arguments are able to specify the gain factor which can be used to individually adjust the volume of each music file (several chiptunes and MIDI files do sound too quite, and it's must be an ability to supply the gainig factor to make it have the volume that is on similar level as other music files).
• The Tempo change feature which allows to set different tempo of tracker musics, MIDI musics played via my libraries or via my custom MIDI playing module (I licensed it under MIT license), and chiptunes played via GME library.
• I have an API that can mute/unmute every individual channel/track of the GME chiptune, MIDI or tracker song.
• I reworked the file loading code to DON'T TRUST the filename extensions, and ALWAYS forward to the magic number checker. I kept the extension checker for a few number of formats only. Why I did that? Because several du@$#&es among users had to place MP3 files named as .ogg or different formats, or oppositely. And that became a problem in my projects intended to work with the old content made by users for the another project.
• Additionally, I implemented the support for PTCOP and PTTUNE file formats using PXTone library under its own very permissive license.

ALSO:

• I added an ability to set INDIVIDUAL hooks for Mix_Music instances.
• I added an ability to apply effects such as (panning / position) to every individual Mix_Music independently. It's needed for Multi-Music feature where are large streams can be used as a part of atmosphere streams, etc.
• Somebody contributed to me an ability to start the Mixer instance without opening of the SDL Audio output and allowing user to manually pull the output of Mixer and then play it through somewhere also.
• I added the usage of the FFMPEG to play some formats such as WMA, AAC, WebM-packed OPUS, etc.
• I added an ability to turn multi-channel OGG file into the "multi-track" OGG file with an ability to use the channel/track muting API mentioned above. This mode plays OGG files differently than usually: instead of passing the whole stream through SDL's AudioStream to convert it into stereo, channels of the OGG file gets requested as isolated chunks, then gets merged locally into stereo and then sent into the SDL AudioStream as a stereo (or mono, or surround, depending on setup of the mode). And when requesting muting of certain track, the group of channels are not gets mixed with the output.
• Additionally, I implemented a very experimental ability to change the speed of OGG files by changing of SDL AudioStream's sample rate, but, that goes not clear, and every act of the fade the sample rate change results in a sequence of clicks rather than do that clearly and smoothly, and I didn't figured why that happens. Don't worry, I know about SDL3's AudioStream that is able to perform such things, but in my case I attempted to work around the SDL2 as I still need for it to support platforms and APIs are dropped by SDL3 (for example, I need for WinMM as a stable fallback, on some platforms users report me problems with DirectSound and WASAPI because of bugs in their ASUS Xonar drivers, and the best workaround is use of WinMM for them).

I may extend the list if I remember something also I forgot to mention.