Skip to content

Commit

Permalink
DUMB 2.0 and DUMB 0.9.3 compatibility
Browse files Browse the repository at this point in the history 
We support two versions of DUMB to play tracked music:

* 0.9.3 from Sourceforge <http://dumb.sourceforge.net>
* 2.0.0 by kode54 on github <https://github.com/kode54/dumb>

The DUMB homepage on Sourceforge endorses kode54's fork and links to it.
The Allegro source tests the DUMB version and supports either API.
Supported tracker formats in DUMB 2.0 that Allegro 5 can now play:

* IT (Impulse Tracker)
* XM (Fasttracker II)
* MOD (Ultimate SoundTracker, ProTracker)
* STM (Scream Tracker)
* S3M (Scream Tracker 3)
* 669 (Composer 669)
* AMF (Asylum Music Format)
* AMF (Digital Sound and Music Interface Advanced Music Format)
* DSM (Digital Sound Interface Kit module format)
* MTM (MultiTracker)
* OKT (Oktalyzer)
* PSM (Protracker Studio, both the older PSM16 and the newer PSM)
* PTM (PolyTracker)
* RIFF AM/AMFF (Galaxy Music System internal format)

The CMake build script uses -lm on all platforms for the compile test.
Without -lm, cmake finds the include paths and libraries, can compile
the test program, but couldn't link it on Arch Linux.
  • Loading branch information
