From f6060a51488e9cd3989bb362940e60b62fb1f410 Mon Sep 17 00:00:00 2001 From: krcroft Date: Fri, 24 Jan 2020 10:04:53 -0800 Subject: [PATCH] Allow Opus CDDA support to be optionally disabled Adds a `--disable-opus-cdda` flag that explicitly disables support for Ogg Opus CDDA tracks and in turn avoid the need for the Opus package dependencies such as the opusfile, opus, and ogg libraries. This feature does not alter the default operation of ./configure, which is to enable Opus CDDA support and quit if the Opus dependency package, opusfile, is not found. The user can then choose to either a) install the package or b) explicitly disable Opus support. This commit also includes: - fixes for a double-free in the MP3 close routine that was discovered during testing - a message if a CD audio track cannot be added during CDROM mounting (such as attempting to use Opus tracks when the binary does not support them). - the --disable-opus-cdda flag in our config heavy workflow --- .github/workflows/config.yml | 1 + configure.ac | 21 +++++++++++++++++---- src/dos/cdrom_image.cpp | 5 +++-- src/libs/decoders/Makefile.am | 7 ++++++- src/libs/decoders/SDL_sound.c | 27 ++++++++++++++------------- src/libs/decoders/mp3.cpp | 10 +++------- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/.github/workflows/config.yml b/.github/workflows/config.yml index 08af2f33..58a8fe76 100644 --- a/.github/workflows/config.yml +++ b/.github/workflows/config.yml @@ -29,6 +29,7 @@ jobs: - configure_flags: --disable-fpu-x64 - configure_flags: --disable-fpu-x86 - configure_flags: --disable-opengl + - configure_flags: --disable-opus-cdda - configure_flags: --disable-screenshots - configure_flags: --disable-unaligned-memory - without_packages: -x net diff --git a/configure.ac b/configure.ac index 22745d9a..0f8389ef 100644 --- a/configure.ac +++ b/configure.ac @@ -459,10 +459,23 @@ else AC_MSG_WARN([Can't find SDL_net, internal modem and ipx disabled]) fi -# Check for the Opus file-handling library -PKG_CHECK_MODULES([OPUSFILE], [opusfile], - [ LIBS="$LIBS ${OPUSFILE_LIBS}" - CPPFLAGS="$CPPFLAGS ${OPUSFILE_CFLAGS}" ], []) +dnl Ogg Opus handling +dnl ----------------- +dnl We want Opus tracks supported by default, and we also want the user +dnl to be aware if there's a problem finding the Opus dependencies. +dnl If there's a problem, we want the user to either a) install the opus +dnl library or b) explicitly disable Opus. + +dnl To achieve this, we provide a --disable-opus-cdda configure option +dnl but by default we look for Opus and fail if we can't find it. +dnl We only skip supporting Opus if the user explicitly passes the +dnl --disable-opus-cdda argument. + +AC_ARG_ENABLE([opus-cdda], AS_HELP_STRING([--disable-opus-cdda], + [Disable support for Opus-compressed CD-Audio tracks])) +AS_IF([test "x$enable_opus_cdda" != "xno"], + [PKG_CHECK_MODULES([OPUSFILE], [opusfile], [HAVE_OPUSFILE=1])]) +AM_CONDITIONAL([USE_OPUS], [test "$HAVE_OPUSFILE" == "1"]) AH_TEMPLATE(C_OPENGL,[Define to 1 to use opengl display output support]) AC_CHECK_LIB(GL, main, have_gl_lib=yes, have_gl_lib=no , ) diff --git a/src/dos/cdrom_image.cpp b/src/dos/cdrom_image.cpp index 2896295c..7bd78562 100644 --- a/src/dos/cdrom_image.cpp +++ b/src/dos/cdrom_image.cpp @@ -125,16 +125,17 @@ CDROM_Interface_Image::AudioFile::AudioFile(const char *filename, bool &error) // Use the audio file's actual sample rate and number of channels as opposed to overriding Sound_AudioInfo desired = {AUDIO_S16, 0, 0}; sample = Sound_NewSampleFromFile(filename, &desired); + std::string filename_only(filename); + filename_only = filename_only.substr(filename_only.find_last_of("\\/") + 1); if (sample) { error = false; - std::string filename_only(filename); - filename_only = filename_only.substr(filename_only.find_last_of("\\/") + 1); LOG_MSG("CDROM: Loaded %s [%d Hz, %d-channel, %2.1f minutes]", filename_only.c_str(), getRate(), getChannels(), getLength()/static_cast(REDBOOK_PCM_BYTES_PER_MS * 1000 * 60)); } else { + LOG_MSG("CDROM: Failed adding '%s' as CDDA track!", filename_only.c_str()); error = true; } } diff --git a/src/libs/decoders/Makefile.am b/src/libs/decoders/Makefile.am index 76d425b3..5f180767 100644 --- a/src/libs/decoders/Makefile.am +++ b/src/libs/decoders/Makefile.am @@ -9,7 +9,6 @@ libdecoders_a_SOURCES = \ mp3.cpp \ mp3_seek_table.cpp \ mp3_seek_table.h \ - opus.c \ SDL_sound.c \ SDL_sound.h \ SDL_sound_internal.h \ @@ -21,6 +20,12 @@ libdecoders_a_SOURCES = \ xxhash.c \ xxhash.h +if USE_OPUS + libdecoders_a_SOURCES += opus.c + AM_CFLAGS += -DUSE_OPUS $(OPUSFILE_CFLAGS) + LDADD += $(OPUSFILE_LIBS) +endif + libdecoders_a_CXXFLAGS = \ $(AM_CXXFLAGS) \ $(CXXFLAGS) \ diff --git a/src/libs/decoders/SDL_sound.c b/src/libs/decoders/SDL_sound.c index 6fdd28b6..4dc30bb0 100644 --- a/src/libs/decoders/SDL_sound.c +++ b/src/libs/decoders/SDL_sound.c @@ -41,29 +41,30 @@ #define __SDL_SOUND_INTERNAL__ #include "SDL_sound_internal.h" - -/* The various decoder drivers... */ - -/* All these externs may be missing; we check SOUND_SUPPORTS_xxx before use. */ -extern const Sound_DecoderFunctions __Sound_DecoderFunctions_WAV; -extern const Sound_DecoderFunctions __Sound_DecoderFunctions_VORBIS; -extern const Sound_DecoderFunctions __Sound_DecoderFunctions_OPUS; -extern const Sound_DecoderFunctions __Sound_DecoderFunctions_FLAC; -extern const Sound_DecoderFunctions __Sound_DecoderFunctions_MP3; - typedef struct { int available; const Sound_DecoderFunctions *funcs; } decoder_element; +/* Supported decoder drivers... */ +extern const Sound_DecoderFunctions __Sound_DecoderFunctions_FLAC; +extern const Sound_DecoderFunctions __Sound_DecoderFunctions_MP3; +#ifdef USE_OPUS +extern const Sound_DecoderFunctions __Sound_DecoderFunctions_OPUS; +#endif +extern const Sound_DecoderFunctions __Sound_DecoderFunctions_VORBIS; +extern const Sound_DecoderFunctions __Sound_DecoderFunctions_WAV; + static decoder_element decoders[] = { - { 0, &__Sound_DecoderFunctions_WAV }, - { 0, &__Sound_DecoderFunctions_VORBIS }, - { 0, &__Sound_DecoderFunctions_OPUS }, { 0, &__Sound_DecoderFunctions_FLAC }, { 0, &__Sound_DecoderFunctions_MP3 }, +#ifdef USE_OPUS + { 0, &__Sound_DecoderFunctions_OPUS }, +#endif + { 0, &__Sound_DecoderFunctions_VORBIS }, + { 0, &__Sound_DecoderFunctions_WAV }, { 0, NULL } }; diff --git a/src/libs/decoders/mp3.cpp b/src/libs/decoders/mp3.cpp index 2f6568d9..7484ca83 100644 --- a/src/libs/decoders/mp3.cpp +++ b/src/libs/decoders/mp3.cpp @@ -90,15 +90,11 @@ static void MP3_close(Sound_Sample* const sample) { Sound_SampleInternal* const internal = static_cast(sample->opaque); mp3_t* p_mp3 = static_cast(internal->decoder_private); - if (p_mp3 != nullptr) { - if (p_mp3->p_dr != nullptr) { - drmp3_uninit(p_mp3->p_dr); - SDL_free(p_mp3->p_dr); - } - // maps and vector destructors free their memory + if (p_mp3) { + SDL_free(p_mp3->p_dr); SDL_free(p_mp3); - internal->decoder_private = nullptr; } + internal->decoder_private = nullptr; } /* MP3_close */ static Uint32 MP3_read(Sound_Sample* const sample, void* buffer, Uint32 desired_frames)