changeset 1290:fa574c909c3d 3.0-integration

Add MAD_BUFFER_GUARD padding at end of mp3 buffer, in order to ensure last frame is decoded successfully (otherwise the decoded audio is truncated). Another thing learned from madplay.
author Chris Cannam
date Thu, 24 Nov 2016 17:06:31 +0000 (2016-11-24)
parents a45312bd9306
children 22f66068b464 3cde25cbe7f8
files data/fileio/MP3FileReader.cpp data/fileio/MP3FileReader.h
diffstat 2 files changed, 52 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/data/fileio/MP3FileReader.cpp	Thu Nov 24 13:38:45 2016 +0000
+++ b/data/fileio/MP3FileReader.cpp	Thu Nov 24 17:06:31 2016 +0000
@@ -73,9 +73,11 @@
 
     m_fileSize = stat.st_size;
 
-    m_filebuffer = 0;
-    m_samplebuffer = 0;
-    m_samplebuffersize = 0;
+    m_fileBuffer = 0;
+    m_fileBufferSize = 0;
+
+    m_sampleBuffer = 0;
+    m_sampleBufferSize = 0;
 
     int fd = -1;
     if ((fd = ::open(m_path.toLocal8Bit().data(), O_RDONLY
@@ -88,7 +90,12 @@
     }	
 
     try {
-        m_filebuffer = new unsigned char[m_fileSize];
+        // We need a mysterious MAD_BUFFER_GUARD (== 8) zero bytes at
+        // end of input, to ensure libmad decodes the last frame
+        // correctly. Otherwise the decoded audio is truncated.
+        m_fileBufferSize = m_fileSize + MAD_BUFFER_GUARD;
+        m_fileBuffer = new unsigned char[m_fileBufferSize];
+        memset(m_fileBuffer + m_fileSize, 0, MAD_BUFFER_GUARD);
     } catch (...) {
         m_error = QString("Out of memory");
         ::close(fd);
@@ -98,17 +105,19 @@
     ssize_t sz = 0;
     ssize_t offset = 0;
     while (offset < m_fileSize) {
-        sz = ::read(fd, m_filebuffer + offset, m_fileSize - offset);
+        sz = ::read(fd, m_fileBuffer + offset, m_fileSize - offset);
         if (sz < 0) {
             m_error = QString("Read error for file %1 (after %2 bytes)")
                 .arg(m_path).arg(offset);
-            delete[] m_filebuffer;
+            delete[] m_fileBuffer;
             ::close(fd);
             return;
         } else if (sz == 0) {
-            cerr << QString("MP3FileReader::MP3FileReader: Warning: reached EOF after only %1 of %2 bytes")
+            SVCERR << QString("MP3FileReader::MP3FileReader: Warning: reached EOF after only %1 of %2 bytes")
                 .arg(offset).arg(m_fileSize) << endl;
             m_fileSize = offset;
+            m_fileBufferSize = m_fileSize + MAD_BUFFER_GUARD;
+            memset(m_fileBuffer + m_fileSize, 0, MAD_BUFFER_GUARD);
             break;
         }
         offset += sz;
@@ -126,12 +135,12 @@
                 (tr("Decoding %1...").arg(QFileInfo(m_path).fileName()));
         }
 
-        if (!decode(m_filebuffer, m_fileSize)) {
+        if (!decode(m_fileBuffer, m_fileBufferSize)) {
             m_error = QString("Failed to decode file %1.").arg(m_path);
         }
         
-        delete[] m_filebuffer;
-        m_filebuffer = 0;
+        delete[] m_fileBuffer;
+        m_fileBuffer = 0;
 
         if (isDecodeCacheInitialised()) finishDecodeCache();
         endSerialised();
@@ -267,19 +276,19 @@
 void
 MP3FileReader::DecodeThread::run()
 {
-    if (!m_reader->decode(m_reader->m_filebuffer, m_reader->m_fileSize)) {
+    if (!m_reader->decode(m_reader->m_fileBuffer, m_reader->m_fileBufferSize)) {
         m_reader->m_error = QString("Failed to decode file %1.").arg(m_reader->m_path);
     }
 
-    delete[] m_reader->m_filebuffer;
-    m_reader->m_filebuffer = 0;
+    delete[] m_reader->m_fileBuffer;
+    m_reader->m_fileBuffer = 0;
 
-    if (m_reader->m_samplebuffer) {
+    if (m_reader->m_sampleBuffer) {
         for (int c = 0; c < m_reader->m_channelCount; ++c) {
-            delete[] m_reader->m_samplebuffer[c];
+            delete[] m_reader->m_sampleBuffer[c];
         }
-        delete[] m_reader->m_samplebuffer;
-        m_reader->m_samplebuffer = 0;
+        delete[] m_reader->m_sampleBuffer;
+        m_reader->m_sampleBuffer = 0;
     }
 
     if (m_reader->isDecodeCacheInitialised()) m_reader->finishDecodeCache();
@@ -430,24 +439,27 @@
         }
     }
 
-    if (m_cancelled) return MAD_FLOW_STOP;
+    if (m_cancelled) {
+        SVDEBUG << "MP3FileReader: Decoding cancelled" << endl;
+        return MAD_FLOW_STOP;
+    }
 
     if (!isDecodeCacheInitialised()) {
         initialiseDecodeCache();
     }
 
-    if (int(m_samplebuffersize) < frames) {
-        if (!m_samplebuffer) {
-            m_samplebuffer = new float *[channels];
+    if (m_sampleBufferSize < size_t(frames)) {
+        if (!m_sampleBuffer) {
+            m_sampleBuffer = new float *[channels];
             for (int c = 0; c < channels; ++c) {
-                m_samplebuffer[c] = 0;
+                m_sampleBuffer[c] = 0;
             }
         }
         for (int c = 0; c < channels; ++c) {
-            delete[] m_samplebuffer[c];
-            m_samplebuffer[c] = new float[frames];
+            delete[] m_sampleBuffer[c];
+            m_sampleBuffer[c] = new float[frames];
         }
-        m_samplebuffersize = frames;
+        m_sampleBufferSize = frames;
     }
 
     int activeChannels = int(sizeof(pcm->samples) / sizeof(pcm->samples[0]));
@@ -462,11 +474,11 @@
 	    }
 	    float fsample = float(sample) / float(MAD_F_ONE);
             
-            m_samplebuffer[ch][i] = fsample;
+            m_sampleBuffer[ch][i] = fsample;
 	}
     }
 
-    addSamplesToDecodeCache(m_samplebuffer, frames);
+    addSamplesToDecodeCache(m_sampleBuffer, frames);
 
     ++m_mp3FrameCount;
 
@@ -479,6 +491,13 @@
                               struct mad_frame *)
 {
     DecoderData *data = (DecoderData *)dp;
+
+    if (stream->error == MAD_ERROR_LOSTSYNC &&
+        data->length == 0) {
+        // We are at end of file, losing sync is expected behaviour
+        return MAD_FLOW_CONTINUE;
+    }
+    
     if (!data->reader->m_decodeErrorShown) {
         char buffer[256];
         snprintf(buffer, 255,
--- a/data/fileio/MP3FileReader.h	Thu Nov 24 13:38:45 2016 +0000
+++ b/data/fileio/MP3FileReader.h	Thu Nov 24 17:06:31 2016 +0000
@@ -75,17 +75,18 @@
     int m_completion;
     bool m_done;
 
-    unsigned char *m_filebuffer;
-    float **m_samplebuffer;
-    int m_samplebuffersize;
+    unsigned char *m_fileBuffer;
+    size_t m_fileBufferSize;
+    
+    float **m_sampleBuffer;
+    size_t m_sampleBufferSize;
 
     ProgressReporter *m_reporter;
     bool m_cancelled;
 
     bool m_decodeErrorShown;
 
-    struct DecoderData
-    {
+    struct DecoderData {
 	unsigned char const *start;
 	sv_frame_t length;
 	MP3FileReader *reader;