From 25ca76ec354120efa561879f50c486340e14d0ca Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Tue, 25 Sep 2018 21:41:20 +0200 Subject: [PATCH] afh: Constify definition of audio format handlers. The audio_format_handler structure contains only pointers, and the ->init method of each instance initializes these pointers to constant values. The ->init() method is thus useless at best, and it prevents the structures from being declared constant. This patch removes ->init() of struct audio_format_handler and the public afh_init() which iterates over all audio formats to call each ->init() method. The audio format handlers are modified to define an instance of the structure rather than an init function which fills the fields of the given structure. The structure can be declared constant, but not static because afh_common.c needs a way to refer to it. We rely on weak symbols to deal with audio format handlers which are not compiled in. The codec-independent code in afh_common.c defines a weak instance of the audio_format_handler structure for each audio format. The command handlers which are compiled in override the weak symbol with their own definition. The afh receiver used to define afh_init() as its (receiver!) init function, which no longer exists. Since receiver init functions are optional, we don't need to supply a replacement. However, play.c calls ->init() of the afh_receiver unconditionally. This call needs to be removed to avoid a null pointer dereference. --- aac_afh.c | 22 +++++------ afh.c | 1 - afh.h | 7 ---- afh_common.c | 101 +++++++++++---------------------------------------- afh_recv.c | 1 - flac_afh.c | 15 ++++---- mp3_afh.c | 15 ++++---- ogg_afh.c | 19 +++++----- opus_afh.c | 19 +++++----- play.c | 1 - server.c | 3 -- spx_afh.c | 17 +++++---- wma_afh.c | 16 ++++---- 13 files changed, 83 insertions(+), 154 deletions(-) diff --git a/aac_afh.c b/aac_afh.c index 3315a42d..7f2a22a2 100644 --- a/aac_afh.c +++ b/aac_afh.c @@ -333,17 +333,17 @@ close: } static const char * const aac_suffixes[] = {"m4a", "mp4", NULL}; + /** - * the init function of the aac audio format handler + * The audio format handler for the Advanced Audio Codec. * - * \param afh pointer to the struct to initialize + * This is only compiled in if the faad library is installed. */ -void aac_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = aac_get_file_info, - afh->suffixes = aac_suffixes; - afh->rewrite_tags = aac_afh_rewrite_tags; - afh->open = aac_afh_open; - afh->get_chunk = aac_afh_get_chunk; - afh->close = aac_afh_close; -} +const struct audio_format_handler aac_afh = { + .get_file_info = aac_get_file_info, + .suffixes = aac_suffixes, + .rewrite_tags = aac_afh_rewrite_tags, + .open = aac_afh_open, + .get_chunk = aac_afh_get_chunk, + .close = aac_afh_close, +}; diff --git a/afh.c b/afh.c index aa6570e1..69b834d2 100644 --- a/afh.c +++ b/afh.c @@ -203,7 +203,6 @@ int main(int argc, char **argv) loglevel = OPT_UINT32_VAL(LOGLEVEL); version_handle_flag("afh", OPT_GIVEN(VERSION)); handle_help_flags(); - afh_init(); for (i = 0; i < lls_num_inputs(lpr); i++) { int ret2; const char *path = lls_input(i, lpr); diff --git a/afh.h b/afh.h index eb539236..881db3c2 100644 --- a/afh.h +++ b/afh.h @@ -80,12 +80,6 @@ struct audio_file_data { * in the other part of this struct. */ struct audio_format_handler { - /** - * Pointer to the audio format handler's init function. - * - * Must initialize all function pointers and is assumed to succeed. - */ - void (*init)(struct audio_format_handler*); /** Typical file endings for files that can be handled by this afh. */ const char * const *suffixes; /** @@ -133,7 +127,6 @@ struct audio_format_handler { int output_fd, const char *filename); }; -void afh_init(void); int guess_audio_format(const char *name); int compute_afhi(const char *path, char *data, size_t size, int fd, struct afh_info *afhi); diff --git a/afh_common.c b/afh_common.c index fb4759a5..a267f58b 100644 --- a/afh_common.c +++ b/afh_common.c @@ -11,21 +11,6 @@ #include "string.h" #include "afh.h" -typedef void afh_init_func(struct audio_format_handler *); - -/* - * Declaration of the audio format handler init functions. - * - * These symbols are referenced in the afl array below. - * - * Most audio format handlers depend on an external library and are not - * compiled in if the library is not installed. Hence it is well possible that - * not all of these functions are defined. It does not hurt to declare them - * anyway, and this avoids another set of ifdefs. - */ -extern afh_init_func mp3_afh_init, ogg_afh_init, aac_afh_init, wma_afh_init, - spx_afh_init, flac_afh_init, opus_afh_init; - /** The list of all status items */ const char *status_item_list[] = {STATUS_ITEMS}; @@ -44,50 +29,21 @@ const char *status_item_list[] = {STATUS_ITEMS}; AUDIO_FORMAT(flac) \ AUDIO_FORMAT(opus) \ +/** \cond audio_format_handler */ #define AUDIO_FORMAT(_fmt) #_fmt, static const char * const audio_format_names[] = {ALL_AUDIO_FORMATS}; #undef AUDIO_FORMAT - -/* - * It can be detected whether an audio format is compiled in by checking if the - * init function pointer is NULL. - */ -static struct audio_format_handler afl[] = { - { - .init = mp3_afh_init, - }, - { -#if defined(HAVE_OGG) && defined(HAVE_VORBIS) - .init = ogg_afh_init, -#endif - }, - { -#if defined(HAVE_FAAD) - .init = aac_afh_init, -#endif - }, - { - .init = wma_afh_init, - }, - { -#if defined(HAVE_OGG) && defined(HAVE_SPEEX) - .init = spx_afh_init, -#endif - }, - { -#if defined(HAVE_OGG) && defined(HAVE_FLAC) - .init = flac_afh_init, -#endif - }, - { -#if defined(HAVE_OGG) && defined(HAVE_OPUS) - .init = opus_afh_init, -#endif - }, -}; - -/** This includes audio formats not compiled in. */ +/* Weak declarations must be public. */ +#define AUDIO_FORMAT(_fmt) \ + struct audio_format_handler _fmt ## _afh __attribute__ ((weak)) \ + = {.get_file_info = NULL}; +ALL_AUDIO_FORMATS +#undef AUDIO_FORMAT +#define AUDIO_FORMAT(_fmt) & _fmt ## _afh, +static struct audio_format_handler *afl[] = {ALL_AUDIO_FORMATS}; +#undef AUDIO_FORMAT #define NUM_AUDIO_FORMATS (ARRAY_SIZE(afl)) +/** \endcond audio_format_handler */ /** * Get the name of the given audio format. @@ -110,7 +66,7 @@ static inline int next_audio_format(int format) format++; if (format >= NUM_AUDIO_FORMATS) return format; - if (afl[format].init) + if (afl[format]->get_file_info) return format; } } @@ -119,21 +75,6 @@ static inline int next_audio_format(int format) #define FOR_EACH_AUDIO_FORMAT(i) \ for (i = 0; i < NUM_AUDIO_FORMATS; i = next_audio_format(i)) -/** - * Call the init function of each supported audio format handler. - */ -void afh_init(void) -{ - int i; - - PARA_NOTICE_LOG("supported audio formats: %s\n", AUDIO_FORMAT_HANDLERS); - FOR_EACH_AUDIO_FORMAT(i) { - PARA_INFO_LOG("initializing %s handler\n", - audio_format_name(i)); - afl[i].init(&afl[i]); - } -} - /** * Tell whether an audio format handler provides chunk tables. * @@ -147,7 +88,7 @@ void afh_init(void) */ bool afh_supports_dynamic_chunks(int audio_format_id) { - return afl[audio_format_id].get_chunk; + return afl[audio_format_id]->get_chunk; } /** @@ -164,8 +105,8 @@ int guess_audio_format(const char *name) int i,j, len = strlen(name); FOR_EACH_AUDIO_FORMAT(i) { - for (j = 0; afl[i].suffixes[j]; j++) { - const char *p = afl[i].suffixes[j]; + for (j = 0; afl[i]->suffixes[j]; j++) { + const char *p = afl[i]->suffixes[j]; int plen = strlen(p); if (len < plen + 1) continue; @@ -187,7 +128,7 @@ static int get_file_info(int format, const char *path, char *data, const char *fmt = audio_format_name(format); memset(afhi, 0, sizeof(*afhi)); - ret = afl[format].get_file_info(data, size, fd, afhi); + ret = afl[format]->get_file_info(data, size, fd, afhi); if (ret < 0) { PARA_WARNING_LOG("%s: %s format not detected: %s\n", path, fmt, para_strerror(-ret)); @@ -308,7 +249,7 @@ __must_check int afh_get_chunk(long unsigned chunk_num, struct afh_info *afhi, uint8_t audio_format_id, const void *map, size_t mapsize, const char **buf, size_t *len, void **afh_context) { - struct audio_format_handler *afh = afl + audio_format_id; + struct audio_format_handler *afh = afl[audio_format_id]; if (afh_supports_dynamic_chunks(audio_format_id)) { int ret; @@ -345,7 +286,7 @@ __must_check int afh_get_chunk(long unsigned chunk_num, struct afh_info *afhi, */ void afh_close(void *afh_context, uint8_t audio_format_id) { - struct audio_format_handler *afh = afl + audio_format_id; + struct audio_format_handler *afh = afl[audio_format_id]; if (!afh_supports_dynamic_chunks(audio_format_id)) return; @@ -408,7 +349,7 @@ int32_t afh_get_start_chunk(int32_t approx_chunk_num, void afh_get_header(struct afh_info *afhi, uint8_t audio_format_id, void *map, size_t mapsize, char **buf, size_t *len) { - struct audio_format_handler *afh = afl + audio_format_id; + struct audio_format_handler *afh = afl[audio_format_id]; if (!map || !afhi || !afhi->header_len) { *buf = NULL; @@ -431,7 +372,7 @@ void afh_get_header(struct afh_info *afhi, uint8_t audio_format_id, */ void afh_free_header(char *header_buf, uint8_t audio_format_id) { - struct audio_format_handler *afh = afl + audio_format_id; + struct audio_format_handler *afh = afl[audio_format_id]; if (afh->get_header) free(header_buf); @@ -535,7 +476,7 @@ void set_max_chunk_size(struct afh_info *afhi) int afh_rewrite_tags(int audio_format_id, void *map, size_t mapsize, struct taginfo *tags, int output_fd, const char *filename) { - struct audio_format_handler *afh = afl + audio_format_id; + struct audio_format_handler *afh = afl[audio_format_id]; if (!afh->rewrite_tags) return -ERRNO_TO_PARA_ERROR(ENOTSUP); diff --git a/afh_recv.c b/afh_recv.c index 6525209b..e5e87bdd 100644 --- a/afh_recv.c +++ b/afh_recv.c @@ -240,7 +240,6 @@ out: /** See \ref recv_init(). */ const struct receiver lsg_recv_cmd_com_afh_user_data = { - .init = afh_init, .open = afh_recv_open, .close = afh_recv_close, .pre_select = afh_recv_pre_select, diff --git a/flac_afh.c b/flac_afh.c index 45373ea1..6e236839 100644 --- a/flac_afh.c +++ b/flac_afh.c @@ -515,13 +515,12 @@ free_pfad: static const char * const flac_suffixes[] = {"flac", NULL}; /** - * The init function of the flac audio format handler. + * The audio format handler for flac (free lossless audio decoder). * - * \param afh pointer to the struct to initialize + * It depends on libflac and on libogg. */ -void flac_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = flac_get_file_info, - afh->suffixes = flac_suffixes; - afh->rewrite_tags = flac_rewrite_tags; -} +const struct audio_format_handler flac_afh = { + .get_file_info = flac_get_file_info, + .suffixes = flac_suffixes, + .rewrite_tags = flac_rewrite_tags, +}; diff --git a/mp3_afh.c b/mp3_afh.c index 42dd7539..e5027a0b 100644 --- a/mp3_afh.c +++ b/mp3_afh.c @@ -682,15 +682,14 @@ static int mp3_get_file_info(char *map, size_t numbytes, int fd, static const char * const mp3_suffixes[] = {"mp3", NULL}; /** - * the init function of the mp3 audio format handler + * The mp3 audio format handler. * - * \param afh pointer to the struct to initialize + * It does not depend on any libraries and is hence always compiled in. */ -void mp3_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = mp3_get_file_info; - afh->suffixes = mp3_suffixes; +const struct audio_format_handler mp3_afh = { + .get_file_info = mp3_get_file_info, + .suffixes = mp3_suffixes, #ifdef HAVE_LIBID3TAG - afh->rewrite_tags = mp3_rewrite_tags; + .rewrite_tags = mp3_rewrite_tags, #endif /* HAVE_LIBID3TAG */ -} +}; diff --git a/ogg_afh.c b/ogg_afh.c index 93ad14bb..5da339fe 100644 --- a/ogg_afh.c +++ b/ogg_afh.c @@ -173,14 +173,15 @@ static int vorbis_rewrite_tags(const char *map, size_t mapsize, static const char * const ogg_suffixes[] = {"ogg", NULL}; /** - * The init function of the ogg vorbis audio format handler. + * The ogg vorbis audio format handler. * - * \param afh Pointer to the struct to initialize. + * It should actually be called vorbis because the ogg container format is also + * employed for the speex and flac codecs. Both the ogg and the vorbis + * libraries must be installed to get this audio format handler. */ -void ogg_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = ogg_vorbis_get_file_info; - afh->get_header = vorbis_get_header; - afh->suffixes = ogg_suffixes; - afh->rewrite_tags = vorbis_rewrite_tags; -} +const struct audio_format_handler ogg_afh = { + .get_file_info = ogg_vorbis_get_file_info, + .get_header = vorbis_get_header, + .suffixes = ogg_suffixes, + .rewrite_tags = vorbis_rewrite_tags +}; diff --git a/opus_afh.c b/opus_afh.c index ed6fe5c8..dca6cfba 100644 --- a/opus_afh.c +++ b/opus_afh.c @@ -291,14 +291,15 @@ static void opus_get_header(void *map, size_t mapsize, char **buf, } /** - * The init function of the ogg/opus audio format handler. + * The audio format handler for ogg/opus. * - * \param afh Pointer to the struct to initialize. + * The opus codec was standardized by the Internet Engineering Task Force and + * is described in RFC 6716 (2012). The audio format handler depends on the ogg + * and the opus libraries. */ -void opus_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = opus_get_file_info, - afh->get_header = opus_get_header; - afh->suffixes = opus_suffixes; - afh->rewrite_tags = opus_rewrite_tags; -} +const struct audio_format_handler opus_afh = { + .get_file_info = opus_get_file_info, + .get_header = opus_get_header, + .suffixes = opus_suffixes, + .rewrite_tags = opus_rewrite_tags, +}; diff --git a/play.c b/play.c index 647367a3..c860208c 100644 --- a/play.c +++ b/play.c @@ -1243,7 +1243,6 @@ int main(int argc, char *argv[]) sched.default_timeout.tv_sec = 5; parse_config_or_die(argc, argv); - AFH_RECV->init(); session_open(); num_inputs = lls_num_inputs(play_lpr); init_shuffle_map(); diff --git a/server.c b/server.c index 6370cefd..e0d50f4f 100644 --- a/server.c +++ b/server.c @@ -583,9 +583,6 @@ static void server_init(int argc, char **argv, struct server_command_task *sct) init_ipc_or_die(); /* init mmd struct, mmd and log mutex */ daemon_set_start_time(); daemon_set_hooks(pre_log_hook, post_log_hook); - PARA_NOTICE_LOG("initializing audio format handlers\n"); - afh_init(); - /* * Although afs uses its own signal handling we must ignore SIGUSR1 * _before_ the afs child process gets born by init_afs() below. It's diff --git a/spx_afh.c b/spx_afh.c index e2323647..caeacb19 100644 --- a/spx_afh.c +++ b/spx_afh.c @@ -248,13 +248,14 @@ static int spx_rewrite_tags(const char *map, size_t mapsize, static const char * const speex_suffixes[] = {"spx", "speex", NULL}; /** - * The init function of the ogg/speex audio format handler. + * The ogg/speex audio format handler. * - * \param afh Pointer to the struct to initialize. + * This codec is considered obsolete because the opus codec surpasses its + * performance in all areas. It is only compiled in if both the ogg and the + * speex library are installed. */ -void spx_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = spx_get_file_info, - afh->suffixes = speex_suffixes; - afh->rewrite_tags = spx_rewrite_tags; -} +const struct audio_format_handler spx_afh = { + .get_file_info = spx_get_file_info, + .suffixes = speex_suffixes, + .rewrite_tags = spx_rewrite_tags, +}; diff --git a/wma_afh.c b/wma_afh.c index e6048a86..63e49677 100644 --- a/wma_afh.c +++ b/wma_afh.c @@ -646,13 +646,13 @@ out: static const char * const wma_suffixes[] = {"wma", NULL}; /** - * The init function of the wma audio format handler. + * The audio format handler for Windows Media Audio. * - * \param afh Pointer to the struct to initialize. + * Only WMA version 2 is supported. This audio format handler does not depend + * on any third party libraries and is therefore always compiled in. */ -void wma_afh_init(struct audio_format_handler *afh) -{ - afh->get_file_info = wma_get_file_info; - afh->suffixes = wma_suffixes; - afh->rewrite_tags = wma_rewrite_tags; -} +const struct audio_format_handler wma_afh = { + .get_file_info = wma_get_file_info, + .suffixes = wma_suffixes, + .rewrite_tags = wma_rewrite_tags, +}; -- 2.39.2