[Rhythmbox-devel] more mp3 handling fixes

[Bastien Nocera <hadess hadess net>]

Heya,

It didn't in any one bugzilla anymore, so I'm posting it here.

- Special-cases wave files (doesn't go as deep if they're waves, to
avoid false positives, bz # 124298)
- Discard empty tag fields, fixes metadata reading from emusic.com files
(bz # 121841)
- implement TLEN (Track Length) tag from id3v2

Please test and apply. Don't forget to close the bugs when done ;)

Cheers

---
Bastien Nocera <hadess@hadess.net> 
Shots rang out, as shots are wont to do. 
? foo
Index: stream-info-impl/mp3-stream-info-impl.c
===================================================================
RCS file: /cvs/gnome/rhythmbox/monkey-media/stream-info-impl/mp3-stream-info-impl.c,v
retrieving revision 1.8
diff -u -r1.8 mp3-stream-info-impl.c
--- stream-info-impl/mp3-stream-info-impl.c	16 Nov 2003 16:56:43 -0000	1.8
+++ stream-info-impl/mp3-stream-info-impl.c	16 Nov 2003 23:34:56 -0000
@@ -42,6 +42,7 @@
 static void MP3_stream_info_impl_init (MP3StreamInfoImpl *ma);
 static void MP3_stream_info_impl_finalize (GObject *object);
 static void MP3_stream_info_impl_open_stream (MonkeyMediaStreamInfo *info);
+static gboolean MP3_stream_info_impl_get_bitrate_info (MP3StreamInfoImpl *impl);
 static gboolean MP3_stream_info_impl_get_value (MonkeyMediaStreamInfo *info,
 					        MonkeyMediaStreamInfoField field,
 					        int index,
@@ -169,25 +170,83 @@
 	}
 
 	impl->priv->tag = id3_vfs_tag (impl->priv->file);
+
+	if (MP3_stream_info_impl_get_bitrate_info (impl) == FALSE)
+	{
+		error = g_error_new (MONKEY_MEDIA_STREAM_INFO_ERROR,
+				     MONKEY_MEDIA_STREAM_INFO_ERROR_OPEN_FAILED,
+				     _("Failed to gather information about the file"));
+		g_object_set (G_OBJECT (info), "error", error, NULL);
+		return;
+	}
 }
 
 static void
-MP3_stream_info_impl_get_bitrate_info (MP3StreamInfoImpl *info)
+MP3_stream_info_impl_get_length_from_tag (MP3StreamInfoImpl *impl)
+{
+	struct id3_frame const *frame;
+
+	/* The following is based on information from the
+	 * ID3 tag version 2.4.0 Native Frames informal standard.
+	 */
+	frame = id3_tag_findframe(impl->priv->tag, "TLEN", 0);
+
+	if (frame == NULL)
+		return;
+
+	{
+		union id3_field const *field;
+		unsigned int nstrings;
+		id3_latin1_t *latin1;
+		signed long ms;
+
+		field = id3_frame_field (frame, 1);
+		nstrings = id3_field_getnstrings (field);
+		if (nstrings <= 0)
+			return;
+
+		latin1 = id3_ucs4_latin1duplicate
+			(id3_field_getstrings(field, 0));
+
+		if (latin1 == NULL)
+			return;
+
+		/* "The 'Length' frame contains the length of the
+		 * audio file in milliseconds, represented as a
+		 * numeric string."
+		 */
+		if (atol(latin1) > 0)
+			impl->priv->info_num->time = atol(latin1);
+		g_free (latin1);
+	}
+}
+
+static gboolean
+MP3_stream_info_impl_get_bitrate_info (MP3StreamInfoImpl *impl)
 {
-	if (info->priv->info_num == NULL)
+	if (impl->priv->info_num == NULL)
 	{
-		struct MP3BitrateInfo *info_num;
+		struct MP3BitrateInfo *info;
+
+		info = g_new0 (struct MP3BitrateInfo, 1);
+		if (id3_vfs_bitrate (impl->priv->file,
+				&info->bitrate,
+				&info->samplerate,
+				&info->time,
+				&info->version,
+				&info->vbr,
+				&info->channels) == 0)
+		{
+			impl->priv->info_num = info;
+			return FALSE;
+		}
+
+		impl->priv->info_num = info;
 
-		info_num = g_new0 (struct MP3BitrateInfo, 1);
-		id3_vfs_bitrate (info->priv->file,
-				&info_num->bitrate,
-				&info_num->samplerate,
-				&info_num->time,
-				&info_num->version,
-				&info_num->vbr,
-				&info_num->channels);
-		info->priv->info_num = info_num;
+		MP3_stream_info_impl_get_length_from_tag (impl);
 	}
+
+	return TRUE;
 }
 
 static int
@@ -469,7 +528,6 @@
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_DURATION:
 		g_value_init (value, G_TYPE_LONG);
-		MP3_stream_info_impl_get_bitrate_info (impl);
 
 		if (impl->priv->info_num->vbr == 0)
 		{
@@ -498,7 +556,6 @@
 		g_value_set_boolean (value, TRUE);
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_CODEC_INFO:
-		MP3_stream_info_impl_get_bitrate_info (impl);
 		g_value_init (value, G_TYPE_STRING);
 		if (impl->priv->info_num->version != 3)
 		{
@@ -511,12 +568,10 @@
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_BIT_RATE:
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_AVERAGE_BIT_RATE:
-		MP3_stream_info_impl_get_bitrate_info (impl);
 		g_value_init (value, G_TYPE_INT);
 		g_value_set_int (value, impl->priv->info_num->bitrate / 1000);
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_QUALITY:
-		MP3_stream_info_impl_get_bitrate_info (impl);
 		g_value_init (value, MONKEY_MEDIA_TYPE_AUDIO_QUALITY);
 		g_value_set_enum (value, monkey_media_audio_quality_from_bit_rate (impl->priv->info_num->bitrate / 1000));
 		break;
@@ -526,17 +581,14 @@
 		g_value_set_string (value, NULL);
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_VARIABLE_BIT_RATE:
-		MP3_stream_info_impl_get_bitrate_info (impl);
 		g_value_init (value, G_TYPE_BOOLEAN);
 		g_value_set_boolean (value, impl->priv->info_num->vbr);
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_SAMPLE_RATE:
-		MP3_stream_info_impl_get_bitrate_info (impl);
 		g_value_init (value, G_TYPE_LONG);
 		g_value_set_long (value, impl->priv->info_num->samplerate);
 		break;
 	case MONKEY_MEDIA_STREAM_INFO_FIELD_AUDIO_CHANNELS:
-		MP3_stream_info_impl_get_bitrate_info (impl);
 		g_value_init (value, G_TYPE_INT);
 		g_value_set_int (value, impl->priv->info_num->channels);
 		break;
@@ -595,16 +647,22 @@
 	if (frame == 0)
 		return NULL;
 
-	field = &frame->fields[1];
+	field = id3_frame_field(frame, 1);
 	nstrings = id3_field_getnstrings (field);
 	for (j = 0; j < nstrings; j++)
 	{
+		id3_utf8_t *tmp = NULL;
+
 		ucs4 = id3_field_getstrings (field, j);
 
 		if (strcmp (field_name, ID3_FRAME_GENRE) == 0)
 			ucs4 = id3_genre_name (ucs4);
 
-		utf8 = id3_ucs4_utf8duplicate (ucs4);
+		tmp = id3_ucs4_utf8duplicate (ucs4);
+		if (strcmp (tmp, "") != 0)
+			utf8 = tmp;
+		else
+			g_free (tmp);
 	}
 
 	if (utf8 && !g_utf8_validate (utf8, -1, NULL)) {
Index: stream-info-impl/id3-vfs/id3-vfs.c
===================================================================
RCS file: /cvs/gnome/rhythmbox/monkey-media/stream-info-impl/id3-vfs/id3-vfs.c,v
retrieving revision 1.5
diff -u -r1.5 id3-vfs.c
--- stream-info-impl/id3-vfs/id3-vfs.c	16 Nov 2003 17:59:09 -0000	1.5
+++ stream-info-impl/id3-vfs/id3-vfs.c	16 Nov 2003 23:34:57 -0000
@@ -458,6 +458,21 @@
   return 0;
 }
 
+static int
+id3_vfs_is_wave (guchar *buffer)
+{
+  if (buffer[8] != 'W')
+    return 0;
+  if (buffer[9] != 'A')
+    return 0;
+  if (buffer[10] != 'V')
+    return 0;
+  if (buffer[11] != 'E' && buffer[11] != ' ')
+    return 0;
+
+  return 1;
+}
+
 int
 id3_vfs_bitrate (struct id3_vfs_file *file, int *bitrate, int *samplerate,
 		int *time, int *version, int *vbr, int *channels)
@@ -466,7 +481,7 @@
   GnomeVFSHandle *iofile = file->iofile;
   GnomeVFSResult res;
   guchar buffer[16384];
-  int found, i;
+  int is_wave, found, i;
 
   *bitrate = 0;
   *samplerate = 0;
@@ -485,6 +500,11 @@
   if( res != GNOME_VFS_OK || length_read < 512 )
     goto bitdone;
 
+  /* Reduce false positive by castrating the search if we have a WAVE file */
+  is_wave = id3_vfs_is_wave (buffer);
+  if (is_wave == 1)
+    length_read = 4096;
+
   for (i = 0; i + 4 < length_read; i++)
   {
     if (mp3_bitrate_parse_header (buffer+i, length_read - i, bitrate, samplerate, time, version, vbr, channels))
@@ -495,7 +515,7 @@
   }
 
   /* If we haven't found anything, try again with 8 more kB */
-  if (found == 0)
+  if (is_wave == 0 && found == 0)
   {
     res = gnome_vfs_read (iofile, buffer, sizeof (buffer), &length_read);
 
@@ -516,6 +536,6 @@
   if (gnome_vfs_seek(iofile, GNOME_VFS_SEEK_START, save_position) != GNOME_VFS_OK)
     return 0;
 
-  return 1;
+  return found;
 }