# HG changeset patch # User Chris Cannam # Date 1480007191 0 # Node ID fa574c909c3db7dc9ef6f2294e100315553b0498 # Parent a45312bd93065d0c3fcbd8f285c27ea9ce1839c6 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. diff -r a45312bd9306 -r fa574c909c3d data/fileio/MP3FileReader.cpp --- 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, diff -r a45312bd9306 -r fa574c909c3d data/fileio/MP3FileReader.h --- 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;