Make libcddb optional in cdaudio plugin

[diizzyy]

CDDB(1) is a legacy way of quering for metadata, public service(s) have changed operators multiple times, some less concerned about privacy and there's no "easy" way to submit new data. It would be nice if support for CDDB were optional and not hardcoded to libcddb.

Reference:
https://hydrogenaud.io/index.php?PHPSESSID=029bfbfmciujgqp7o212mip5vd&topic=125880.0

[radioactiveman]

From the referenced forum thread:

gnudb.org have started requiring emails to access their system.

Can you confirm that the Audio CD plugin is no longer working then?
And does replacing gnudb.gnudb.org with freedb.dbpoweramp.com it its settings make it work again?

All my computers have no longer a CD drive so I can't test it myself. 😃

[diizzyy]

It should however CDDB more or less deprecated in favour of Musicbrainz and/or AccurateRip Meta and it would it be nice if (lib)cddb wasn't a hard dependency.

[jlindgren90]

libcddb is a pretty small library (about 70 KB on x86_64). I don't think it's worth adding more complexity to the build system to save that amount of space.

If gnudb.gnudb.org is no longer publicly available, I suggest we simply disable CDDB by default at runtime (i.e. set use_cddb to FALSE in CDAudio::defaults).

[radioactiveman]

libcddb is a pretty small library (about 70 KB on x86_64). I don't think it's worth adding more complexity to the build system to save that amount of space.

Agreed.

If gnudb.gnudb.org is no longer publicly available, I suggest we simply disable CDDB by default at runtime (i.e. set use_cddb to FALSE in CDAudio::defaults).

I would prefer to change the URL instead of disabling CDDB by default.
Are you able to test this, @jlindgren90? I can't - see my previous post.

From the referenced forum thread:

gnudb.org have started requiring emails to access their system.

Can you confirm that the Audio CD plugin is no longer working then? And does replacing gnudb.gnudb.org with freedb.dbpoweramp.com it its settings make it work again?

All my computers have no longer a CD drive so I can't test it myself. 😃

[diizzyy]

Just to be clear, the issue isn't about space more of a library that doesn't service much of a purpose given "current" state of things.

fwiw, it's the last consumer of libcddb in FreeBSDs ports tree and it would be nice if we wouldn't need to disable the cdaudio completely.

[radioactiveman]

The patch below makes libcddb optional when using Meson where this was rather straightforward.

@jlindgren90: I couldn't figure out how to do that in configure.ac. Could you please assist here? Or are you fine with the Meson only approach?

