From faa0b42a726e96263ae3a1c7e89ce9b014fea44d Mon Sep 17 00:00:00 2001
From: Andre Noll <maan@tuebingen.mpg.de>
Date: Sun, 8 May 2016 11:56:23 +0200
Subject: [PATCH] afh: Improve error diagnostics.

If compute_afhi() can not figure out the type of an audio file, it
prints a rather incomprehensive error message for each audiod format
which was tried to no avail. This commit improves the readability of
these error messages by including the path and the name of the audio
format that caused the error.

Before:

	$ para_afh /etc/resolv.conf
	mp3_read_info: could not read mp3 info
	compute_afhi: could not read mp3 info
	compute_afhi: ogg sync page-out error (no ogg file?)
	compute_afhi: mp4v2 library error
	compute_afhi: asf/wma format not recognized
	compute_afhi: ogg sync page-out error (no ogg file?)
	compute_afhi: could not read meta chain
	compute_afhi: ogg sync page-out error (no ogg file?)
	main: audio format not recognized

After:

	$ para_afh /etc/resolv.conf
	get_file_info: /etc/resolv.conf: mp3 format not detected: could not read mp3 info
	get_file_info: /etc/resolv.conf: ogg format not detected: ogg sync page-out error (no ogg file?)
	get_file_info: /etc/resolv.conf: aac format not detected: did not find esds atom
	get_file_info: /etc/resolv.conf: wma format not detected: asf/wma format not recognized
	get_file_info: /etc/resolv.conf: spx format not detected: ogg sync page-out error (no ogg file?)
	get_file_info: /etc/resolv.conf: flac format not detected: could not read meta chain
	get_file_info: /etc/resolv.conf: opus format not detected: ogg sync page-out error (no ogg file?)
	main: audio format not recognized

The patch also removes a call to PARA_ERROR_LOG() in the mp3 audio
format handler which is unnecessary because we return the error code
and print the message in the caller anyway.

A new helper, get_file_info(), is introduced to print the diagnostic
messages. Since audio_format_name() is called from this helper,
that function needed to be moved up to avoid a forward declaration.
---
 afh_common.c | 68 +++++++++++++++++++++++++++-------------------------
 mp3_afh.c    |  1 -
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/afh_common.c b/afh_common.c
index 063ae8f0..f36c32e2 100644
--- a/afh_common.c
+++ b/afh_common.c
@@ -139,6 +139,38 @@ int guess_audio_format(const char *name)
 	return -E_AUDIO_FORMAT;
 }
 
+/**
+ * Get the name of the given audio format.
+ *
+ * \param i The audio format number.
+ *
+ * \return This returns a pointer to statically allocated memory so it
+ * must not be freed by the caller.
+ */
+const char *audio_format_name(int i)
+{
+	if (i < 0 || i >= ARRAY_SIZE(afl) - 1)
+		return "???";
+	return afl[i].name;
+}
+
+static int get_file_info(int format, const char *path, char *data,
+		size_t size, int fd, struct afh_info *afhi)
+{
+	int ret;
+	const char *fmt = audio_format_name(format);
+
+	memset(afhi, 0, sizeof(*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));
+		return ret;
+	}
+	PARA_NOTICE_LOG("%s: detected %s format\n", path, fmt);
+	return format;
+}
+
 /**
  * Call get_file_info() to obtain an afhi structure.
  *
@@ -163,31 +195,18 @@ int compute_afhi(const char *path, char *data, size_t size, int fd,
 {
 	int ret, i, format;
 
-	afhi->header_len = 0;
-	afhi->techinfo = NULL;
-	afhi->tags.artist = NULL;
-	afhi->tags.title = NULL;
-	afhi->tags.year = NULL;
-	afhi->tags.album = NULL;
-	afhi->tags.comment = NULL;
 	format = guess_audio_format(path);
-
 	if (format >= 0) {
-		ret = afl[format].get_file_info(data, size, fd, afhi);
-		if (ret >= 0) {
-			ret = format;
+		ret = get_file_info(format, path, data, size, fd, afhi);
+		if (ret >= 0)
 			goto success;
-		}
 	}
 	FOR_EACH_AUDIO_FORMAT(i) {
 		if (i == format) /* we already tried this one to no avail */
 			continue;
-		ret = afl[i].get_file_info(data, size, fd, afhi);
-		if (ret >= 0) {
-			ret = i;
+		ret = get_file_info(i, path, data, size, fd, afhi);
+		if (ret >= 0)
 			goto success;
-		}
-		PARA_WARNING_LOG("%s\n", para_strerror(-ret));
 	}
 	return -E_AUDIO_FORMAT;
 success:
@@ -233,21 +252,6 @@ void clear_afhi(struct afh_info *afhi)
 	free(afhi->tags.comment);
 }
 
-/**
- * Get the name of the given audio format.
- *
- * \param i The audio format number.
- *
- * \return This returns a pointer to statically allocated memory so it
- * must not be freed by the caller.
- */
-const char *audio_format_name(int i)
-{
-	if (i < 0 || i >= ARRAY_SIZE(afl) - 1)
-		return "???";
-	return afl[i].name;
-}
-
 static inline size_t get_chunk_len(long unsigned chunk_num,
 		const struct afh_info *afhi)
 {
diff --git a/mp3_afh.c b/mp3_afh.c
index 382b0e90..70f2ec9f 100644
--- a/mp3_afh.c
+++ b/mp3_afh.c
@@ -662,7 +662,6 @@ static int mp3_read_info(unsigned char *map, size_t numbytes, int fd,
 		header_mode(&header), tag_versions[ret]);
 	return 1;
 err_out:
-	PARA_ERROR_LOG("%s\n", para_strerror(-ret));
 	free(afhi->chunk_table);
 	return ret;
 }
-- 
2.39.5