From 6d9c25962d2efa2cf0e7d028458851f8a7a7de52 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 12:47:28 +0200 Subject: [PATCH 1/7] Fix handling of mono MP3 frames within a stereo file --- src/sources/soundsourcemp3.cpp | 69 +++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index 525a56d338f9..1642acc84131 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -204,6 +204,7 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { mad_header madHeader; mad_header_init(&madHeader); + SINT maxChannelCount = getChannelCount(); do { if (!decodeFrameHeader(&madHeader, &m_madStream, true)) { if (isStreamValid(m_madStream)) { @@ -216,22 +217,29 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { } // Grab data from madHeader + const SINT madChannelCount = MAD_NCHANNELS(&madHeader); - if (isChannelCountValid()) { - // check for consistent number of channels - if (getChannelCount() != madChannelCount) { - qWarning() << "Differing number of channels in some headers:" - << m_file.fileName() - << madChannelCount << "<>" << getChannelCount(); - qWarning() << "MP3 files with varying channel configurations are not supported!"; - // Abort - mad_header_finish(&madHeader); - return ERR; + if (0 < madChannelCount) { + if ((0 < maxChannelCount) && (madChannelCount != maxChannelCount)) { + qWarning() << "Differing number of channels" + << madChannelCount << "<>" << maxChannelCount + << "in some MP3 frame headers:" + << m_file.fileName(); + if ((madChannelCount > 2) || (maxChannelCount > 2)) { + // Abort, because we only support mono -> stereo conversion + mad_header_finish(&madHeader); + return ERR; + } } } else { - // initially set the number of channels - setChannelCount(madChannelCount); + qWarning() << "Invalid number of channels" << madChannelCount + << "in MP3 frame header:" << m_file.fileName(); + // Abort + mad_header_finish(&madHeader); + return ERR; } + maxChannelCount = math_max(madChannelCount, maxChannelCount); + const unsigned int madSampleRate = madHeader.samplerate; const int frameRateIndex = getIndexByFrameRate(madSampleRate); if (frameRateIndex >= kFrameRateCount) { @@ -322,7 +330,8 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { return ERR; } - // Initialize the audio stream length + // Initialize the AudioSource + setChannelCount(maxChannelCount); setFrameCount(m_curFrameIndex); // Calculate average values @@ -585,10 +594,24 @@ SINT SoundSourceMp3::readSampleFrames( } DEBUG_ASSERT(isStreamValid(m_madStream)); - DEBUG_ASSERT(getChannelCount() == MAD_NCHANNELS(&m_madFrame.header)); + +#ifndef QT_NO_DEBUG_OUTPUT + const SINT madFrameChannelCount = MAD_NCHANNELS(&m_madFrame.header); + if (madFrameChannelCount != getChannelCount()) { + qDebug() << "MP3 frame header with mismatching number of channels" + << madFrameChannelCount << "<>" << getChannelCount(); + } +#endif // Once decoded the frame is synthesized to PCM samples mad_synth_frame(&m_madSynth, &m_madFrame); +#ifndef QT_NO_DEBUG_OUTPUT + const SINT madSynthSampleRate = m_madSynth.pcm.samplerate; + if (madSynthSampleRate != getFrameRate()) { + qDebug() << "Reading MP3 data with different sampling rate" + << madSynthSampleRate << "<>" << getFrameRate(); + } +#endif m_madSynthCount = m_madSynth.pcm.length; DEBUG_ASSERT(0 < m_madSynthCount); } @@ -600,18 +623,29 @@ SINT SoundSourceMp3::readSampleFrames( const SINT madSynthOffset = m_madSynth.pcm.length - m_madSynthCount; DEBUG_ASSERT(madSynthOffset < m_madSynth.pcm.length); - if (isChannelCountMono()) { + const SINT madSynthChannelCount = m_madSynth.pcm.channels; + DEBUG_ASSERT(0 < madSynthChannelCount); + DEBUG_ASSERT(madSynthChannelCount <= getChannelCount()); +#ifndef QT_NO_DEBUG_OUTPUT + if (madSynthChannelCount != getChannelCount()) { + qDebug() << "Reading MP3 data with different number of channels" + << madSynthChannelCount << "<>" << getChannelCount(); + } +#endif + if (1 == math_min(madSynthChannelCount, getChannelCount())) { for (SINT i = 0; i < synthReadCount; ++i) { const CSAMPLE sampleValue = madScale( m_madSynth.pcm.samples[0][madSynthOffset + i]); *pSampleBuffer = sampleValue; ++pSampleBuffer; - if (readStereoSamples) { + if (readStereoSamples || isChannelCountStereo()) { + // Duplicate the 1st channel *pSampleBuffer = sampleValue; ++pSampleBuffer; } } - } else if (isChannelCountStereo() || readStereoSamples) { + } else if (readStereoSamples || isChannelCountStereo()) { + DEBUG_ASSERT(2 <= madSynthChannelCount); for (SINT i = 0; i < synthReadCount; ++i) { *pSampleBuffer = madScale( m_madSynth.pcm.samples[0][madSynthOffset + i]); @@ -621,6 +655,7 @@ SINT SoundSourceMp3::readSampleFrames( ++pSampleBuffer; } } else { + DEBUG_ASSERT(madSynthChannelCount == getChannelCount()); for (SINT i = 0; i < synthReadCount; ++i) { for (SINT j = 0; j < getChannelCount(); ++j) { *pSampleBuffer = madScale( From 18d7ec3346bcefda67d6e5c738b043b0bfacb117 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 13:02:24 +0200 Subject: [PATCH 2/7] Abort loading of MP3 file on error before determining sampling rate --- src/sources/soundsourcemp3.cpp | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index 1642acc84131..d1228c4998db 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -273,6 +273,25 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { mad_header_finish(&madHeader); + if (MAD_ERROR_NONE != m_madStream.error) { + // Unreachable code for recoverable errors + DEBUG_ASSERT(!MAD_RECOVERABLE(m_madStream.error)); + if (MAD_ERROR_BUFLEN != m_madStream.error) { + qWarning() << "Unrecoverable MP3 header error:" + << mad_stream_errorstr(&m_madStream); + // Abort + return ERR; + } + } + + if (m_seekFrameList.empty()) { + // This is not a working MP3 file. + qWarning() << "SSMP3: This is not a working MP3 file:" + << m_file.fileName(); + // Abort + return ERR; + } + int mostCommonFrameRateIndex = kFrameRateCount; // invalid int mostCommonFrameRatecount = 0; int differentRates = 0; @@ -311,25 +330,6 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { return ERR; } - if (MAD_ERROR_NONE != m_madStream.error) { - // Unreachable code for recoverable errors - DEBUG_ASSERT(!MAD_RECOVERABLE(m_madStream.error)); - if (MAD_ERROR_BUFLEN != m_madStream.error) { - qWarning() << "Unrecoverable MP3 header error:" - << mad_stream_errorstr(&m_madStream); - // Abort - return ERR; - } - } - - if (m_seekFrameList.empty()) { - // This is not a working MP3 file. - qWarning() << "SSMP3: This is not a working MP3 file:" - << m_file.fileName(); - // Abort - return ERR; - } - // Initialize the AudioSource setChannelCount(maxChannelCount); setFrameCount(m_curFrameIndex); From 4a2023a9899fbe18c74dbb0c8c0c604ec1087462 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 13:10:27 +0200 Subject: [PATCH 3/7] Limit the maximum number of channels in MP3 files to 2 --- src/sources/soundsourcemp3.cpp | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index d1228c4998db..d6f0fc0644f1 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -8,8 +8,11 @@ namespace Mixxx { namespace { +// MP3 does only support 1 or 2 channels +const SINT kChannelCountMax = 2; + // mp3 supports 9 different frame rates -static const int kFrameRateCount = 9; +const int kFrameRateCount = 9; int getIndexByFrameRate(unsigned int frameRate) { switch (frameRate) { @@ -220,16 +223,19 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { const SINT madChannelCount = MAD_NCHANNELS(&madHeader); if (0 < madChannelCount) { + if (madChannelCount > kChannelCountMax) { + qWarning() << "Invalid number of channels" + << madChannelCount << ">" << kChannelCountMax + << "in MP3 frame header:" << m_file.fileName(); + // Abort + mad_header_finish(&madHeader); + return ERR; + } if ((0 < maxChannelCount) && (madChannelCount != maxChannelCount)) { qWarning() << "Differing number of channels" << madChannelCount << "<>" << maxChannelCount << "in some MP3 frame headers:" << m_file.fileName(); - if ((madChannelCount > 2) || (maxChannelCount > 2)) { - // Abort, because we only support mono -> stereo conversion - mad_header_finish(&madHeader); - return ERR; - } } } else { qWarning() << "Invalid number of channels" << madChannelCount @@ -644,8 +650,9 @@ SINT SoundSourceMp3::readSampleFrames( ++pSampleBuffer; } } - } else if (readStereoSamples || isChannelCountStereo()) { - DEBUG_ASSERT(2 <= madSynthChannelCount); + } else { + DEBUG_ASSERT(readStereoSamples || isChannelCountStereo()); + DEBUG_ASSERT(2 == madSynthChannelCount); for (SINT i = 0; i < synthReadCount; ++i) { *pSampleBuffer = madScale( m_madSynth.pcm.samples[0][madSynthOffset + i]); @@ -654,15 +661,6 @@ SINT SoundSourceMp3::readSampleFrames( m_madSynth.pcm.samples[1][madSynthOffset + i]); ++pSampleBuffer; } - } else { - DEBUG_ASSERT(madSynthChannelCount == getChannelCount()); - for (SINT i = 0; i < synthReadCount; ++i) { - for (SINT j = 0; j < getChannelCount(); ++j) { - *pSampleBuffer = madScale( - m_madSynth.pcm.samples[j][madSynthOffset + i]); - ++pSampleBuffer; - } - } } } // consume decoded output data From 26949dcbe65dd992f869e7364ea6a4d53d00457a Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 13:20:46 +0200 Subject: [PATCH 4/7] Fix debug assertion to accept MP3 files without any tags --- src/sources/soundsourcemp3.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index d6f0fc0644f1..290d1c9b8777 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -274,7 +274,7 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { mad_timer_count(madDuration, madUnits); DEBUG_ASSERT(m_madStream.this_frame); - DEBUG_ASSERT(0 < (m_madStream.this_frame - m_pFileData)); + DEBUG_ASSERT(0 <= (m_madStream.this_frame - m_pFileData)); } while (quint64(m_madStream.this_frame - m_pFileData) < m_fileSize); mad_header_finish(&madHeader); From 1b07544c72c63a6f8ed923f8379416e18ecf5866 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 14:20:03 +0200 Subject: [PATCH 5/7] Some minor code reorderings when loading MP3 files --- src/sources/soundsourcemp3.cpp | 39 +++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index 290d1c9b8777..18d1c5979bab 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -63,11 +63,10 @@ int getFrameRateByIndex(int frameRateIndex) { } -const CSAMPLE kMadScale = AudioSource::kSampleValuePeak - / CSAMPLE(MAD_F_ONE); +const CSAMPLE kMadScale = AudioSource::kSampleValuePeak / CSAMPLE(MAD_F_ONE); -inline CSAMPLE madScale(mad_fixed_t sample) { - return sample * kMadScale; +inline CSAMPLE madScaleSampleValue(mad_fixed_t sampleValue) { + return sampleValue * kMadScale; } // Optimization: Reserve initial capacity for seek frame list @@ -201,7 +200,6 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { // Decode all the headers and calculate audio properties - mad_timer_t madDuration = mad_timer_zero; unsigned long sumBitrate = 0; mad_header madHeader; @@ -220,6 +218,21 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { } // Grab data from madHeader + const unsigned int madSampleRate = madHeader.samplerate; + + // TODO(XXX): Replace DEBUG_ASSERT with static_assert + // MAD must not change its enum values! + DEBUG_ASSERT(MAD_UNITS_8000_HZ == 8000); + const mad_units madUnits = static_cast(madSampleRate); + + const long madFrameLength = mad_timer_count(madHeader.duration, madUnits); + if (0 >= madFrameLength) { + qWarning() << "Skipping MP3 frame with invalid length" + << madFrameLength + << "in:" << m_file.fileName(); + // Skip frame + continue; + } const SINT madChannelCount = MAD_NCHANNELS(&madHeader); if (0 < madChannelCount) { @@ -246,7 +259,6 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { } maxChannelCount = math_max(madChannelCount, maxChannelCount); - const unsigned int madSampleRate = madHeader.samplerate; const int frameRateIndex = getIndexByFrameRate(madSampleRate); if (frameRateIndex >= kFrameRateCount) { qWarning() << "Invalid sample rate:" << m_file.fileName() @@ -262,16 +274,9 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { // Accumulate data from the header sumBitrate += madHeader.bitrate; - mad_timer_add(&madDuration, madHeader.duration); - - // TODO() uses static_assert - // MAD must not change its enum values - DEBUG_ASSERT(MAD_UNITS_8000_HZ == 8000); - mad_units madUnits = static_cast(madSampleRate); // Update current stream position - m_curFrameIndex = kFrameIndexMin + - mad_timer_count(madDuration, madUnits); + m_curFrameIndex += madFrameLength; DEBUG_ASSERT(m_madStream.this_frame); DEBUG_ASSERT(0 <= (m_madStream.this_frame - m_pFileData)); @@ -640,7 +645,7 @@ SINT SoundSourceMp3::readSampleFrames( #endif if (1 == math_min(madSynthChannelCount, getChannelCount())) { for (SINT i = 0; i < synthReadCount; ++i) { - const CSAMPLE sampleValue = madScale( + const CSAMPLE sampleValue = madScaleSampleValue( m_madSynth.pcm.samples[0][madSynthOffset + i]); *pSampleBuffer = sampleValue; ++pSampleBuffer; @@ -654,10 +659,10 @@ SINT SoundSourceMp3::readSampleFrames( DEBUG_ASSERT(readStereoSamples || isChannelCountStereo()); DEBUG_ASSERT(2 == madSynthChannelCount); for (SINT i = 0; i < synthReadCount; ++i) { - *pSampleBuffer = madScale( + *pSampleBuffer = madScaleSampleValue( m_madSynth.pcm.samples[0][madSynthOffset + i]); ++pSampleBuffer; - *pSampleBuffer = madScale( + *pSampleBuffer = madScaleSampleValue( m_madSynth.pcm.samples[1][madSynthOffset + i]); ++pSampleBuffer; } From b7d998549998ce01cc7b71309b7830bf3bb8987b Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 14:24:52 +0200 Subject: [PATCH 6/7] Fix libMAD error handling for MP3 decoding The decoding functions do NOT return the actual error code!! Instead the error code must be read from mad_stream if the decoding functions returned != 0. This caused some really ugly sounds at the beginning of some MP3 files, because frames were not skipped as suppossed to be. --- src/sources/soundsourcemp3.cpp | 124 +++++++++++++++++---------------- 1 file changed, 64 insertions(+), 60 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index 18d1c5979bab..65ffe220a352 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -91,16 +91,16 @@ void logFrameHeader(QDebug logger, const mad_header& madHeader) { << "flags:" << formatHeaderFlags(madHeader.flags); } -inline bool isRecoverableError(int madError) { - return MAD_RECOVERABLE(madError); +inline bool isRecoverableError(const mad_stream& madStream) { + return MAD_RECOVERABLE(madStream.error); } -inline bool isUnrecoverableError(int madError) { - return (MAD_ERROR_NONE != madError) && !isRecoverableError(madError); +inline bool isUnrecoverableError(const mad_stream& madStream) { + return (MAD_ERROR_NONE != madStream.error) && !isRecoverableError(madStream); } inline bool isStreamValid(const mad_stream& madStream) { - return !isUnrecoverableError(madStream.error); + return !isUnrecoverableError(madStream); } bool decodeFrameHeader( @@ -108,38 +108,40 @@ bool decodeFrameHeader( mad_stream* pMadStream, bool skipId3Tag) { DEBUG_ASSERT(isStreamValid(*pMadStream)); - const int decodeResult = mad_header_decode(pMadHeader, pMadStream); - if (MAD_ERROR_BUFLEN == decodeResult) { - // EOF - return false; - } - if (isUnrecoverableError(decodeResult)) { - DEBUG_ASSERT(!isStreamValid(*pMadStream)); - qWarning() << "Unrecoverable MP3 header decoding error:" - << mad_stream_errorstr(pMadStream); - return false; - } -#ifndef QT_NO_DEBUG_OUTPUT - // Logging of MP3 frame headers should only be enabled - // for debugging purposes. - //logFrameHeader(qDebug(), *pMadHeader); -#endif - if (isRecoverableError(decodeResult)) { - if ((MAD_ERROR_LOSTSYNC == decodeResult) && skipId3Tag) { - long tagsize = id3_tag_query(pMadStream->this_frame, - pMadStream->bufend - pMadStream->this_frame); - if (0 < tagsize) { - // Skip ID3 tag data - mad_stream_skip(pMadStream, tagsize); - // Return immediately to suppress lost - // synchronization warnings - return false; + if (mad_header_decode(pMadHeader, pMadStream)) { + // Something went wrong when decoding the frame header... + if (MAD_ERROR_BUFLEN == pMadStream->error) { + // EOF + return false; + } + if (isUnrecoverableError(*pMadStream)) { + DEBUG_ASSERT(!isStreamValid(*pMadStream)); + qWarning() << "Unrecoverable MP3 header decoding error:" + << mad_stream_errorstr(pMadStream); + return false; + } + #ifndef QT_NO_DEBUG_OUTPUT + // Logging of MP3 frame headers should only be enabled + // for debugging purposes. + //logFrameHeader(qDebug(), *pMadHeader); + #endif + if (isRecoverableError(*pMadStream)) { + if ((MAD_ERROR_LOSTSYNC == pMadStream->error) && skipId3Tag) { + long tagsize = id3_tag_query(pMadStream->this_frame, + pMadStream->bufend - pMadStream->this_frame); + if (0 < tagsize) { + // Skip ID3 tag data + mad_stream_skip(pMadStream, tagsize); + // Return immediately to suppress lost + // synchronization warnings + return false; + } } + qWarning() << "Recoverable MP3 header decoding error:" + << mad_stream_errorstr(pMadStream); + logFrameHeader(qWarning(), *pMadHeader); + return false; } - qWarning() << "Recoverable MP3 header decoding error:" - << mad_stream_errorstr(pMadStream); - logFrameHeader(qWarning(), *pMadHeader); - return false; } DEBUG_ASSERT(isStreamValid(*pMadStream)); return true; @@ -569,34 +571,36 @@ SINT SoundSourceMp3::readSampleFrames( // Don't change anything at the following lines of code // unless you know what you are doing!!! unsigned char const* pMadThisFrame = m_madStream.this_frame; - const int decodeResult = mad_frame_decode(&m_madFrame, &m_madStream); - if (MAD_ERROR_BUFLEN == decodeResult) { - // Abort - break; - } - if (isUnrecoverableError(decodeResult)) { - qWarning() << "Unrecoverable MP3 frame decoding error:" - << mad_stream_errorstr(&m_madStream); - // Abort - break; - } - if (isRecoverableError(decodeResult)) { - if (pMadThisFrame != m_madStream.this_frame) { - // Ignore all recoverable errors (and especially - // "lost synchronization" warnings) while skipping - // over prefetched frames after seeking. - if (pSampleBuffer) { - qWarning() << "Recoverable MP3 frame decoding error:" - << mad_stream_errorstr(&m_madStream); - } else { - // Decoded samples will simply be discarded - qDebug() << "Recoverable MP3 frame decoding error while skipping:" + if (mad_frame_decode(&m_madFrame, &m_madStream)) { + // Something went wrong when decoding the frame... + if (MAD_ERROR_BUFLEN == m_madStream.error) { + // Abort + break; + } + if (isUnrecoverableError(m_madStream)) { + qWarning() << "Unrecoverable MP3 frame decoding error:" << mad_stream_errorstr(&m_madStream); + // Abort + break; + } + if (isRecoverableError(m_madStream)) { + if (pMadThisFrame != m_madStream.this_frame) { + // Ignore all recoverable errors (and especially + // "lost synchronization" warnings) while skipping + // over prefetched frames after seeking. + if (pSampleBuffer) { + qWarning() << "Recoverable MP3 frame decoding error:" + << mad_stream_errorstr(&m_madStream); + } else { + // Decoded samples will simply be discarded + qDebug() << "Recoverable MP3 frame decoding error while skipping:" + << mad_stream_errorstr(&m_madStream); + } } + // Acknowledge error... + m_madStream.error = MAD_ERROR_NONE; + // ...and continue } - // Acknowledge error... - m_madStream.error = MAD_ERROR_NONE; - // ...and continue } if (pMadThisFrame == m_madStream.this_frame) { qDebug() << "Retry decoding MP3 frame @" << m_curFrameIndex; From e003d061c81d43260f0594db90e3c063a75ee6f4 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 25 Apr 2015 20:12:00 +0200 Subject: [PATCH 7/7] Separate code paths for reading mono/stereo MP3 data Slightly more code, but hopefully easier to understand. --- src/sources/soundsourcemp3.cpp | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index 65ffe220a352..3b37c9e51735 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -647,21 +647,37 @@ SINT SoundSourceMp3::readSampleFrames( << madSynthChannelCount << "<>" << getChannelCount(); } #endif - if (1 == math_min(madSynthChannelCount, getChannelCount())) { - for (SINT i = 0; i < synthReadCount; ++i) { - const CSAMPLE sampleValue = madScaleSampleValue( - m_madSynth.pcm.samples[0][madSynthOffset + i]); - *pSampleBuffer = sampleValue; - ++pSampleBuffer; - if (readStereoSamples || isChannelCountStereo()) { - // Duplicate the 1st channel + if (1 == madSynthChannelCount) { + // MP3 frame contains a mono signal + if (readStereoSamples || isChannelCountStereo()) { + // The reader explicitly requested a stereo signal + // or the AudioSource itself provides a stereo signal. + // Mono -> Stereo: Copy 1st channel twice + for (SINT i = 0; i < synthReadCount; ++i) { + const CSAMPLE sampleValue = madScaleSampleValue( + m_madSynth.pcm.samples[0][madSynthOffset + i]); + *pSampleBuffer = sampleValue; + ++pSampleBuffer; + *pSampleBuffer = sampleValue; + ++pSampleBuffer; + } + } else { + // Mono -> Mono: Copy 1st channel + for (SINT i = 0; i < synthReadCount; ++i) { + const CSAMPLE sampleValue = madScaleSampleValue( + m_madSynth.pcm.samples[0][madSynthOffset + i]); *pSampleBuffer = sampleValue; ++pSampleBuffer; } } } else { - DEBUG_ASSERT(readStereoSamples || isChannelCountStereo()); + // MP3 frame contains a stereo signal DEBUG_ASSERT(2 == madSynthChannelCount); + // If the MP3 frame contains a stereo signal then the whole + // AudioSource must also provide 2 channels, because the + // maximum channel count of all MP3 frames is used. + DEBUG_ASSERT(2 == getChannelCount()); + // Stereo -> Stereo: Copy 1st+2nd channel for (SINT i = 0; i < synthReadCount; ++i) { *pSampleBuffer = madScaleSampleValue( m_madSynth.pcm.samples[0][madSynthOffset + i]);