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.

diff --git a/afh_common.c b/afh_common.c
index 063ae8f0b104ccab15e9f10f637ec34e7783e785..f36c32e20ec6332a24720d9bf106ab9b39d756ba 100644 (file)
--- 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 382b0e9091c4cb3d6679f9779ad6faff9fb1fe15..70f2ec9f6b3817887799edab3292f7136aec0a84 100644 (file)
--- 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;
 }