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

@@ -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

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 , )

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<double>(REDBOOK_PCM_BYTES_PER_MS * 1000 * 60));
 	} else {
+		LOG_MSG("CDROM: Failed adding '%s' as CDDA track!", filename_only.c_str());
 		error = true;
 	}
 }

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) \

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 }
 };
 

src/libs/decoders/mp3.cpp

@@ -90,15 +90,11 @@ static void MP3_close(Sound_Sample* const sample)
 {
     Sound_SampleInternal* const internal = static_cast<Sound_SampleInternal*>(sample->opaque);
     mp3_t* p_mp3 = static_cast<mp3_t*>(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)