Better generation of Xing headers, now they contain the correct MPEG version and sample rate info.

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

4 files changed, 118 insertions, 97 deletions

diff --git a/apps/onplay.c b/apps/onplay.c
index ceaae2c..3ba4c1b 100644
--- a/apps/onplay.c
+++ b/apps/onplay.c
@@ -210,14 +210,15 @@ static const unsigned char empty_id3_header[] =
 
 static bool vbr_fix(void)
 {
-    unsigned char xingbuf[417];
+    unsigned char xingbuf[1500];
     struct mp3entry entry;
     int fd;
     int rc;
     int flen;
     int num_frames;
-    int fpos;
     int numbytes;
+    int framelen;
+    int unused_space;
 
     if(mpeg_status()) {
         splash(HZ*2, 0, true, str(LANG_VBRFIX_STOP_PLAY));
@@ -250,26 +251,45 @@ static bool vbr_fix(void)
                                   flen, xingupdate);
 
     if(num_frames) {
-        create_xing_header(fd, entry.first_frame_offset,
-                           flen, xingbuf, num_frames, xingupdate, true);
+        /* Note: We don't need to pass any values for mpeg_version and
+           sample_rate because they will be extracted from the mpeg stream */
+        framelen = create_xing_header(fd, entry.first_frame_offset,
+                                      flen, xingbuf, num_frames,
+                                      0, 0, xingupdate, true);
         
         /* Try to fit the Xing header first in the stream. Replace the existing
-           Xing header if there is one, else see if there is room between the
+           VBR header if there is one, else see if there is room between the
            ID3 tag and the first MP3 frame. */
-        if(entry.vbr_header_pos) {
-            /* Reuse existing Xing header */
-            fpos = entry.vbr_header_pos;
-            
-            DEBUGF("Reusing Xing header at %d\n", fpos);
-                
-            rc = lseek(fd, entry.vbr_header_pos, SEEK_SET);
+        if(entry.first_frame_offset - entry.id3v2len >=
+           (unsigned int)framelen) {
+            DEBUGF("Using existing space between ID3 and first frame\n");
+
+            /* Seek to the beginning of the unused space */
+            rc = lseek(fd, entry.id3v2len, SEEK_SET);
             if(rc < 0) {
                 close(fd);
                 fileerror(rc);
                 return true;
             }
+
+            unused_space =
+                entry.first_frame_offset - entry.id3v2len - framelen;
             
-            rc = write(fd, xingbuf, 417);
+            /* Fill the unused space with 0's (using the MP3 buffer)
+               and write it to the file */
+            if(unused_space)
+            {
+                memset(mp3buf, 0, unused_space);
+                rc = write(fd, mp3buf, unused_space);
+                if(rc < 0) {
+                    close(fd);
+                    fileerror(rc);
+                    return true;
+                }
+            }
+
+            /* Then write the Xing header */
+            rc = write(fd, xingbuf, framelen);
             if(rc < 0) {
                 close(fd);
                 fileerror(rc);
@@ -278,61 +298,41 @@ static bool vbr_fix(void)
             
             close(fd);
         } else {
-            /* Any room between ID3 tag and first MP3 frame? */
-            if(entry.first_frame_offset - entry.id3v2len > 417) {
-                DEBUGF("Using existing space between ID3 and first frame\n");
-                rc = lseek(fd, entry.first_frame_offset - 417, SEEK_SET);
-                if(rc < 0) {
-                    close(fd);
-                    fileerror(rc);
-                    return true;
-                }
-                
-                rc = write(fd, xingbuf, 417);
-                if(rc < 0) {
-                    close(fd);
-                    fileerror(rc);
-                    return true;
-                }
-                
-                close(fd);
+            /* If not, insert some space. If there is an ID3 tag in the
+               file we only insert just enough to squeeze the Xing header
+               in. If not, we insert an additional empty ID3 tag of 4K. */
+            
+            close(fd);
+            
+            /* Nasty trick alert! The insert_data_in_file() function
+               uses the MP3 buffer when copying the data. We assume
+               that the ID3 tag isn't longer than 1MB so the xing
+               buffer won't be overwritten. */
+            
+            if(entry.first_frame_offset) {
+                DEBUGF("Inserting %d bytes\n", framelen);
+                numbytes = framelen;
             } else {
-                /* If not, insert some space. If there is an ID3 tag in the
-                   file we only insert just enough to squeeze the Xing header
-                   in. If not, we insert an additional empty ID3 tag of 4K. */
-
-                close(fd);
-
-                /* Nasty trick alert! The insert_data_in_file() function
-                   uses the MP3 buffer when copying the data. We assume
-                   that the ID3 tag isn't longer than 1MB so the xing
-                   buffer won't be overwritten. */
-
-                if(entry.first_frame_offset) {
-                    DEBUGF("Inserting 417 bytes\n");
-                    numbytes = 417;
-                } else {
-                    DEBUGF("Inserting 4096+417 bytes\n");
-                    numbytes = 4096 + 417;
-                    
-                    memset(mp3buf + 0x100000, 0, numbytes);
-
-                    /* Insert the ID3 header */
-                    memcpy(mp3buf + 0x100000, empty_id3_header,
-                           sizeof(empty_id3_header));
-                }
-
-                /* Copy the Xing header */
-                memcpy(mp3buf + 0x100000 + numbytes - 417, xingbuf, 417);
+                DEBUGF("Inserting 4096+%d bytes\n", framelen);
+                numbytes = 4096 + framelen;
                 
-                rc = insert_data_in_file(selected_file,
-                                         entry.first_frame_offset,
-                                         mp3buf + 0x100000, numbytes);
+                memset(mp3buf + 0x100000, 0, numbytes);
                 
-                if(rc < 0) {
-                    fileerror(rc);
-                    return true;
-                }
+                /* Insert the ID3 header */
+                memcpy(mp3buf + 0x100000, empty_id3_header,
+                       sizeof(empty_id3_header));
+            }
+            
+            /* Copy the Xing header */
+            memcpy(mp3buf + 0x100000 + numbytes - framelen, xingbuf, framelen);
+            
+            rc = insert_data_in_file(selected_file,
+                                     entry.first_frame_offset,
+                                     mp3buf + 0x100000, numbytes);
+            
+            if(rc < 0) {
+                fileerror(rc);
+                return true;
             }
         }
         
diff --git a/firmware/export/mp3data.h b/firmware/export/mp3data.h
index 54a6899..d180935 100644
--- a/firmware/export/mp3data.h
+++ b/firmware/export/mp3data.h
@@ -61,6 +61,7 @@ int count_mp3_frames(int fd, int startpos, int filesize,
                      void (*progressfunc)(int));
 int create_xing_header(int fd, int startpos, int filesize,
                        unsigned char *buf, int num_frames,
+                       int mpeg_version, int sample_rate,
                        void (*progressfunc)(int), bool generate_toc);
 
 #endif
diff --git a/firmware/mp3data.c b/firmware/mp3data.c
index 28f2cb2..98dff50 100644
--- a/firmware/mp3data.c
+++ b/firmware/mp3data.c
@@ -35,7 +35,7 @@
 #include "mp3data.h"
 #include "file.h"
 
-#undef DEBUG_VERBOSE
+#define DEBUG_VERBOSE
 
 #define BYTES2INT(b1,b2,b3,b4) (((b1 & 0xFF) << (3*8)) | \
                                 ((b2 & 0xFF) << (2*8)) | \
@@ -530,9 +530,10 @@ int get_mp3file_info(int fd, struct mp3info *info)
     return bytecount;
 }
 
-/* This is an MP3 header, 128kbit/s, 44.1kHz, with silence */
+/* This is an MP3 header, 128kbit/s, with silence
+   MPEG version and sample frequency are not set */
 static const unsigned char xing_frame_header[] = {
-    0xff, 0xfa, 0x90, 0x64, 0x86, 0x1f
+    0xff, 0xe2, 0x90, 0x64, 0x86, 0x1f
 };
 
 static const char cooltext[] = "Rockbox rocks";
@@ -601,11 +602,15 @@ int count_mp3_frames(int fd, int startpos, int filesize,
     }
 }
 
+/* Note: mpeg_version and sample_rate are 2-bit values, as specified by the
+   MPEG frame standard. See the tables above. */
 int create_xing_header(int fd, int startpos, int filesize,
                        unsigned char *buf, int num_frames,
+                       int mpeg_version, int sample_rate,
                        void (*progressfunc)(int), bool generate_toc)
 {
     unsigned long header = 0;
+    unsigned long saved_header;
     struct mp3info info;
     int pos, last_pos;
     int i, j;
@@ -618,9 +623,9 @@ int create_xing_header(int fd, int startpos, int filesize,
     DEBUGF("create_xing_header()\n");
     
     /* Create the frame header */
-    memset(buf, 0, 417);
+    memset(buf, 0, 1500);
     memcpy(buf, xing_frame_header, 6);
-           
+
     lseek(fd, startpos, SEEK_SET);
     buf_init();
     
@@ -664,6 +669,10 @@ int create_xing_header(int fd, int startpos, int filesize,
                 buf_seek(fd, info.frame_size-4);
                 filepos += info.frame_size;
             }
+
+            /* Save one header for later use */
+            if(i == 1)
+                saved_header = header;
             
             if(progressfunc)
             {
@@ -684,6 +693,26 @@ int create_xing_header(int fd, int startpos, int filesize,
 
     memcpy(buf + index + 100, cooltext, sizeof(cooltext));
 
+    /* We must fill in the correct sample rate and mpeg version. If the TOC
+       should be generated, we take that data from the actual stream. If not,
+       we use the supplied parameters. */
+    if(generate_toc)
+    {
+        saved_header &= (VERSION_MASK | SAMPLERATE_MASK);
+    
+        buf[1] |= (saved_header >> 16) & 0xff;
+        buf[2] |= (saved_header >> 8) & 0xff;
+    }
+    else
+    {
+        buf[1] |= mpeg_version << 3;
+        buf[2] |= sample_rate << 2;
+    }
+
+    /* Now get the length of the newly created frame */
+    header = BYTES2INT(buf[0], buf[1], buf[2], buf[3]);
+    mp3headerinfo(&info, header);
+    
 #ifdef DEBUG
     for(i = 0;i < 417;i++)
     {
@@ -694,5 +723,5 @@ int create_xing_header(int fd, int startpos, int filesize,
     }
 #endif
     
-    return 0;
+    return info.frame_size;
 }
diff --git a/firmware/mpeg.c b/firmware/mpeg.c
index 85f9c72..7e48040 100644
--- a/firmware/mpeg.c
+++ b/firmware/mpeg.c
@@ -489,7 +489,8 @@ unsigned long record_start_time; /* Value of current_tick when recording
                                     was started */
 static bool saving; /* We are saving the buffer to disk */
 static char recording_filename[MAX_PATH];
-
+static int rec_frequency_index; /* For create_xing_header() calls */
+static int rec_version_index;   /* For create_xing_header() calls */
 #endif
 
 static int mpeg_file;
@@ -1010,13 +1011,6 @@ void IRQ3(void)
         dma_tick();
     else
         postpone_dma_tick();
-
-#if 0
-    if(mpeg_mode == MPEG_ENCODER)
-        /* Shut off if recording is stopped */
-        if(!is_recording)
-            demand_irq_enable(false);
-#endif
 }
 #endif
 
@@ -1271,6 +1265,7 @@ static void mpeg_thread(void)
 #ifdef HAVE_MAS3587F
     int amount_to_save;
     int writelen;
+    int framelen;
 #endif
     
     is_playing = false;
@@ -1734,15 +1729,6 @@ static void mpeg_thread(void)
                     DEBUGF("R\n");
                     t1 = current_tick;
                     len = read(mpeg_file, mp3buf+mp3buf_write, amount_to_read);
-#if 0
-                    if(id3tags[tag_read_idx]->id3.vbr)
-                        /* Average bitrate * 1.5 */
-                        recalculate_watermark(
-                            (id3tags[tag_read_idx]->id3.bitrate * 3) / 2);
-                    else
-                        recalculate_watermark(
-                            id3tags[tag_read_idx]->id3.bitrate);
-#endif               
                     if(len > 0)
                     {
                         t2 = current_tick;
@@ -1854,10 +1840,10 @@ static void mpeg_thread(void)
                     DEBUGF("Recording...\n");
                     reset_mp3_buffer();
 
-                    /* Advance the write pointer 4096+417 bytes to make
+                    /* Advance the write pointer 4096+1500 bytes to make
                        room for an ID3 tag plus a VBR header */
-                    mp3buf_write = 4096+417;
-                    memset(mp3buf, 0, 4096+417);
+                    mp3buf_write = 4096+1500;
+                    memset(mp3buf, 0, 4096+1500);
 
                     /* Insert the ID3 header */
                     memcpy(mp3buf, empty_id3_header, sizeof(empty_id3_header));
@@ -1898,12 +1884,14 @@ static void mpeg_thread(void)
                     if(num_recorded_frames == 0x7ffff)
                         num_recorded_frames = 0;
                     
-                    create_xing_header(mpeg_file, 0, num_rec_bytes,
-                                       mp3buf, num_recorded_frames, NULL,
-                                       false);
+                    framelen = create_xing_header(mpeg_file, 0, num_rec_bytes,
+                                                  mp3buf, num_recorded_frames,
+                                                  rec_version_index,
+                                                  rec_frequency_index,
+                                                  NULL, false);
                     
-                    lseek(mpeg_file, 4096, SEEK_SET);
-                    write(mpeg_file, mp3buf, 417);
+                    lseek(mpeg_file, 4096+1500-framelen, SEEK_SET);
+                    write(mpeg_file, mp3buf, framelen);
                     close(mpeg_file);
                     
                     mpeg_file = -1;
@@ -2841,9 +2829,12 @@ void mpeg_set_recording_options(int frequency, int quality,
     unsigned long val;
 
     is_mpeg1 = (frequency < 3)?true:false;
+
+    rec_version_index = is_mpeg1?3:2;
+    rec_frequency_index = frequency % 3;
     
     val = (quality << 17) |
-        ((frequency % 3) << 10) |
+        (rec_frequency_index << 10) |
         ((is_mpeg1?1:0) << 9) |
         (1 << 8) | /* CRC on */
         (((channel_mode * 2 + 1) & 3) << 6) |