Skip to content

Commit

Permalink
Fix the of-by-one issue that leads to a decoding error when decoding …
Browse files Browse the repository at this point in the history
…the last frame.

This is fixed by skipping the first frame likely containing the MP3 Info Tag explicit during tryOpen():
Explain why this can't be made conditional without handling the offset of the analysis data.
  • Loading branch information
daschuer committed Jan 1, 2023
1 parent 0594a62 commit b2353a6
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions src/sources/soundsourcemp3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@ SoundSource::OpenResult SoundSourceMp3::tryOpen(
quint64 sumBitrateFrames = 0; // nominator
quint64 cntBitrateFrames = 0; // denominator

// Normally mp3 files starts with an extra frame of silence containing
// engoder infos called "LAME Tag". Since the early days of we skip the
// first frame uncoditionally, to not have these extra portion of silence
// in the track. This has the isseue that with files without this frame real
// samples are dropped.
// Since this issue exists since the early days of Mixxx the analysis data
// is affected by the offset. Fixing this without fixing the amalysis data
// will silently invalidate analysis, cues and loops.
// Note: A relates issue with not accurate seeks has been fixed in Mixxx 2.1.0 2015
// https://github.com/mixxxdj/mixxx/pull/411
bool mp3InfoTagSkipped = false;

mad_header madHeader;
mad_header_init(&madHeader);

Expand Down Expand Up @@ -273,6 +285,13 @@ SoundSource::OpenResult SoundSourceMp3::tryOpen(
continue;
}

if (!mp3InfoTagSkipped) {
// We assume that the first frame contains the mp3 info frame
// which needs to be skipped
mp3InfoTagSkipped = true;
continue;
}

const audio::ChannelCount madChannelCount(MAD_NCHANNELS(&madHeader));
if (madChannelCount.isValid()) {
if (maxChannelCount.isValid() && (madChannelCount != maxChannelCount)) {
Expand Down Expand Up @@ -464,12 +483,7 @@ void SoundSourceMp3::restartDecoding(
mad_synth_mute(&m_madSynth);
}

if (decodeFrameHeader(&m_madFrame.header, &m_madStream, false)) {
m_curFrameIndex = seekFrame.frameIndex;
} else {
// Failure -> Seek to EOF
m_curFrameIndex = frameIndexMax();
}
m_curFrameIndex = seekFrame.frameIndex;
}

void SoundSourceMp3::addSeekFrame(
Expand Down

0 comments on commit b2353a6

Please sign in to comment.