@SimonN
SimonN committed Sep 28, 2017
1 parent c97f009 commit 5e2ac76
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 35 deletions.
9 changes: 7 additions & 2 deletions addons/acodec/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ if(WANT_MODAUDIO)
set(CMAKE_REQUIRED_LIBRARIES ${DUMB_LIBRARIES})
run_c_compile_test("
#include <dumb.h>
#if (DUMB_MAJOR_VERSION) == 1
#error libdumb 1.0 not supported, get >= 2.0 or 0.9.3
#endif
int main(void)
{
dumb_register_stdfiles();
Expand All @@ -155,8 +158,10 @@ if(WANT_MODAUDIO)
endif(DUMB_COMPILES)
endif(DUMB_FOUND)
if(NOT SUPPORT_MODAUDIO)
message("WARNING: libdumb not found or compile test failed, "
"disabling support. <http://dumb.sourceforge.net/>")
message("WARNING: libdumb >= 2.0 or <= 0.9.3 not found or compile "
"test failed, disabling support. See "
"<https://github.com/kode54/dumb> for 2.0 or "
"<http://dumb.sourceforge.net/> for 0.9.3.")
endif(NOT SUPPORT_MODAUDIO)
endif(WANT_MODAUDIO)

Expand Down
45 changes: 44 additions & 1 deletion addons/acodec/acodec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
#include "allegro5/internal/aintern_acodec_cfg.h"
#include "acodec.h"

#ifdef ALLEGRO_CFG_ACODEC_MODAUDIO
#include <dumb.h>
#endif

/* Function: al_get_allegro_acodec_version
*/
Expand Down Expand Up @@ -38,6 +41,45 @@ bool al_init_acodec_addon(void)
#endif

#ifdef ALLEGRO_CFG_ACODEC_MODAUDIO
#if (DUMB_MAJOR_VERSION) >= 2
/*
* DUMB 2.0 offers a single loader for at least 13 formats, see their
* readme. Amiga NoiseTracker isn't listed there, but it's DUMB-supported.
* It merely has no common extensions, see:
* https://github.com/kode54/dumb/issues/53
*/
ret &= al_register_audio_stream_loader(".669", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".669", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".amf", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".amf", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".asy", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".asy", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".it", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".it", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".mod", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".mod", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".mtm", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".mtm", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".okt", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".okt", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".psm", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".psm", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".ptm", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".ptm", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".riff", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".riff", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".s3m", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".s3m", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".stm", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".stm", _al_load_dumb_audio_stream_f);
ret &= al_register_audio_stream_loader(".xm", _al_load_dumb_audio_stream);
ret &= al_register_audio_stream_loader_f(".xm", _al_load_dumb_audio_stream_f);
#else
/*
* DUMB 0.9.3 supported only these 4 formats and had no *_any loader.
* Avoid DUMB 1.0 because of versioning problems: dumb.h from git tag 1.0
* reports 0.9.3 in its version numbers.
*/
ret &= al_register_audio_stream_loader(".xm", _al_load_xm_audio_stream);
ret &= al_register_audio_stream_loader_f(".xm", _al_load_xm_audio_stream_f);
ret &= al_register_audio_stream_loader(".it", _al_load_it_audio_stream);
Expand All @@ -46,7 +88,8 @@ bool al_init_acodec_addon(void)
ret &= al_register_audio_stream_loader_f(".mod", _al_load_mod_audio_stream_f);
ret &= al_register_audio_stream_loader(".s3m", _al_load_s3m_audio_stream);
ret &= al_register_audio_stream_loader_f(".s3m", _al_load_s3m_audio_stream_f);
#endif
#endif // DUMB_MAJOR_VERSION
#endif // ALLEGRO_CFG_ACODEC_MODAUDIO

#ifdef ALLEGRO_CFG_ACODEC_VORBIS
ret &= al_register_sample_loader(".ogg", _al_load_ogg_vorbis);
Expand Down
10 changes: 9 additions & 1 deletion addons/acodec/acodec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ ALLEGRO_AUDIO_STREAM *_al_load_flac_audio_stream_f(ALLEGRO_FILE* f,
#endif

#ifdef ALLEGRO_CFG_ACODEC_MODAUDIO
#include <dumb.h>
#if (DUMB_MAJOR_VERSION) >= 2
ALLEGRO_AUDIO_STREAM *_al_load_dumb_audio_stream(const char *filename,
size_t buffer_count, unsigned int samples);
ALLEGRO_AUDIO_STREAM *_al_load_dumb_audio_stream_f(ALLEGRO_FILE *f,
size_t buffer_count, unsigned int samples);
#else
ALLEGRO_AUDIO_STREAM *_al_load_mod_audio_stream(const char *filename,
size_t buffer_count, unsigned int samples);
ALLEGRO_AUDIO_STREAM *_al_load_it_audio_stream(const char *filename,
Expand All @@ -47,7 +54,8 @@ ALLEGRO_AUDIO_STREAM *_al_load_xm_audio_stream_f(ALLEGRO_FILE *f,
size_t buffer_count, unsigned int samples);
ALLEGRO_AUDIO_STREAM *_al_load_s3m_audio_stream_f(ALLEGRO_FILE *f,
size_t buffer_count, unsigned int samples);
#endif
#endif // DUMB_MAJOR_VERSION
#endif // ALLEGRO_CFG_ACODEC_MODAUDIO

#ifdef ALLEGRO_CFG_ACODEC_VORBIS
ALLEGRO_SAMPLE *_al_load_ogg_vorbis(const char *filename);
Expand Down
Loading

11 comments on commit 5e2ac76

@NewCreature
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having a problem building Allegro with DUMB 2.0 on my machine that runs Ubuntu 16.04 32-bit. I get this error:

/usr/local/include/dumb.h:121:1: error: static assertion failed: "fuse: off_t must be 64bit"

I found this comment in the DUMB source code:

/*
 * If the build fails here, it does so, because we need a 64-bit-type for
 * defining offsets. To fix this do either of the following:
 *
 * 1. Compile your code with -D_FILE_OFFSET_BITS=64, so that off_t is 64-bit
 *    (recommended, but make sure the rest of your code can handle it)
 * 2. Supply your own definition of a signed 64-bit integer
 *    such as off64_t or int64_t before including dumb.h as follows:
 *    #define DUMB_OFF_T_CUSTOM int64_t
 */

Seems like the issue should be easy enough to fix, but I'm not sure how implementing one of these solutions might affect the rest of Allegro (option 1). DUMB has an issue with option 2 at the moment due to them having the syntax for typedef reversed.

@SiegeLord
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried option 1, and it appears to work fine (although you have to define it in quite a few files since we include dumb.h in an internal header). We already use #define _FILE_OFFSET_BITS 64 in a few spots, so in principle things should be fine. I tested it with DUMB 0.9.3, and everything seemed to be okay (as it should be, as that version doesn't use off_t).

@fatcerberus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn’t we just define _FILE_OFFSET_BITS in one place (the internal header you mentioned, where dumb.h is included)? I don’t understand why we’d have to define it in multiple places, that doesn’t sound very nice for maintenance...

@SimonN
Copy link
Contributor Author

@SimonN SimonN commented on 5e2ac76 Oct 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option 1 sounds reasonable if A5 has #define _FILE_OFFSET_BITS 64 already elsewhere. I'll leave it to you where to put it, one central place sounds nice.

DUMB has an issue with option 2 at the moment due to them having the syntax for typedef reversed.

kode54 fixed it upstream within the last hour: kode54/dumb#66

@SimonN
Copy link
Contributor Author

@SimonN SimonN commented on 5e2ac76 Oct 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rondom found a bug in my patch (the Allegro 5 diff above), too.

/*
 * dumb_off_t is introduced in DUMB 2.0, it's a signed 64+ bit integer.
 * dumb_ssize_t is a platform-independent signed size_t.
 * DUMB 0.9.3 expects long wherever dumb_off_t or dumb_ssize_t appear.
 */
#ifndef dumb_off_t
#define dumb_off_t long
#endif
#ifndef dumb_ssize_t
#define dumb_ssize_t long
#endif

These #ifndefs should be #if (DUMB_MAJOR_VERSION) < 2. Reason: dumb_off_t is not a macro, it is a typedef. The preprocessor will always believe this is not defined, and therefore replace too much.

PR for this: #839

@SiegeLord
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fatcerberus, that's just a C/C++ thing. The internal header isn't included first in those files, so putting the define there wouldn't work unless you also changed a few more things. I'll see what the cleanest option ends up being.

@fatcerberus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant though, is since the #define is only needed for dumb.h (correct me if I'm wrong), then it should be enough to do this regardless of include order:

#define _FILE_OFFSET_BITS 64
#include <dumb.h>

Allegro doesn't need the define itself, right?

@SiegeLord
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, that define acts on some system headers that we also include before <dumb.h> (I think stdio.h?)

@fatcerberus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that makes more sense then, sorry for being stupid :)

@SiegeLord
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine now as of dfbef0e.

@SimonN
Copy link
Contributor Author

@SimonN SimonN commented on 5e2ac76 Oct 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allegro will probably release during 2017-10 before TINS 2017 with SiegeLord's fix for option 1 under the hood -- defining _FILE_OFFSET_BITS 64 in a private header.

DUMB 2.0.2 got tagged 2 days ago, with the correct line typedef DUMB_OFF_T_CUSTOM dumb_off_t; and Rondom's better platform/compiler-checking code to define a 64-bit offset.

I hope everything will turn out fine now!

Please sign in to comment.