From: Andre Noll Date: Fri, 23 Jan 2015 12:17:38 +0000 (+0100) Subject: Merge branch 'refs/heads/t/afh_improvements' X-Git-Tag: v0.5.4~1 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=a7967902aa937e91d35767b3eba2b77343e8b822;hp=56d75bd90d78cf44cd3984ce2a45627ef5646d38 Merge branch 'refs/heads/t/afh_improvements' Was cooking for about a week. mp3_afh: Detect both v1 and v2 tags. ogg: Improve documentation of struct ogg_afh_callback_info. ogg_afh: Add a comment about the three vorbis header packets. afh_common.c: Avoid ifdefs. afh loglevel adjustments. wma: Rename comment_header. afh: Fix fd leak. afh: Unmap the audio file on errors. --- diff --git a/NEWS b/NEWS index b9d032fc..ddd70a13 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,7 @@ Another cleanup and bugfix release. - audiod memory leak fixes. - Miscellaneous improvements to the build system. - oss_writer improvements. + - Improved handling of mp3 files with both id3v1 and id3v2 tags. Download: ./releases/paraslash-git.tar.bz2 (tarball) diff --git a/afh.c b/afh.c index d06e4420..f3c25a26 100644 --- a/afh.c +++ b/afh.c @@ -102,16 +102,16 @@ int main(int argc, char **argv) } ret = compute_afhi(conf.inputs[i], audio_file_data, audio_file_size, fd, &afhi); - if (ret < 0) - goto out; - - audio_format_num = ret; - printf("File %d: %s\n", i + 1, conf.inputs[i]); - print_info(audio_format_num, &afhi); - if (conf.chunk_table_given) - print_chunk_table(&afhi); - printf("\n"); - clear_afhi(&afhi); + if (ret >= 0) { + audio_format_num = ret; + printf("File %d: %s\n", i + 1, conf.inputs[i]); + print_info(audio_format_num, &afhi); + if (conf.chunk_table_given) + print_chunk_table(&afhi); + printf("\n"); + clear_afhi(&afhi); + } + close(fd); ret2 = para_munmap(audio_file_data, audio_file_size); if (ret2 < 0 && ret >= 0) ret = ret2; diff --git a/afh_common.c b/afh_common.c index 914d1a32..edfc8d1d 100644 --- a/afh_common.c +++ b/afh_common.c @@ -15,27 +15,10 @@ #include "string.h" #include "afh.h" -/* The mp3 audio format handler does not need any libs. */ -void mp3_init(struct audio_format_handler *); - -#ifdef HAVE_OGGVORBIS - void ogg_init(struct audio_format_handler *); -#endif -#ifdef HAVE_FAAD - void aac_afh_init(struct audio_format_handler *); -#endif -#ifdef HAVE_SPEEX - void spx_afh_init(struct audio_format_handler *); -#endif -#ifdef HAVE_FLAC - void flac_afh_init(struct audio_format_handler *); -#endif - -#ifdef HAVE_OPUS - void opus_afh_init(struct audio_format_handler *); -#endif - -void wma_afh_init(struct audio_format_handler *); +typedef void afh_init_func(struct audio_format_handler *); +/* It does not hurt to declare init functions which are not available. */ +extern afh_init_func mp3_init, ogg_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_ITEM_ARRAY}; @@ -118,9 +101,9 @@ void afh_init(void) { int i; - PARA_INFO_LOG("supported audio formats: %s\n", AUDIO_FORMAT_HANDLERS); + PARA_NOTICE_LOG("supported audio formats: %s\n", AUDIO_FORMAT_HANDLERS); FOR_EACH_AUDIO_FORMAT(i) { - PARA_NOTICE_LOG("initializing %s handler\n", + PARA_INFO_LOG("initializing %s handler\n", audio_format_name(i)); afl[i].init(&afl[i]); } diff --git a/mp3_afh.c b/mp3_afh.c index cc3f49ff..3b540c0b 100644 --- a/mp3_afh.c +++ b/mp3_afh.c @@ -123,20 +123,11 @@ static char *get_strings(struct id3_frame *fr) return NULL; } -static void mp3_get_id3(__a_unused unsigned char *map, - __a_unused size_t numbytes, int fd, struct taginfo *tags) +/* this only sets values which are undefined so far */ +static void parse_frames(struct id3_tag *id3_t, struct taginfo *tags) { int i; - struct id3_tag *id3_t; - struct id3_file *id3_f = id3_file_fdopen(fd, ID3_FILE_MODE_READONLY); - - if (!id3_f) - return; - id3_t = id3_file_tag(id3_f); - if (!id3_t) { - id3_file_close(id3_f); - return; - } + for (i = 0; i < id3_t->nframes; i++) { struct id3_frame *fr = id3_t->frames[i]; if (!strcmp(fr->id, ID3_FRAME_TITLE)) { @@ -165,7 +156,31 @@ static void mp3_get_id3(__a_unused unsigned char *map, continue; } } - id3_file_close(id3_f); +} + +static int mp3_get_id3(unsigned char *map, size_t numbytes, __a_unused int fd, + struct taginfo *tags) +{ + int ret = 0; + struct id3_tag *id3_t; + + /* id3v2 tags are usually located at the beginning. */ + id3_t = id3_tag_parse(map, numbytes); + if (id3_t) { + parse_frames(id3_t, tags); + ret |= 2; + id3_tag_delete(id3_t); + } + /* Also look for an id3v1 tag at the end of the file. */ + if (numbytes >= 128) { + id3_t = id3_tag_parse(map + numbytes - 128, 128); + if (id3_t) { + parse_frames(id3_t, tags); + ret |= 1; + id3_tag_delete(id3_t); + } + } + return ret; } #else /* HAVE_LIBID3TAG */ @@ -181,7 +196,7 @@ static char *unpad(char *string) return string; } -static void mp3_get_id3(unsigned char *map, size_t numbytes, __a_unused int fd, +static int mp3_get_id3(unsigned char *map, size_t numbytes, __a_unused int fd, struct taginfo *tags) { char title[31], artist[31], album[31], year[5], comment[31]; @@ -189,7 +204,7 @@ static void mp3_get_id3(unsigned char *map, size_t numbytes, __a_unused int fd, if (numbytes < 128 || strncmp("TAG", (char *)map + numbytes - 128, 3)) { PARA_DEBUG_LOG("no id3 v1 tag\n"); - return; + return 0; } fpos = numbytes - 125; memcpy(title, map + fpos, 30); @@ -216,6 +231,7 @@ static void mp3_get_id3(unsigned char *map, size_t numbytes, __a_unused int fd, tags->year = para_strdup(year); tags->album = para_strdup(album); tags->comment = para_strdup(comment); + return 1; } #endif /* HAVE_LIBID3TAG */ @@ -387,6 +403,7 @@ static int mp3_read_info(unsigned char *map, size_t numbytes, int fd, unsigned chunk_table_size = 1000; /* gets increased on demand */ off_t fpos = 0; struct mp3header header; + const char *tag_versions[] = {"no", "id3v1", "id3v2", "id3v1+id3v2"}; afhi->chunks_total = 0; afhi->chunk_table = para_malloc(chunk_table_size * sizeof(uint32_t)); @@ -445,9 +462,9 @@ static int mp3_read_info(unsigned char *map, size_t numbytes, int fd, tv_divide(afhi->chunks_total, &total_time, &afhi->chunk_tv); PARA_DEBUG_LOG("%lu chunks, each %lums\n", afhi->chunks_total, tv2ms(&afhi->chunk_tv)); - afhi->techinfo = make_message("%cbr, %s", vbr? 'v' : 'c', - header_mode(&header)); - mp3_get_id3(map, numbytes, fd, &afhi->tags); + ret = mp3_get_id3(map, numbytes, fd, &afhi->tags); + afhi->techinfo = make_message("%cbr, %s, %s tags", vbr? 'v' : 'c', + header_mode(&header), tag_versions[ret]); return 1; err_out: PARA_ERROR_LOG("%s\n", para_strerror(-ret)); diff --git a/ogg_afh.c b/ogg_afh.c index 1e389d9e..9dfb028d 100644 --- a/ogg_afh.c +++ b/ogg_afh.c @@ -20,6 +20,20 @@ struct private_vorbis_data { vorbis_comment vc; }; +/* + * Vorbis uses three header packets, all of which are required: the + * identification header, the comments header, and the setup header. + * + * The identification header identifies the bitstream as Vorbis. It contains + * the Vorbis version and simple audio characteristics of the stream such as + * sample rate and number of channels. + * + * The comment header includes user text comments (tags) and a vendor string + * for the application/library that produced the bitstream. + * + * The setup header includes extensive CODEC setup information as well as the + * complete VQ and Huffman codebooks needed for decoding. + */ static int vorbis_packet_callback(ogg_packet *packet, int packet_num, __a_unused int serial, struct afh_info *afhi, void *private_data) { diff --git a/ogg_afh_common.h b/ogg_afh_common.h index 09870a99..47e133bf 100644 --- a/ogg_afh_common.h +++ b/ogg_afh_common.h @@ -10,22 +10,26 @@ */ /** - * Callback structure provided by vorbis/speex audio format handlers. + * Callback structure provided by audio format handlers. * - * Both audio formats utilize the ogg container format. Meta info about - * the audio file is contained in the first three ogg packets. + * All audio formats which utilize the ogg container format define a callback + * function whose purpose is to extract the meta information about the audio + * file from the first few ogg packets of the bitstream. */ struct ogg_afh_callback_info { /** - * ogg_get_file_info() calls this function for each of the three - * header packets. If this callback returns a negative value, the - * audio file is considered invalid and the chunk table is not - * created. If it returns zero, the end of the header has been - * reached and no further ogg packets should be processed. - */ + * ogg_get_file_info() calls this function for the first three ogg + * packets, or until the callback returns a non-positive value. If it + * returns negative, the audio file was not recognized by the audio + * format handler. If it returns zero, the audio file is considered + * valid, and no further processing by the callback is needed. In this + * case the header chunk, i.e. chunk number zero, is defined as the + * concatenation of all packets that have been processed by the + * callback. + */ int (*packet_callback)(ogg_packet *packet, int packet_num, int serial, struct afh_info *afhi, void *private_data); - /** Vorbis/speex specific data. */ + /** Data specific to the audio format handler. */ void *private_data; }; diff --git a/wma_afh.c b/wma_afh.c index 3df9b5fb..f1edacf0 100644 --- a/wma_afh.c +++ b/wma_afh.c @@ -100,7 +100,7 @@ static char *get_str16(const char *in, int len) return out; } -static const char comment_header[] = { +static const char content_description_header[] = { 0x33, 0x26, 0xb2, 0x75, 0x8E, 0x66, 0xCF, 0x11, 0xa6, 0xd9, 0x00, 0xaa, 0x00, 0x62, 0xce, 0x6c }; @@ -127,10 +127,10 @@ static void read_asf_tags(const char *buf, int buf_size, struct taginfo *ti) const char *p, *end = buf + buf_size, *q; uint16_t len1, len2, len3, len4; - p = search_pattern(comment_header, sizeof(comment_header), - buf, buf_size); + p = search_pattern(content_description_header, + sizeof(content_description_header), buf, buf_size); if (!p || p + 34 >= end) { - PARA_NOTICE_LOG("comment header not found\n"); + PARA_NOTICE_LOG("content description header not found\n"); goto next; } p += 24;