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

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,45 +251,45 @@ static bool vbr_fix(void)
                                   flen, xingupdate);
 
     if(num_frames) {
-        create_xing_header(fd, entry.first_frame_offset,
+        /* Note: We don't need to pass any values for mpeg_version and
-                           flen, xingbuf, num_frames, xingupdate, true);
+           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) {
+        if(entry.first_frame_offset - entry.id3v2len >=
-            /* Reuse existing Xing header */
+           (unsigned int)framelen) {
-            fpos = entry.vbr_header_pos;
-            
-            DEBUGF("Reusing Xing header at %d\n", fpos);
-                
-            rc = lseek(fd, entry.vbr_header_pos, 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);
-        } 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);
+
+            /* Seek to the beginning of the unused space */
+            rc = lseek(fd, entry.id3v2len, SEEK_SET);
             if(rc < 0) {
                 close(fd);
                 fileerror(rc);
                 return true;
             }
 
-                rc = write(fd, xingbuf, 417);
+            unused_space =
+                entry.first_frame_offset - entry.id3v2len - framelen;
+            
+            /* 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);
@@ -309,11 +310,11 @@ static bool vbr_fix(void)
                buffer won't be overwritten. */
             
             if(entry.first_frame_offset) {
-                    DEBUGF("Inserting 417 bytes\n");
+                DEBUGF("Inserting %d bytes\n", framelen);
-                    numbytes = 417;
+                numbytes = framelen;
             } else {
-                    DEBUGF("Inserting 4096+417 bytes\n");
+                DEBUGF("Inserting 4096+%d bytes\n", framelen);
-                    numbytes = 4096 + 417;
+                numbytes = 4096 + framelen;
                 
                 memset(mp3buf + 0x100000, 0, numbytes);
                 
@@ -323,7 +324,7 @@ static bool vbr_fix(void)
             }
             
             /* Copy the Xing header */
-                memcpy(mp3buf + 0x100000 + numbytes - 417, xingbuf, 417);
+            memcpy(mp3buf + 0x100000 + numbytes - framelen, xingbuf, framelen);
             
             rc = insert_data_in_file(selected_file,
                                      entry.first_frame_offset,
@@ -334,7 +335,6 @@ static bool vbr_fix(void)
                 return true;
             }
         }
-        }
         
         xingupdate(100);
     }

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

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,7 +623,7 @@ 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);
@@ -665,6 +670,10 @@ int create_xing_header(int fd, int startpos, int filesize,
                 filepos += info.frame_size;
             }
 
+            /* Save one header for later use */
+            if(i == 1)
+                saved_header = header;
+            
             if(progressfunc)
             {
                 progressfunc(50 + i/2);
@@ -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;
 }

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;
+                    mp3buf_write = 4096+1500;
-                    memset(mp3buf, 0, 4096+417);
+                    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,
+                    framelen = create_xing_header(mpeg_file, 0, num_rec_bytes,
-                                       mp3buf, num_recorded_frames, NULL,
+                                                  mp3buf, num_recorded_frames,
-                                       false);
+                                                  rec_version_index,
+                                                  rec_frequency_index,
+                                                  NULL, false);
                     
-                    lseek(mpeg_file, 4096, SEEK_SET);
+                    lseek(mpeg_file, 4096+1500-framelen, SEEK_SET);
-                    write(mpeg_file, mp3buf, 417);
+                    write(mpeg_file, mp3buf, framelen);
                     close(mpeg_file);
                     
                     mpeg_file = -1;
@@ -2842,8 +2830,11 @@ void mpeg_set_recording_options(int frequency, int quality,
 
     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) |