From 41506201b908347ae7fba88cfdd3bcc53e6b4056 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Sat, 9 Sep 2017 22:32:55 -0500 Subject: AUDIO: Fix incorrect reading of DK3 ADPCM audio data Previously, _topNibble was not reset at the beginning of a new audio block, and the alignment byte at the end of odd blocks was being read as audio data, which caused audible clicks and out-of-bounds sample generation. There may have also been read errors related to the use of continue/break keywords inside of a macro wrapped with do-while(0). The introduction of partial block reads in this code when it was converted from ffmpeg to a ReadStream interface was also confusing and somewhat inefficient (calling SeekableReadStream::pos frequently), so this code has been refactored for clarity and to improve efficiency by reducing the number of virtual calls. Error detection has also been improved somewhat by ensuring that there are enough bytes to read a block header, and that the step indexes in the header are within the valid range. --- audio/decoders/adpcm.cpp | 80 +++++++++++++++++++++++++++---------------- audio/decoders/adpcm_intern.h | 8 ----- 2 files changed, 51 insertions(+), 37 deletions(-) (limited to 'audio') diff --git a/audio/decoders/adpcm.cpp b/audio/decoders/adpcm.cpp index ffb61a49c0..7f578ddbae 100644 --- a/audio/decoders/adpcm.cpp +++ b/audio/decoders/adpcm.cpp @@ -334,67 +334,89 @@ int MS_ADPCMStream::readBuffer(int16 *buffer, const int numSamples) { #pragma mark - - -#define DK3_READ_NIBBLE() \ +#define DK3_READ_NIBBLE(channelNo) \ do { \ if (_topNibble) { \ _nibble = _lastByte >> 4; \ _topNibble = false; \ } else { \ - if (_stream->pos() >= _endpos) \ - break; \ - if ((_stream->pos() % _blockAlign) == 0) \ - continue; \ _lastByte = _stream->readByte(); \ _nibble = _lastByte & 0xf; \ _topNibble = true; \ + --blockBytesLeft; \ + --audioBytesLeft; \ } \ -} while (0) - + decodeIMA(_nibble, channelNo); \ +} while(0) int DK3_ADPCMStream::readBuffer(int16 *buffer, const int numSamples) { + assert((numSamples % 4) == 0); + + const uint startOffset = _stream->pos() % _blockAlign; + uint audioBytesLeft = _endpos - _stream->pos(); + uint blockBytesLeft; + if (startOffset != 0) { + blockBytesLeft = _blockAlign - startOffset; + } else { + blockBytesLeft = 0; + } + int samples = 0; + while (samples < numSamples && audioBytesLeft) { + if (blockBytesLeft == 0) { + blockBytesLeft = MIN(_blockAlign, audioBytesLeft); + _topNibble = false; - assert((numSamples % 4) == 0); + if (blockBytesLeft < 16) { + warning("Truncated DK3 ADPCM block header"); + break; + } + + _stream->skip(2); + const uint16 rate = _stream->readUint16LE(); + assert(rate == getRate()); + _stream->skip(6); - while (samples < numSamples && !_stream->eos() && _stream->pos() < _endpos) { - if ((_stream->pos() % _blockAlign) == 0) { - _stream->readUint16LE(); // Unknown - uint16 rate = _stream->readUint16LE(); // Copy of rate - _stream->skip(6); // Unknown // Get predictor for both sum/diff channels _status.ima_ch[0].last = _stream->readSint16LE(); _status.ima_ch[1].last = _stream->readSint16LE(); + // Get index for both sum/diff channels _status.ima_ch[0].stepIndex = _stream->readByte(); _status.ima_ch[1].stepIndex = _stream->readByte(); + assert(_status.ima_ch[0].stepIndex < ARRAYSIZE(_imaTable)); + assert(_status.ima_ch[1].stepIndex < ARRAYSIZE(_imaTable)); - if (_stream->eos()) - break; - - // Sanity check - assert(rate == getRate()); + blockBytesLeft -= 16; + audioBytesLeft -= 16; } - DK3_READ_NIBBLE(); - decodeIMA(_nibble, 0); + DK3_READ_NIBBLE(0); + DK3_READ_NIBBLE(1); - DK3_READ_NIBBLE(); - decodeIMA(_nibble, 1); + *buffer++ = _status.ima_ch[0].last + _status.ima_ch[1].last; + *buffer++ = _status.ima_ch[0].last - _status.ima_ch[1].last; - buffer[samples++] = _status.ima_ch[0].last + _status.ima_ch[1].last; - buffer[samples++] = _status.ima_ch[0].last - _status.ima_ch[1].last; + DK3_READ_NIBBLE(0); - DK3_READ_NIBBLE(); - decodeIMA(_nibble, 0); + *buffer++ = _status.ima_ch[0].last + _status.ima_ch[1].last; + *buffer++ = _status.ima_ch[0].last - _status.ima_ch[1].last; - buffer[samples++] = _status.ima_ch[0].last + _status.ima_ch[1].last; - buffer[samples++] = _status.ima_ch[0].last - _status.ima_ch[1].last; + samples += 4; + + // if the last sample of a block ends on an odd byte, the encoder adds + // an extra alignment byte + if (!_topNibble && blockBytesLeft == 1) { + _stream->skip(1); + --blockBytesLeft; + --audioBytesLeft; + } } return samples; } +#undef DK3_READ_NIBBLE #pragma mark - diff --git a/audio/decoders/adpcm_intern.h b/audio/decoders/adpcm_intern.h index f4a708c3fc..4b7b3481f4 100644 --- a/audio/decoders/adpcm_intern.h +++ b/audio/decoders/adpcm_intern.h @@ -229,20 +229,12 @@ private: // Based on FFmpeg's decoder and http://wiki.multimedia.cx/index.php?title=Duck_DK3_IMA_ADPCM class DK3_ADPCMStream : public Ima_ADPCMStream { -protected: - - void reset() { - Ima_ADPCMStream::reset(); - _topNibble = false; - } - public: DK3_ADPCMStream(Common::SeekableReadStream *stream, DisposeAfterUse::Flag disposeAfterUse, uint32 size, int rate, int channels, uint32 blockAlign) : Ima_ADPCMStream(stream, disposeAfterUse, size, rate, channels, blockAlign) { // DK3 only works as a stereo stream assert(channels == 2); - _topNibble = false; } virtual int readBuffer(int16 *buffer, const int numSamples); -- cgit v1.2.3