SCI32: Fix audio, wave, VMD, Duck, CLUT, TGA, ZZZ, Etc patches

Specifically, audio patches are used in at least PQ:SWAT
(40103.AUD), Lighthouse (9103.AUD), and the GK2 demo (300.AUD).

7 files changed, 100 insertions, 138 deletions

diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index 921c8c91ef..d71f4353da 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -335,8 +335,7 @@ bool Resource::loadFromPatchFile() {
 		unalloc();
 		return false;
 	}
-	// Skip resourceid and header size byte
-	file.seek(2, SEEK_SET);
+	file.seek(0, SEEK_SET);
 	return loadPatch(&file);
 }
 
@@ -1401,21 +1400,56 @@ void ResourceManager::processPatch(ResourceSource *source, ResourceType resource
 		return;
 	}
 
-	byte patchType = convertResType(fileStream->readByte());
-	int32 patchDataOffset;
-	if (_volVersion < kResVersionSci2) {
-		patchDataOffset = fileStream->readByte();
-	} else if (patchType == kResourceTypeView) {
-		fileStream->seek(3, SEEK_SET);
-		patchDataOffset = fileStream->readByte() + 22 + 2;
-	} else if (patchType == kResourceTypePic) {
-		patchDataOffset = 2;
-	} else if (patchType == kResourceTypePalette) {
-		fileStream->seek(3, SEEK_SET);
-		patchDataOffset = fileStream->readByte() + 2;
+	byte patchType;
+	if (fileStream->readUint32BE() == MKTAG('R','I','F','F')) {
+		fileStream->seek(-4, SEEK_CUR);
+		patchType = kResourceTypeAudio;
 	} else {
-		patchDataOffset = 0;
+		fileStream->seek(-4, SEEK_CUR);
+		patchType = convertResType(fileStream->readByte());
+	}
+
+	enum {
+		kExtraHeaderSize    = 2, ///< extra header used in gfx resources
+		kViewHeaderSize     = 22 ///< extra header used in view resources
+	};
+
+	int32 patchDataOffset = kResourceHeaderSize;
+	if (_volVersion < kResVersionSci2) {
+		patchDataOffset += fileStream->readByte();
+	}
+#ifdef ENABLE_SCI32
+	else {
+		switch (patchType) {
+		case kResourceTypeView:
+			fileStream->seek(3, SEEK_SET);
+			patchDataOffset += fileStream->readByte() + kViewHeaderSize + kExtraHeaderSize;
+			break;
+		case kResourceTypePic:
+			patchDataOffset += kExtraHeaderSize;
+			break;
+		case kResourceTypePalette:
+			fileStream->seek(3, SEEK_SET);
+			patchDataOffset += fileStream->readByte() + kExtraHeaderSize;
+			break;
+		case kResourceTypeWave:
+		case kResourceTypeAudio:
+		case kResourceTypeAudio36:
+		case kResourceTypeVMD:
+		case kResourceTypeDuck:
+		case kResourceTypeClut:
+		case kResourceTypeTGA:
+		case kResourceTypeZZZ:
+		case kResourceTypeEtc:
+			patchDataOffset = 0;
+			break;
+		default:
+			fileStream->seek(1, SEEK_SET);
+			patchDataOffset += fileStream->readByte();
+			break;
+		}
 	}
+#endif
 
 	delete fileStream;
 
@@ -1427,15 +1461,15 @@ void ResourceManager::processPatch(ResourceSource *source, ResourceType resource
 
 	// Fixes SQ5/German, patch file special case logic taken from SCI View disassembly
 	if (patchDataOffset & 0x80) {
-		switch (patchDataOffset & 0x7F) {
+		switch ((patchDataOffset - kResourceHeaderSize) & 0x7F) {
 			case 0:
-				patchDataOffset = 24;
+				patchDataOffset = kResourceHeaderSize + 24;
 				break;
 			case 1:
-				patchDataOffset = 2;
+				patchDataOffset = kResourceHeaderSize + 2;
 				break;
 			case 4:
-				patchDataOffset = 8;
+				patchDataOffset = kResourceHeaderSize + 8;
 				break;
 			default:
 				error("Resource patch unsupported special case %X", patchDataOffset & 0x7F);
@@ -1443,15 +1477,15 @@ void ResourceManager::processPatch(ResourceSource *source, ResourceType resource
 		}
 	}
 
-	if (patchDataOffset + 2 >= fsize) {
+	if (patchDataOffset >= fsize) {
 		debug("Patching %s failed - patch starting at offset %d can't be in file of size %d",
-		      source->getLocationName().c_str(), patchDataOffset + 2, fsize);
+		      source->getLocationName().c_str(), patchDataOffset, fsize);
 		delete source;
 		return;
 	}
 
 	// Overwrite everything, because we're patching
-	newrsc = updateResource(resId, source, fsize - patchDataOffset - 2);
+	newrsc = updateResource(resId, source, fsize - patchDataOffset);
 	newrsc->_headerSize = patchDataOffset;
 	newrsc->_fileOffset = 0;
 
@@ -2084,28 +2118,17 @@ int Resource::decompress(ResVersion volVersion, Common::SeekableReadStream *file
 	_data = ptr;
 	_status = kResStatusAllocated;
 	errorNum = ptr ? dec->unpack(file, ptr, szPacked, _size) : SCI_ERROR_RESOURCE_TOO_BIG;
-	if (errorNum)
+	if (errorNum) {
 		unalloc();
-	else {
+	} else {
 		// At least Lighthouse puts sound effects in RESSCI.00n/RESSCI.PAT
 		// instead of using a RESOURCE.SFX
 		if (getType() == kResourceTypeAudio) {
-			_headerSize = ptr[1];
-			assert(_headerSize == 12);
+			const uint8 headerSize = ptr[1];
+			assert(headerSize >= 11);
 			uint32 audioSize = READ_LE_UINT32(ptr + 9);
-			assert(audioSize + _headerSize + 2 == _size);
-			_size = audioSize;
-
-			// TODO: This extra memory copying is necessary because
-			// AudioVolumeResourceSource splits the audio header from the rest
-			// of the data; fix AudioVolumeResourceSource to stop doing this and
-			// then this extra copying can be eliminated too
-			byte *dataPtr = new byte[_size];
-			_data = dataPtr;
-			_header = new byte[_headerSize];
-			memcpy(_header, ptr + 2, _headerSize);
-			memcpy(dataPtr, ptr + 2 + _headerSize, _size);
-			delete[] ptr;
+			assert(audioSize + headerSize + kResourceHeaderSize == _size);
+			_size = headerSize + audioSize;
 		}
 	}
 
diff --git a/engines/sci/resource.h b/engines/sci/resource.h
index 3e7a7a5e96..65a26ba56d 100644
--- a/engines/sci/resource.h
+++ b/engines/sci/resource.h
@@ -43,6 +43,8 @@ class SeekableReadStream;
 namespace Sci {
 
 enum {
+	kResourceHeaderSize = 2, ///< patch type + header size
+
 	/** The maximum allowed size for a compressed or decompressed resource */
 	SCI_MAX_RESOURCE_SIZE = 0x0400000
 };
diff --git a/engines/sci/resource_audio.cpp b/engines/sci/resource_audio.cpp
index b9f24cf4cb..7151c67d2e 100644
--- a/engines/sci/resource_audio.cpp
+++ b/engines/sci/resource_audio.cpp
@@ -95,7 +95,6 @@ bool Resource::loadFromAudioVolumeSCI11(Common::SeekableReadStream *file) {
 	// Check for WAVE files here
 	uint32 riffTag = file->readUint32BE();
 	if (riffTag == MKTAG('R','I','F','F')) {
-		_headerSize = 0;
 		_size = file->readUint32LE() + 8;
 		file->seek(-8, SEEK_CUR);
 		return loadFromWaveFile(file);
@@ -105,6 +104,7 @@ bool Resource::loadFromAudioVolumeSCI11(Common::SeekableReadStream *file) {
 	// Rave-resources (King's Quest 6) don't have any header at all
 	if (getType() != kResourceTypeRave) {
 		ResourceType type = _resMan->convertResType(file->readByte());
+
 		if (((getType() == kResourceTypeAudio || getType() == kResourceTypeAudio36) && (type != kResourceTypeAudio))
 			|| ((getType() == kResourceTypeSync || getType() == kResourceTypeSync36) && (type != kResourceTypeSync))) {
 			warning("Resource type mismatch loading %s", _id.toString().c_str());
@@ -112,23 +112,27 @@ bool Resource::loadFromAudioVolumeSCI11(Common::SeekableReadStream *file) {
 			return false;
 		}
 
-		_headerSize = file->readByte();
+		const uint8 headerSize = file->readByte();
 
 		if (type == kResourceTypeAudio) {
-			if (_headerSize != 7 && _headerSize != 11 && _headerSize != 12) {
-				warning("Unsupported audio header size %d", _headerSize);
+			if (headerSize != 7 && headerSize != 11 && headerSize != 12) {
+				warning("Unsupported audio header size %d", headerSize);
 				unalloc();
 				return false;
 			}
 
-			if (_headerSize != 7) { // Size is defined already from the map
+			if (headerSize != 7) { // Size is defined already from the map
 				// Load sample size
 				file->seek(7, SEEK_CUR);
-				_size = file->readUint32LE();
+				_size = file->readUint32LE() + headerSize + kResourceHeaderSize;
 				assert(!file->err() && !file->eos());
-				// Adjust offset to point at the header data again
+				// Adjust offset to point at the beginning of the audio file
+				// again
 				file->seek(-11, SEEK_CUR);
 			}
+
+			// SOL audio files are designed to require the resource header
+			file->seek(-2, SEEK_CUR);
 		}
 	}
 	return loadPatch(file);
diff --git a/engines/sci/sound/audio.cpp b/engines/sci/sound/audio.cpp
index e470b315cc..273c4c1a0d 100644
--- a/engines/sci/sound/audio.cpp
+++ b/engines/sci/sound/audio.cpp
@@ -397,12 +397,13 @@ Audio::RewindableAudioStream *AudioPlayer::getAudioStream(uint32 number, uint32
 #endif
 	} else {
 		// Original source file
-		if (audioRes->_headerSize > 0) {
+		if ((audioRes->getUint8At(0) & 0x7f) == kResourceTypeAudio && audioRes->getUint32BEAt(2) == MKTAG('S','O','L',0)) {
 			// SCI1.1
-			Common::MemoryReadStream headerStream(audioRes->_header, audioRes->_headerSize, DisposeAfterUse::NO);
+			const uint8 headerSize = audioRes->getUint8At(1);
+			Common::MemoryReadStream headerStream = audioRes->subspan(kResourceHeaderSize, headerSize).toStream();
 
-			if (readSOLHeader(&headerStream, audioRes->_headerSize, size, _audioRate, audioFlags, audioRes->size())) {
-				Common::MemoryReadStream dataStream(audioRes->toStream());
+			if (readSOLHeader(&headerStream, headerSize, size, _audioRate, audioFlags, audioRes->size())) {
+				Common::MemoryReadStream dataStream(audioRes->subspan(kResourceHeaderSize + headerSize).toStream());
 				data = readSOLAudio(&dataStream, size, audioFlags, flags);
 			}
 		} else if (audioRes->size() > 4 && audioRes->getUint32BEAt(0) == MKTAG('R','I','F','F')) {
diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp
index ca2402c859..69a83ebc8e 100644
--- a/engines/sci/sound/audio32.cpp
+++ b/engines/sci/sound/audio32.cpp
@@ -46,10 +46,6 @@ namespace Sci {
 bool detectSolAudio(Common::SeekableReadStream &stream) {
 	const size_t initialPosition = stream.pos();
 
-// TODO: Resource manager for audio resources reads past the
-// header so even though this is the detection algorithm
-// in SSCI, ScummVM can't use it
-#if 0
 	byte header[6];
 	if (stream.read(header, sizeof(header)) != sizeof(header)) {
 		stream.seek(initialPosition);
@@ -58,26 +54,11 @@ bool detectSolAudio(Common::SeekableReadStream &stream) {
 
 	stream.seek(initialPosition);
 
-	if (header[0] != 0x8d || READ_BE_UINT32(header + 2) != MKTAG('S', 'O', 'L', 0)) {
+	if ((header[0] & 0x7f) != kResourceTypeAudio || READ_BE_UINT32(header + 2) != MKTAG('S', 'O', 'L', 0)) {
 		return false;
 	}
 
 	return true;
-#else
-	byte header[4];
-	if (stream.read(header, sizeof(header)) != sizeof(header)) {
-		stream.seek(initialPosition);
-		return false;
-	}
-
-	stream.seek(initialPosition);
-
-	if (READ_BE_UINT32(header) != MKTAG('S', 'O', 'L', 0)) {
-		return false;
-	}
-
-	return true;
-#endif
 }
 
 bool detectWaveAudio(Common::SeekableReadStream &stream) {
@@ -723,11 +704,10 @@ uint16 Audio32::play(int16 channelIndex, const ResourceId resourceId, const bool
 		_monitoredChannelIndex = channelIndex;
 	}
 
-	Common::MemoryReadStream headerStream(resource->_header, resource->_headerSize, DisposeAfterUse::NO);
 	Common::SeekableReadStream *dataStream = channel.resourceStream = resource->makeStream();
 
-	if (detectSolAudio(headerStream)) {
-		channel.stream = makeSOLStream(&headerStream, dataStream, DisposeAfterUse::NO);
+	if (detectSolAudio(*dataStream)) {
+		channel.stream = makeSOLStream(dataStream, DisposeAfterUse::NO);
 	} else if (detectWaveAudio(*dataStream)) {
 		channel.stream = Audio::makeWAVStream(dataStream, DisposeAfterUse::NO);
 	} else {
diff --git a/engines/sci/sound/decoders/sol.cpp b/engines/sci/sound/decoders/sol.cpp
index 2dbe98c9e0..973ebf309c 100644
--- a/engines/sci/sound/decoders/sol.cpp
+++ b/engines/sci/sound/decoders/sol.cpp
@@ -25,8 +25,9 @@
 #include "audio/decoders/raw.h"
 #include "common/substream.h"
 #include "common/util.h"
-#include "engines/sci/sci.h"
-#include "engines/sci/sound/decoders/sol.h"
+#include "sci/sci.h"
+#include "sci/sound/decoders/sol.h"
+#include "sci/resource.h"
 
 namespace Sci {
 
@@ -127,16 +128,11 @@ static void deDPCM8Stereo(int16 *out, Common::ReadStream &audioStream, uint32 nu
 # pragma mark -
 
 template<bool STEREO, bool S16BIT>
-SOLStream<STEREO, S16BIT>::SOLStream(Common::SeekableReadStream *stream, const DisposeAfterUse::Flag disposeAfterUse, const int32 dataOffset, const uint16 sampleRate, const int32 rawDataSize) :
+SOLStream<STEREO, S16BIT>::SOLStream(Common::SeekableReadStream *stream, const DisposeAfterUse::Flag disposeAfterUse, const uint16 sampleRate, const int32 rawDataSize) :
 	_stream(stream, disposeAfterUse),
-	_dataOffset(dataOffset),
 	_sampleRate(sampleRate),
 	// SSCI aligns the size of SOL data to 32 bits
 	_rawDataSize(rawDataSize & ~3) {
-		// TODO: This is not valid for stereo SOL files, which
-		// have interleaved L/R compression so need to store the
-		// carried values for each channel separately. See
-		// 60900.aud from Lighthouse for an example stereo file
 		if (S16BIT) {
 			_dpcmCarry16.l = _dpcmCarry16.r = 0;
 		} else {
@@ -166,7 +162,7 @@ bool SOLStream<STEREO, S16BIT>::seek(const Audio::Timestamp &where) {
 		_dpcmCarry8.l = _dpcmCarry8.r = 0x80;
 	}
 
-	return _stream->seek(_dataOffset, SEEK_SET);
+	return _stream->seek(0, SEEK_SET);
 }
 
 template <bool STEREO, bool S16BIT>
@@ -227,72 +223,35 @@ bool SOLStream<STEREO, S16BIT>::rewind() {
 }
 
 Audio::SeekableAudioStream *makeSOLStream(Common::SeekableReadStream *stream, DisposeAfterUse::Flag disposeAfterUse) {
-
-	// TODO: Might not be necessary? Makes seeking work, but
-	// not sure if audio is ever actually seeked in SSCI.
-	const int32 initialPosition = stream->pos();
+	int32 initialPosition = stream->pos();
 
 	byte header[6];
 	if (stream->read(header, sizeof(header)) != sizeof(header)) {
+		stream->seek(initialPosition, SEEK_SET);
 		return nullptr;
 	}
 
-	if (header[0] != 0x8d || READ_BE_UINT32(header + 2) != MKTAG('S', 'O', 'L', 0)) {
+	if ((header[0] & 0x7f) != kResourceTypeAudio || READ_BE_UINT32(header + 2) != MKTAG('S', 'O', 'L', 0)) {
+		stream->seek(initialPosition, SEEK_SET);
 		return nullptr;
 	}
 
-	const uint8 headerSize = header[1];
+	const uint8 headerSize = header[1] + /* resource header */ 2;
 	const uint16 sampleRate = stream->readUint16LE();
 	const byte flags = stream->readByte();
 	const uint32 dataSize = stream->readUint32LE();
 
-	if (flags & kCompressed) {
-		if (flags & kStereo && flags & k16Bit) {
-			return new SOLStream<true, true>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, headerSize, sampleRate, dataSize);
-		} else if (flags & kStereo) {
-			return new SOLStream<true, false>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, headerSize, sampleRate, dataSize);
-		} else if (flags & k16Bit) {
-			return new SOLStream<false, true>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, headerSize, sampleRate, dataSize);
-		} else {
-			return new SOLStream<false, false>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, headerSize, sampleRate, dataSize);
-		}
-	}
-
-	byte rawFlags = Audio::FLAG_LITTLE_ENDIAN;
-	if (flags & k16Bit) {
-		rawFlags |= Audio::FLAG_16BITS;
-	} else {
-		rawFlags |= Audio::FLAG_UNSIGNED;
-	}
-
-	if (flags & kStereo) {
-		rawFlags |= Audio::FLAG_STEREO;
-	}
-
-	return Audio::makeRawStream(new Common::SeekableSubReadStream(stream, initialPosition + headerSize, initialPosition + headerSize + dataSize, disposeAfterUse), sampleRate, rawFlags, disposeAfterUse);
-}
-
-// TODO: This needs to be removed when resource manager is fixed
-// to not split audio into two parts
-Audio::SeekableAudioStream *makeSOLStream(Common::SeekableReadStream *headerStream, Common::SeekableReadStream *dataStream, DisposeAfterUse::Flag disposeAfterUse) {
-
-	if (headerStream->readUint32BE() != MKTAG('S', 'O', 'L', 0)) {
-		return nullptr;
-	}
-
-	const uint16 sampleRate = headerStream->readUint16LE();
-	const byte flags = headerStream->readByte();
-	const int32 dataSize = headerStream->readSint32LE();
+	initialPosition += headerSize;
 
 	if (flags & kCompressed) {
 		if (flags & kStereo && flags & k16Bit) {
-			return new SOLStream<true, true>(dataStream, disposeAfterUse, 0, sampleRate, dataSize);
+			return new SOLStream<true, true>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, sampleRate, dataSize);
 		} else if (flags & kStereo) {
-			return new SOLStream<true, false>(dataStream, disposeAfterUse, 0, sampleRate, dataSize);
+			return new SOLStream<true, false>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, sampleRate, dataSize);
 		} else if (flags & k16Bit) {
-			return new SOLStream<false, true>(dataStream, disposeAfterUse, 0, sampleRate, dataSize);
+			return new SOLStream<false, true>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, sampleRate, dataSize);
 		} else {
-			return new SOLStream<false, false>(dataStream, disposeAfterUse, 0, sampleRate, dataSize);
+			return new SOLStream<false, false>(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), disposeAfterUse, sampleRate, dataSize);
 		}
 	}
 
@@ -307,6 +266,6 @@ Audio::SeekableAudioStream *makeSOLStream(Common::SeekableReadStream *headerStre
 		rawFlags |= Audio::FLAG_STEREO;
 	}
 
-	return Audio::makeRawStream(dataStream, sampleRate, rawFlags, disposeAfterUse);
+	return Audio::makeRawStream(new Common::SeekableSubReadStream(stream, initialPosition, initialPosition + dataSize, disposeAfterUse), sampleRate, rawFlags, disposeAfterUse);
 }
 }
diff --git a/engines/sci/sound/decoders/sol.h b/engines/sci/sound/decoders/sol.h
index 31914c0926..80a2181889 100644
--- a/engines/sci/sound/decoders/sol.h
+++ b/engines/sci/sound/decoders/sol.h
@@ -42,11 +42,6 @@ private:
 	Common::DisposablePtr<Common::SeekableReadStream> _stream;
 
 	/**
-	 * Start offset of the audio data in the read stream.
-	 */
-	int32 _dataOffset;
-
-	/**
 	 * Sample rate of audio data.
 	 */
 	uint16 _sampleRate;
@@ -79,11 +74,9 @@ private:
 	virtual bool rewind() override;
 
 public:
-	SOLStream(Common::SeekableReadStream *stream, const DisposeAfterUse::Flag disposeAfterUse, const int32 dataOffset, const uint16 sampleRate, const int32 rawDataSize);
+	SOLStream(Common::SeekableReadStream *stream, const DisposeAfterUse::Flag disposeAfterUse, const uint16 sampleRate, const int32 rawDataSize);
 };
 
 Audio::SeekableAudioStream *makeSOLStream(Common::SeekableReadStream *stream, DisposeAfterUse::Flag disposeAfterUse);
-
-Audio::SeekableAudioStream *makeSOLStream(Common::SeekableReadStream *headerStream, Common::SeekableReadStream *dataStream, DisposeAfterUse::Flag disposeAfterUse);
 }
 #endif