-
Notifications
You must be signed in to change notification settings - Fork 33
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
Switch to libxmp for most tracker formats #62
Conversation
DSIK still uses foo_DUMB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix these warnings:
6>LINK : warning LNK4217: symbol 'xmp_create_context' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"class StreamSource * __cdecl XMP_OpenSong(struct MusicIO::FileInterface *,int)" (?XMP_OpenSong@@YAPEAVStreamSource@@PEAUFileInterface@MusicIO@@h@Z)'
6>LINK : warning LNK4217: symbol 'xmp_load_module_from_callbacks' defined in 'load.obj' is imported by 'music_libxmp.obj' in function '"class StreamSource * __cdecl XMP_OpenSong(struct MusicIO::FileInterface *,int)" (?XMP_OpenSong@@YAPEAVStreamSource@@PEAUFileInterface@MusicIO@@h@Z)'
6>LINK : warning LNK4217: symbol 'xmp_test_module_from_callbacks' defined in 'load.obj' is imported by 'music_libxmp.obj' in function '"class StreamSource * __cdecl XMP_OpenSong(struct MusicIO::FileInterface *,int)" (?XMP_OpenSong@@YAPEAVStreamSource@@PEAUFileInterface@MusicIO@@h@Z)'
6>LINK : warning LNK4217: symbol 'xmp_start_player' defined in 'player.obj' is imported by 'music_libxmp.obj' in function '"public: virtual bool __cdecl XMPSong::Start(void)" (?Start@XMPSong@@UEAA_NXZ)'
6>LINK : warning LNK4217: symbol 'xmp_play_buffer' defined in 'player.obj' is imported by 'music_libxmp.obj' in function '"protected: virtual bool __cdecl XMPSong::GetData(void *,unsigned __int64)" (?GetData@XMPSong@@MEAA_NPEAX_K@Z)'
6>LINK : warning LNK4217: symbol 'xmp_set_position' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"protected: virtual bool __cdecl XMPSong::GetData(void *,unsigned __int64)" (?GetData@XMPSong@@MEAA_NPEAX_K@Z)'
6>LINK : warning LNK4217: symbol 'xmp_restart_module' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"protected: virtual bool __cdecl XMPSong::GetData(void *,unsigned __int64)" (?GetData@XMPSong@@MEAA_NPEAX_K@Z)'
6>LINK : warning LNK4217: symbol 'xmp_set_player' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"public: __cdecl XMPSong::XMPSong(char *,int)" (??0XMPSong@@qeaa@PEADH@Z)'
6>LINK : warning LNK4217: symbol 'xmp_get_player' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"public: virtual bool __cdecl XMPSong::SetSubsong(int)" (?SetSubsong@XMPSong@@UEAA_NH@Z)'
5>LINK : warning LNK4217: symbol 'xmp_create_context' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"class StreamSource * __cdecl XMP_OpenSong(struct MusicIO::FileInterface *,int)" (?XMP_OpenSong@@YAPEAVStreamSource@@PEAUFileInterface@MusicIO@@h@Z)'
5>LINK : warning LNK4217: symbol 'xmp_load_module_from_callbacks' defined in 'load.obj' is imported by 'music_libxmp.obj' in function '"class StreamSource * __cdecl XMP_OpenSong(struct MusicIO::FileInterface *,int)" (?XMP_OpenSong@@YAPEAVStreamSource@@PEAUFileInterface@MusicIO@@h@Z)'
5>LINK : warning LNK4217: symbol 'xmp_test_module_from_callbacks' defined in 'load.obj' is imported by 'music_libxmp.obj' in function '"class StreamSource * __cdecl XMP_OpenSong(struct MusicIO::FileInterface *,int)" (?XMP_OpenSong@@YAPEAVStreamSource@@PEAUFileInterface@MusicIO@@h@Z)'
5>LINK : warning LNK4217: symbol 'xmp_start_player' defined in 'player.obj' is imported by 'music_libxmp.obj' in function '"public: virtual bool __cdecl XMPSong::Start(void)" (?Start@XMPSong@@UEAA_NXZ)'
5>LINK : warning LNK4217: symbol 'xmp_play_buffer' defined in 'player.obj' is imported by 'music_libxmp.obj' in function '"protected: virtual bool __cdecl XMPSong::GetData(void *,unsigned __int64)" (?GetData@XMPSong@@MEAA_NPEAX_K@Z)'
5>LINK : warning LNK4217: symbol 'xmp_set_position' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"protected: virtual bool __cdecl XMPSong::GetData(void *,unsigned __int64)" (?GetData@XMPSong@@MEAA_NPEAX_K@Z)'
5>LINK : warning LNK4217: symbol 'xmp_restart_module' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"protected: virtual bool __cdecl XMPSong::GetData(void *,unsigned __int64)" (?GetData@XMPSong@@MEAA_NPEAX_K@Z)'
5>LINK : warning LNK4217: symbol 'xmp_set_player' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"public: __cdecl XMPSong::XMPSong(char *,int)" (??0XMPSong@@qeaa@PEADH@Z)'
5>LINK : warning LNK4217: symbol 'xmp_get_player' defined in 'control.obj' is imported by 'music_libxmp.obj' in function '"public: virtual bool __cdecl XMPSong::SetSubsong(int)" (?SetSubsong@XMPSong@@UEAA_NH@Z)'
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not apply the master volume through the library's API - the output is only 16 bit integer which has finite range. Instead do it like the DUMB player does by converting the sound to float and then apply it during the conversion. With 16 bit this can easily cause clipping and distortion.
libxmp's output shouldn't require float conversion on any merits; it already internally clamps the output to avoid clipping problems and was tested specifically for that. It also employs additional safeguards to not cause clipping problems IIRC. So it doesn't make sense to convert the output to float and apply the master volume like that when the library can already do it without any problems. |
Switched to float output anyways. |
That's better. I think we should add an option to select the default module player as a user setting and as a song setting, similar to $mididevice. |
I don't really believe DUMB should be made available anymore; the author tells people to use libopenmpt because of it having similar or better playback quality than it, and it's not exactly maintained. It can be maybe left as an option anyways, but libxmp should remain the default for now. |
Our Dumb version hasn't been updated for 10 years or so, so it doesn't really matter what the author says - but the fact remains that it can sound different and some people may be bothered by it. The option doesn't cost much except for a new entry in the menu, so why not? |
This PR switches to libxmp (the 4.6.0 release) for most tracker formats (DUMB was officially declared dead in April of this year).
DSM still needs foo_DUMB for playback since libxmp does not support it so it has been left there.
What is supported:
What isn't supported: