From 322ef3cb805dacff84aea1e18a840e2bbf8cc881 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 12:57:03 +0200 Subject: mad: skip ID3 frames when libid3tag is disabled When libid3tag is disabled, the libmad decoder plugin is unable to identify ID3 frames. If the file starts with an (unidentified) ID3 frame, it assumes that the file is not a valid MP3 song. This patch solves this by adding minimal stubs for the ID3 functions. --- src/decoder/mad_plugin.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) (limited to 'src/decoder') diff --git a/src/decoder/mad_plugin.c b/src/decoder/mad_plugin.c index c6b9d32d3..1ef7183fa 100644 --- a/src/decoder/mad_plugin.c +++ b/src/decoder/mad_plugin.c @@ -350,11 +350,11 @@ parse_id3_replay_gain_info(struct id3_tag *tag) } #endif -#ifdef HAVE_ID3TAG static void mp3_parse_id3(struct mp3_data *data, size_t tagsize, struct tag **mpd_tag, struct replay_gain_info **replay_gain_info_r) { +#ifdef HAVE_ID3TAG struct id3_tag *id3_tag = NULL; id3_length_t count; id3_byte_t const *id3_data; @@ -418,8 +418,34 @@ static void mp3_parse_id3(struct mp3_data *data, size_t tagsize, id3_tag_delete(id3_tag); g_free(allocated); -} +#else /* !HAVE_ID3TAG */ + (void)mpd_tag; + (void)replay_gain_info_r; + + /* This code is enabled when libid3tag is disabled. Instead + of parsing the ID3 frame, it just skips it. */ + + mad_stream_skip(&data->stream, tagsize); #endif +} + +#ifndef HAVE_ID3TAG +/** + * This function emulates libid3tag when it is disabled. Instead of + * doing a real analyzation of the frame, it just checks whether the + * frame begins with the string "ID3". If so, it returns the full + * length. + */ +static signed long +id3_tag_query(const void *p0, size_t length) +{ + const char *p = p0; + + return length > 3 && memcmp(p, "ID3", 3) == 0 + ? length + : 0; +} +#endif /* !HAVE_ID3TAG */ static enum mp3_action decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, @@ -433,7 +459,6 @@ decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, return DECODE_BREAK; } if (mad_header_decode(&data->frame.header, &data->stream)) { -#ifdef HAVE_ID3TAG if ((data->stream).error == MAD_ERROR_LOSTSYNC && (data->stream).this_frame) { signed long tagsize = id3_tag_query((data->stream). @@ -454,7 +479,6 @@ decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, return DECODE_CONT; } } -#endif if (MAD_RECOVERABLE((data->stream).error)) { return DECODE_SKIP; } else { @@ -493,7 +517,6 @@ decodeNextFrame(struct mp3_data *data) return DECODE_BREAK; } if (mad_frame_decode(&data->frame, &data->stream)) { -#ifdef HAVE_ID3TAG if ((data->stream).error == MAD_ERROR_LOSTSYNC) { signed long tagsize = id3_tag_query((data->stream). this_frame, @@ -506,7 +529,6 @@ decodeNextFrame(struct mp3_data *data) return DECODE_CONT; } } -#endif if (MAD_RECOVERABLE((data->stream).error)) { return DECODE_SKIP; } else { -- cgit v1.2.3 From 8e2d9879962775bb29aa4d7556666b2216353bbe Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:31:43 +0200 Subject: decoder/flac: removed misplaced authorship comment This belongs into "git annotate" or AUTHORS. --- src/decoder/_flac_common.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/decoder') diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index 713dfe9b2..a6c3a2134 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -67,7 +67,6 @@ flac_find_float_comment(const FLAC__StreamMetadata *block, return false; } -/* replaygain stuff by AliasMrJones */ static void flac_parse_replay_gain(const FLAC__StreamMetadata *block, struct flac_data *data) -- cgit v1.2.3 From cf1fd2b0da4be4675ddb16c5421d949a0e923b20 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:31:46 +0200 Subject: decoder/flac: return early from flac_find_float_comment() When one metadata check fails, return quickly. This removes 2 levels of indent. --- src/decoder/_flac_common.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) (limited to 'src/decoder') diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index a6c3a2134..a450b6886 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -44,27 +44,28 @@ static bool flac_find_float_comment(const FLAC__StreamMetadata *block, const char *cmnt, float *fl) { - int offset = - FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, cmnt); - - if (offset >= 0) { - size_t pos = strlen(cmnt) + 1; /* 1 is for '=' */ - int len = block->data.vorbis_comment.comments[offset].length - - pos; - if (len > 0) { - unsigned char tmp; - unsigned char *p = &(block->data.vorbis_comment. - comments[offset].entry[pos]); - tmp = p[len]; - p[len] = '\0'; - *fl = (float)atof((char *)p); - p[len] = tmp; - - return true; - } - } - - return false; + int offset; + size_t pos; + int len; + unsigned char tmp, *p; + + offset = FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, + cmnt); + if (offset < 0) + return false; + + pos = strlen(cmnt) + 1; /* 1 is for '=' */ + len = block->data.vorbis_comment.comments[offset].length - pos; + if (len <= 0) + return false; + + p = &block->data.vorbis_comment.comments[offset].entry[pos]; + tmp = p[len]; + p[len] = '\0'; + *fl = (float)atof((char *)p); + p[len] = tmp; + + return true; } static void -- cgit v1.2.3 From 47ed89bd4c6499d475d5f16cb89d7be95763670c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:31:48 +0200 Subject: decoder/flac: parse all replaygain tags The FLAC replaygain parser used the "||" operator. This made the code stop after the first value which was found. --- src/decoder/_flac_common.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) (limited to 'src/decoder') diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index a450b6886..9e2d9f437 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -40,9 +40,9 @@ flac_data_init(struct flac_data *data, struct decoder * decoder, data->tag = NULL; } -static bool +static void flac_find_float_comment(const FLAC__StreamMetadata *block, - const char *cmnt, float *fl) + const char *cmnt, float *fl, bool *found_r) { int offset; size_t pos; @@ -52,12 +52,12 @@ flac_find_float_comment(const FLAC__StreamMetadata *block, offset = FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, cmnt); if (offset < 0) - return false; + return; pos = strlen(cmnt) + 1; /* 1 is for '=' */ len = block->data.vorbis_comment.comments[offset].length - pos; if (len <= 0) - return false; + return; p = &block->data.vorbis_comment.comments[offset].entry[pos]; tmp = p[len]; @@ -65,28 +65,32 @@ flac_find_float_comment(const FLAC__StreamMetadata *block, *fl = (float)atof((char *)p); p[len] = tmp; - return true; + *found_r = true; } static void flac_parse_replay_gain(const FLAC__StreamMetadata *block, struct flac_data *data) { - bool found; + bool found = false; if (data->replay_gain_info) replay_gain_info_free(data->replay_gain_info); data->replay_gain_info = replay_gain_info_new(); - found = flac_find_float_comment(block, "replaygain_album_gain", - &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].gain) || - flac_find_float_comment(block, "replaygain_album_peak", - &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].peak) || - flac_find_float_comment(block, "replaygain_track_gain", - &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].gain) || - flac_find_float_comment(block, "replaygain_track_peak", - &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].peak); + flac_find_float_comment(block, "replaygain_album_gain", + &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].gain, + &found); + flac_find_float_comment(block, "replaygain_album_peak", + &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].peak, + &found); + flac_find_float_comment(block, "replaygain_track_gain", + &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].gain, + &found); + flac_find_float_comment(block, "replaygain_track_peak", + &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].peak, + &found); if (!found) { replay_gain_info_free(data->replay_gain_info); -- cgit v1.2.3 From f4b39bc263b46685742ad120ac9a02d4ba022532 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:34:33 +0200 Subject: decoder/flac: fixed indentation of flac_comment_value() --- src/decoder/_flac_common.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'src/decoder') diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index 9e2d9f437..ae7d039ce 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -110,25 +110,27 @@ flac_comment_value(const FLAC__StreamMetadata_VorbisComment_Entry *entry, size_t char_tnum_length = 0; const char *comment = (const char*)entry->entry; - if (entry->length > name_length && - g_ascii_strncasecmp(comment, name, name_length) == 0) { - if (char_tnum != NULL) { - char_tnum_length = strlen(char_tnum); - if (entry->length > name_length + char_tnum_length + 2 && - comment[name_length] == '[' && - g_ascii_strncasecmp(comment + name_length + 1, - char_tnum, char_tnum_length) == 0 && - comment[name_length + char_tnum_length + 1] == ']') - name_length = name_length + char_tnum_length + 2; - else if (entry->length > name_length + char_tnum_length && - g_ascii_strncasecmp(comment + name_length, - char_tnum, char_tnum_length) == 0) - name_length = name_length + char_tnum_length; - } - if (comment[name_length] == '=') { - *length_r = entry->length - name_length - 1; - return comment + name_length + 1; - } + if (entry->length <= name_length || + g_ascii_strncasecmp(comment, name, name_length) != 0) + return NULL; + + if (char_tnum != NULL) { + char_tnum_length = strlen(char_tnum); + if (entry->length > name_length + char_tnum_length + 2 && + comment[name_length] == '[' && + g_ascii_strncasecmp(comment + name_length + 1, + char_tnum, char_tnum_length) == 0 && + comment[name_length + char_tnum_length + 1] == ']') + name_length = name_length + char_tnum_length + 2; + else if (entry->length > name_length + char_tnum_length && + g_ascii_strncasecmp(comment + name_length, + char_tnum, char_tnum_length) == 0) + name_length = name_length + char_tnum_length; + } + + if (comment[name_length] == '=') { + *length_r = entry->length - name_length - 1; + return comment + name_length + 1; } return NULL; -- cgit v1.2.3 From 1c4f407a6db4c4795bbbc354f5cf311762fb8e33 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:51:42 +0200 Subject: decoder/flac: don't allocate cuesheet twice (memleak) The function flac_cue_track() first calls FLAC__metadata_object_new(), then overwrites this pointer with FLAC__metadata_get_cuesheet(). This allocate two FLAC__StreamMetadata objects, but the first pointer is lost, and never freed. --- src/decoder/_flac_common.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src/decoder') diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index ae7d039ce..e096750f3 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -377,13 +377,15 @@ char* flac_cue_track( const char* pathname, const unsigned int tnum) { - FLAC__StreamMetadata* cs = FLAC__metadata_object_new(FLAC__METADATA_TYPE_CUESHEET); + FLAC__bool success; + FLAC__StreamMetadata* cs; - FLAC__metadata_get_cuesheet(pathname, &cs); - - if (cs == NULL) + success = FLAC__metadata_get_cuesheet(pathname, &cs); + if (!success) return NULL; + assert(cs != NULL); + if (cs->data.cue_sheet.num_tracks <= 1) { FLAC__metadata_object_delete(cs); -- cgit v1.2.3