diff --git a/configure.ac b/configure.ac
index 242229f95..e78e45f5b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -223,6 +223,7 @@ ENABLE_PLUGIN_WITH_DEP(cdaudio,
 
 if test $have_cdaudio = yes ; then
     GENERAL_PLUGINS="$GENERAL_PLUGINS cd-menu-items"
+    AC_DEFINE(HAVE_LIBCDDB, 1, [Define if libcddb is installed])
 fi
 
 ENABLE_PLUGIN_WITH_DEP(flac,
diff --git a/meson_options.txt b/meson_options.txt
index 2a29a125b..fbef006bb 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -30,6 +30,8 @@ option('amidiplug', type: 'boolean', value: true,
        description: 'Whether AMidiPlug plugin is enabled')
 option('cdaudio', type: 'boolean', value: true,
        description: 'Whether CD audio support is enabled')
+option('cdaudio-cddb', type: 'boolean', value: true,
+       description: 'Whether CDDB support for the Audio CD plugin is enabled')
 option('console', type: 'boolean', value: true,
        description: 'Whether Game Music Emu support is enabled')
 option('ffaudio', type: 'boolean', value: true,
diff --git a/src/cdaudio/cdaudio-ng.cc b/src/cdaudio/cdaudio-ng.cc
index 29b696655..d3eba968c 100644
--- a/src/cdaudio/cdaudio-ng.cc
+++ b/src/cdaudio/cdaudio-ng.cc
@@ -20,6 +20,7 @@
 
 #include <errno.h>
 #include <pthread.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -39,7 +40,9 @@
 #include <cdio/cdda.h>
 #endif
 
+#ifdef HAVE_LIBCDDB
 #include <cddb/cddb.h>
+#endif
 
 #include <libaudcore/audstrings.h>
 #include <libaudcore/hook.h>
@@ -124,11 +127,14 @@ const char CDAudio::about[] =
 const char * const CDAudio::defaults[] = {
  "disc_speed", "2",
  "use_cdtext", "TRUE",
+#ifdef HAVE_LIBCDDB
  "use_cddb", "TRUE",
  "cddbhttp", "FALSE",
  "cddbserver", "gnudb.gnudb.org",
  "cddbport", "8880",
- nullptr};
+#endif
+ nullptr
+};
 
 const PreferencesWidget CDAudio::widgets[] = {
     WidgetLabel (N_("<b>Device</b>")),
@@ -140,6 +146,7 @@ const PreferencesWidget CDAudio::widgets[] = {
     WidgetLabel (N_("<b>Metadata</b>")),
     WidgetCheck (N_("Use CD-Text"),
         WidgetBool ("CDDA", "use_cdtext")),
+#ifdef HAVE_LIBCDDB
     WidgetCheck (N_("Use CDDB"),
         WidgetBool ("CDDA", "use_cddb")),
     WidgetCheck (N_("Use HTTP instead of CDDBP"),
@@ -157,6 +164,7 @@ const PreferencesWidget CDAudio::widgets[] = {
         WidgetInt ("CDDA", "cddbport"),
         {0, 65535, 1},
         WIDGET_CHILD)
+#endif
 };
 
 const PluginPreferences CDAudio::prefs = {{widgets}};
@@ -221,7 +229,9 @@ bool CDAudio::init ()
         return false;
     }
 
+#ifdef HAVE_LIBCDDB
     libcddb_init ();
+#endif
 
     return true;
 }
@@ -345,7 +355,9 @@ void CDAudio::cleanup ()
     reset_trackinfo ();
     purge_func.stop ();
 
+#ifdef HAVE_LIBCDDB
     libcddb_shutdown ();
+#endif
 
     pthread_mutex_unlock (& mutex);
 }
@@ -598,6 +610,7 @@ static bool scan_cd ()
 
     if (!cdtext_was_available)
     {
+#ifdef HAVE_LIBCDDB
         /* initialize de cddb subsystem */
         cddb_conn_t *pcddb_conn = nullptr;
         cddb_disc_t *pcddb_disc = nullptr;
@@ -735,6 +748,7 @@ static bool scan_cd ()
 
         if (pcddb_conn != nullptr)
             cddb_destroy (pcddb_conn);
+#endif /* HAVE_LIBCDDB */
     }
 
     return true;
diff --git a/src/cdaudio/meson.build b/src/cdaudio/meson.build
index ad1fbc1ac..dad6df2e6 100644
--- a/src/cdaudio/meson.build
+++ b/src/cdaudio/meson.build
@@ -1,17 +1,24 @@
 libcdio_dep = dependency('libcdio', version: '>= 0.70', required: false)
 libcdio_cdda_dep = dependency('libcdio_cdda', version: '>= 0.70', required: false)
-libcddb_dep = dependency('libcddb', version: '>= 1.2.1', required: false)
 
+have_cdaudio = libcdio_dep.found() and libcdio_cdda_dep.found()
+cdaudio_deps = [audacious_dep, libcdio_dep, libcdio_cdda_dep]
 
-have_cdaudio = libcdio_dep.found() and \
-               libcdio_cdda_dep.found() and \
-               libcddb_dep.found()
+if get_option('cdaudio-cddb')
+  libcddb_dep = dependency('libcddb', version: '>= 1.2.1', required: false)
 
+  if libcddb_dep.found()
+    cdaudio_deps += libcddb_dep
+    conf.set10('HAVE_LIBCDDB', true)
+  else
+    warning('CDDB support for the Audio CD plugin was disabled due to missing dependency.')
+  endif
+endif
 
 if have_cdaudio
   shared_module('cdaudio-ng',
     'cdaudio-ng.cc',
-    dependencies: [audacious_dep, libcdio_dep, libcdio_cdda_dep, libcddb_dep],
+    dependencies: cdaudio_deps,
     name_prefix: '',
     install: true,
     install_dir: input_plugin_dir
diff --git a/src/config.h.meson b/src/config.h.meson
index 70d6c30f6..167206deb 100644
--- a/src/config.h.meson
+++ b/src/config.h.meson
@@ -28,6 +28,7 @@
 #mesondefine FILEWRITER_FLAC
 #mesondefine FILEWRITER_VORBIS
 
+#mesondefine HAVE_LIBCDDB
 #mesondefine HAVE_LIBCUE2
 #mesondefine HAVE_SNDIO_1_9
 

[jlindgren90]

@radioactiveman I am fine with adding the option for meson only as your patch does. Since autoconf is a "legacy" option anyway, I don't think it is worth the hassle to add the new option there.

[radioactiveman]

All right, pushed as audacious-media-player/audacious-plugins@0ffe6d8.

[diizzyy]

Thanks!