From 229f63f8c8ba61569afdc5440db4cf02caa45141 Mon Sep 17 00:00:00 2001
From: Andre Noll <maan@systemlinux.org>
Date: Fri, 27 Dec 2013 03:14:57 +0000
Subject: [PATCH] mp3_afh: Detect both v1 and v2 tags.

Currently the mp3 audio format handler first looks at id3v2 tags and
ignores any id3v1 tags if a version 2 tag is present. This patch makes
it look at both types of tags, and combine the result if necessary.

We use the opportunity to also get rid of the file descriptor mess
in mp3_get_id3(). It is completely unnecessary as lib3d3tag can make
a id3_t structure from a memory buffer. We simply pass (a part of)
the memory map of the audio file and are done.

The techinfo field of struct audio_format_handler_info is changed
to show which tag versions the various tags were extracted. Possible
values are none, v1, v2 and v1+v2.
---
 mp3_afh.c | 65 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 29 deletions(-)

diff --git a/mp3_afh.c b/mp3_afh.c
index 2506b95e..3b540c0b 100644
--- a/mp3_afh.c
+++ b/mp3_afh.c
@@ -123,30 +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, new_fd;
-	struct id3_tag *id3_t;
-	struct id3_file *id3_f;
-
-	/*
-	 * We are not supposed to close fd, but to avoid memory leaks we must
-	 * call id3_file_close() on the id3_file after we are done. As
-	 * id3_file_close() closes fd, we first create a copy for libid3tag.
-	 */
-	new_fd = dup(fd);
-	if (new_fd < 0)
-		return;
-	id3_f = id3_file_fdopen(new_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;
-	}
+	int i;
+
 	for (i = 0; i < id3_t->nframes; i++) {
 		struct id3_frame *fr = id3_t->frames[i];
 		if (!strcmp(fr->id, ID3_FRAME_TITLE)) {
@@ -175,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 */
@@ -191,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];
@@ -199,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);
@@ -226,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 */
 
@@ -397,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));
@@ -455,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));
-- 
2.39.5