Improved Vorbis comment reader. The tags no longer need to fit in the first Ogg page to be fully read (album art can make the tags not fit).

git-svn-id: svn://svn.rockbox.org/rockbox/trunk@21134 a1c6a512-1295-4272-9138-f99709370657

apps/metadata/flac.c

@@ -101,7 +101,7 @@ bool get_flac_metadata(int fd, struct mp3entry* id3)
         else if (type == 4)  /* 4 is the VORBIS_COMMENT block */
         {
             /* The next i bytes of the file contain the VORBIS COMMENTS. */
-            if (!read_vorbis_tags(fd, id3, i))
+            if (read_vorbis_tags(fd, id3, i) == 0)
             {
                 return rc;
             }

apps/metadata/metadata_common.h

@@ -32,7 +32,7 @@
 enum tagtype { TAGTYPE_APE = 1, TAGTYPE_VORBIS };
 
 bool read_ape_tags(int fd, struct mp3entry* id3);
-bool read_vorbis_tags(int fd, struct mp3entry *id3,
+long read_vorbis_tags(int fd, struct mp3entry *id3,
     long tag_remaining);
 
 bool skip_id3v2(int fd, struct mp3entry *id3);

apps/metadata/ogg.c

@@ -115,64 +115,7 @@ bool get_ogg_metadata(int fd, struct mp3entry* id3)
      * one from the last page (since we only support a single bitstream).
      */
     serial = get_long_le(&buf[14]);
-
-    /* Minimum header length for Ogg pages is 27. */
-    if (read(fd, buf, 27) < 27) 
-    {
-        return false;
-    }
-
-    if (memcmp(buf, "OggS", 4) !=0 )
-    {
-        return false;
-    }
-
-    segments = buf[26];
-
-    /* read in segment table */
-    if (read(fd, buf, segments) < segments) 
-    {
-        return false;
-    }
-
-    /* The second packet in a vorbis stream is the comment packet. It *may*
-     * extend beyond the second page, but usually does not. Here we find the
-     * length of the comment packet (or the rest of the page if the comment
-     * packet extends to the third page).
-     */
-    for (i = 0; i < segments; i++) 
-    {
-        remaining += buf[i];
-        
-        /* The last segment of a packet is always < 255 bytes */
-        if (buf[i] < 255) 
-        {
-              break;
-        }
-    }
-
-    comment_size = remaining;
-    
-    if (id3->codectype == AFMT_OGG_VORBIS) {
-        /* Now read in packet header (type and id string) */
-        if (read(fd, buf, 7) < 7) 
-        {
-            return false;
-        }
-
-        remaining -= 7;
-
-        /* The first byte of a packet is the packet type; comment packets are
-         * type 3.
-         */
-        if (buf[0] != 3)
-        {
-            return false;
-        }
-    }
-
-    /* Failure to read the tags isn't fatal. */
-    read_vorbis_tags(fd, id3, remaining);
+    comment_size = read_vorbis_tags(fd, id3, remaining);
 
     /* We now need to search for the last page in the file - identified by 
      * by ('O','g','g','S',0) and retrieve totalsamples.

apps/metadata/vorbis.c

@@ -31,90 +31,335 @@
 #include "structec.h"
 #include "logf.h"
 
-/* Read the items in a Vorbis comment packet. Returns true the items were
- * fully read, false otherwise.
+
+struct file
+{
+    int fd;
+    bool packet_ended;
+    long packet_remaining;
+};
+
+
+/* Read an Ogg page header. file->packet_remaining is set to the size of the
+ * first packet on the page; file->packet_ended is set to true if the packet
+ * ended on the current page. Returns true if the page header was
+ * successfully read.
  */
-bool read_vorbis_tags(int fd, struct mp3entry *id3, 
+static bool file_read_page_header(struct file* file)
+{
+    unsigned char buffer[64];
+    ssize_t table_left;
+
+    /* Size of page header without segment table */
+    if (read(file->fd, buffer, 27) != 27)
+    {
+        return false;
+    }
+        
+    if (memcmp("OggS", buffer, 4))
+    {
+        return false;
+    }
+
+    /* Skip pattern (4), version (1), flags (1), granule position (8),
+     * serial (4), pageno (4), checksum (4)
+     */
+    table_left = buffer[26];
+    file->packet_remaining = 0;
+
+    /* Read segment table for the first packet */
+    do
+    {
+        ssize_t count = MIN(sizeof(buffer), (size_t) table_left);
+        int i;
+
+        if (read(file->fd, buffer, count) < count)
+        {
+            return false;
+        }
+        
+        table_left -= count;
+        
+        for (i = 0; i < count; i++)
+        {
+            file->packet_remaining += buffer[i];
+            
+            if (buffer[i] < 255)
+            {
+                file->packet_ended = true;
+                
+                /* Skip remainder of the table */
+                if (lseek(file->fd, table_left, SEEK_CUR) < 0)
+                {
+                    return false;
+                }
+                
+                table_left = 0;
+                break;
+            }
+        }
+    }
+    while (table_left > 0);
+    
+    return true;
+}
+
+
+/* Read (up to) buffer_size of data from the file. If buffer is NULL, just
+ * skip ahead buffer_size bytes (like lseek). Returns number of bytes read,
+ * 0 if there is no more data to read (in the packet or the file), < 0 if a
+ * read error occurred.
+ */
+static ssize_t file_read(struct file* file, void* buffer, size_t buffer_size)
+{
+    ssize_t done = 0;
+    ssize_t count = -1;
+    
+    do
+    {
+        if (file->packet_remaining <= 0)
+        {
+            if (file->packet_ended)
+            {
+                break;
+            }
+
+            if (!file_read_page_header(file))
+            {
+                count = -1;
+                break;
+            }
+        }
+
+        count = MIN(buffer_size, (size_t) file->packet_remaining);
+
+        if (buffer)
+        {
+            count = read(file->fd, buffer, count);
+        }
+        else
+        {
+            if (lseek(file->fd, count, SEEK_CUR) < 0)
+            {
+                count = -1;
+            }
+        }
+        
+        if (count <= 0)
+        {
+            break;
+        }
+
+        if (buffer)
+        {
+            buffer += count;
+        }
+
+        buffer_size -= count;
+        done += count;
+        file->packet_remaining -= count;
+    }
+    while (buffer_size > 0);
+
+    return (count < 0 ? count : done);
+}
+
+
+/* Read an int32 from file. Returns false if a read error occurred.
+ */
+static bool file_read_int32(struct file* file, int32_t* value)
+{
+    char buf[sizeof(int32_t)];
+
+    if (file_read(file, buf, sizeof(buf)) < (ssize_t) sizeof(buf))
+    {
+        return false;
+    }
+    
+    *value = get_long_le(buf);
+    return true;
+}
+
+
+/* Read a string from the file. Read up to buffer_size bytes, or, if eos
+ * != -1, until the eos character is found (eos is not stored in buf,
+ * unless it is nil). Writes up to buffer_size chars to buf, always
+ * terminating with a nil. Returns number of chars read or < 0 if a read
+ * error occurred.
+ *
+ * Unfortunately this is a slightly modified copy of read_string() in
+ * metadata_common.c...
+ */
+static long file_read_string(struct file* file, char* buffer,
+    long buffer_size, int eos, long size)
+{
+    long read_bytes = 0;
+    
+    while (size > 0)
+    {
+        char c;
+
+        if (file_read(file, &c, 1) != 1)
+        {
+            read_bytes = -1;
+            break;
+        }
+        
+        read_bytes++;
+        size--;
+        
+        if ((eos != -1) && (eos == (unsigned char) c))
+        {
+            break;
+        }
+        
+        if (buffer_size > 1)
+        {
+            *buffer++ = c;
+            buffer_size--;
+        }
+        else if (eos == -1)
+        {
+            /* No point in reading any more, skip remaining data */
+            if (file_read(file, NULL, size) < 0)
+            {
+                read_bytes = -1;
+            }
+            else
+            {
+                read_bytes += size;
+            }
+            
+            break;
+        }
+    }
+    
+    *buffer = 0;
+    return read_bytes;
+}
+
+
+/* Init struct file for reading from fd. type is the AFMT_* codec type of
+ * the file, and determines if Ogg pages are to be read. remaining is the
+ * max amount to read if codec type is FLAC; it is ignored otherwise.
+ * Returns true if the file was successfully initialized.
+ */
+static bool file_init(struct file* file, int fd, int type, int remaining)
+{
+    memset(file, 0, sizeof(*file));
+    file->fd = fd;
+    
+    if (type == AFMT_OGG_VORBIS || type == AFMT_SPEEX)
+    {
+        if (!file_read_page_header(file))
+        {
+            return false;
+        }
+    }
+
+    if (type == AFMT_OGG_VORBIS)
+    {
+        char buffer[7];
+
+        /* Read packet header (type and id string) */
+        if (file_read(file, buffer, sizeof(buffer)) < (ssize_t) sizeof(buffer))
+        {
+            return false;
+        }
+
+        /* The first byte of a packet is the packet type; comment packets
+         * are type 3.
+         */
+        if (buffer[0] != 3)
+        {
+            return false;
+        }
+    }
+    else if (type == AFMT_FLAC)
+    {
+        file->packet_remaining = remaining;
+        file->packet_ended = true;
+    }
+    
+    return true;
+}
+
+
+/* Read the items in a Vorbis comment packet. For Ogg files, the file must
+ * be located on a page start, for other files, the beginning of the comment
+ * data (i.e., the vendor string length). Returns total size of the
+ * comments, or 0 if there was a read error.
+ */
+long read_vorbis_tags(int fd, struct mp3entry *id3, 
     long tag_remaining)
 {
+    struct file file;
     char *buf = id3->id3v2buf;
     int32_t comment_count;
     int32_t len;
+    long comment_size = 0;
     int buf_remaining = sizeof(id3->id3v2buf) + sizeof(id3->id3v1buf);
     int i;
-
-    if (ecread(fd, &len, 1, "l", IS_BIG_ENDIAN) < (long) sizeof(len)) 
+    
+    if (!file_init(&file, fd, id3->codectype, tag_remaining))
     {
-        return false;
+        return 0;
     }
     
-    if ((lseek(fd, len, SEEK_CUR) < 0)
-        || (ecread(fd, &comment_count, 1, "l", IS_BIG_ENDIAN) 
-            < (long) sizeof(comment_count)))
+    /* Skip vendor string */
+
+    if (!file_read_int32(&file, &len) || (file_read(&file, NULL, len) < 0))
     {
-        return false;
+        return 0;
+    }
+
+    if (!file_read_int32(&file, &comment_count))
+    {
+        return 0;
     }
     
-    tag_remaining -= len + sizeof(len) + sizeof(comment_count);
+    comment_size += 4 + len + 4;
 
-    if (tag_remaining <= 0)
-    {
-        return true;
-    }
-
-    for (i = 0; i < comment_count; i++)
+    for (i = 0; i < comment_count && file.packet_remaining > 0; i++)
     {
         char name[TAG_NAME_LENGTH];
-        char value[TAG_VALUE_LENGTH];
         int32_t read_len;
 
-        if (tag_remaining < 4) 
+        if (!file_read_int32(&file, &len))
         {
-            break;
+            return 0;
         }
-
-        if (ecread(fd, &len, 1, "l", IS_BIG_ENDIAN) < (long) sizeof(len))
-        {
-            return false;
-        }
-
-        tag_remaining -= 4;
-
-        /* Quit if we've passed the end of the page */
-        if (tag_remaining < len)
-        {
-            break;
-        }
-
-        tag_remaining -= len;
-        read_len = read_string(fd, name, sizeof(name), '=', len);
+        
+        comment_size += 4 + len;
+        read_len = file_read_string(&file, name, sizeof(name), '=', len);
         
         if (read_len < 0)
         {
-            return false;
+            return 0;
         }
 
         len -= read_len;
 
-        if (read_string(fd, value, sizeof(value), -1, len) < 0)
+        if (file_read_string(&file, id3->path, sizeof(id3->path), -1, len) < 0)
         {
-            return false;
+            return 0;
         }
 
-        len = parse_tag(name, value, id3, buf, buf_remaining, 
+        DEBUGF("Vorbis comment %d: %s=%s\n", i, name, id3->path);
+        len = parse_tag(name, id3->path, id3, buf, buf_remaining, 
             TAGTYPE_VORBIS);
         buf += len;
         buf_remaining -= len;
     }
 
-    /* Skip to the end of the block */
-    if (tag_remaining) 
+    /* Skip to the end of the block (needed by FLAC) */
+    if (file.packet_remaining)
     {
-        if (lseek(fd, tag_remaining, SEEK_CUR) < 0)
+        if (file_read(&file, NULL, file.packet_remaining) < 0)
         {
-            return false;
+            return 0;
         }
     }
 
-    return true;
+    return comment_size;
 }