5 files changed, 246 insertions, 59 deletions

diff --git a/image/codecs/indeo/indeo.cpp b/image/codecs/indeo/indeo.cpp
index f420069139..34377537d7 100644
--- a/image/codecs/indeo/indeo.cpp
+++ b/image/codecs/indeo/indeo.cpp
@@ -444,7 +444,8 @@ IVI45DecContext::IVI45DecContext() : _gb(nullptr), _frameNum(0), _frameType(0),
 		_bRefBuf(0), _rvmapSel(0), _inImf(false), _inQ(false), _picGlobQuant(0),
 		_unknown1(0), _gopHdrSize(0), _gopFlags(0), _lockWord(0), _hasBFrames(false),
 		_hasTransp(false), _usesTiling(false), _usesHaar(false), _usesFullpel(false),
-		_gopInvalid(false), _isIndeo4(false), _pFrame(nullptr), _gotPFrame(false) {
+		_gopInvalid(false), _isIndeo4(false), _transKeyColor(0), _pFrame(nullptr),
+		_gotPFrame(false) {
 	Common::fill(&_bufInvalid[0], &_bufInvalid[4], 0);
 	Common::copy(&_ff_ivi_rvmap_tabs[0], &_ff_ivi_rvmap_tabs[9], &_rvmapTabs[0]);
 
@@ -479,18 +480,18 @@ IndeoDecoderBase::IndeoDecoderBase(uint16 width, uint16 height, uint bitsPerPixe
 		break;
 	}
 
-	_surface = new Graphics::Surface();
-	_surface->create(width, height, _pixelFormat);
-	_surface->fillRect(Common::Rect(0, 0, width, height), (bitsPerPixel == 32) ? 0xff : 0);
+	_surface.create(width, height, _pixelFormat);
+	_surface.fillRect(Common::Rect(0, 0, width, height), (bitsPerPixel == 32) ? 0xff : 0);
 	_ctx._bRefBuf = 3; // buffer 2 is used for scalability mode
 }
 
 IndeoDecoderBase::~IndeoDecoderBase() {
-	_surface->free();
-	delete _surface;
+	_surface.free();
 	IVIPlaneDesc::freeBuffers(_ctx._planes);
 	if (_ctx._mbVlc._custTab._table)
 		_ctx._mbVlc._custTab.freeVlc();
+	if (_ctx._transVlc._custTab._table)
+		_ctx._transVlc._custTab.freeVlc();
 
 	delete _ctx._pFrame;
 }
@@ -555,7 +556,7 @@ int IndeoDecoderBase::decodeIndeoFrame() {
 	if (!isNonNullFrame())
 		return 0;
 
-	assert(_ctx._planes[0]._width <= _surface->w && _ctx._planes[0]._height <= _surface->h);
+	assert(_ctx._planes[0]._width <= _surface.w && _ctx._planes[0]._height <= _surface.h);
 	result = frame->setDimensions(_ctx._planes[0]._width, _ctx._planes[0]._height);
 	if (result < 0)
 		return result;
@@ -575,11 +576,22 @@ int IndeoDecoderBase::decodeIndeoFrame() {
 	outputPlane(&_ctx._planes[2], frame->_data[1], frame->_linesize[1]);
 	outputPlane(&_ctx._planes[1], frame->_data[2], frame->_linesize[2]);
 
+	// Merge the planes into the final surface
+	YUVToRGBMan.convert410(&_surface, Graphics::YUVToRGBManager::kScaleITU,
+		frame->_data[0], frame->_data[1], frame->_data[2], frame->_width, frame->_height,
+		frame->_width, frame->_width);
+
+	if (_ctx._hasTransp)
+		decodeTransparency();
+
 	// If the bidirectional mode is enabled, next I and the following P
 	// frame will be sent together. Unfortunately the approach below seems
 	// to be the only way to handle the B-frames mode.
 	// That's exactly the same Intel decoders do.
 	if (_ctx._isIndeo4 && _ctx._frameType == IVI4_FRAMETYPE_INTRA) {
+		// TODO: It appears from the reference decoder that this should be
+		// aligning GetBits to a 32-bit boundary before reading again?
+
 		int left;
 
 		// skip version string
@@ -595,19 +607,9 @@ int IndeoDecoderBase::decodeIndeoFrame() {
 		}
 	}
 
-	// Merge the planes into the final surface
-	Graphics::Surface s = _surface->getSubArea(Common::Rect(0, 0, _surface->w, _surface->h));
-	YUVToRGBMan.convert410(&s, Graphics::YUVToRGBManager::kScaleITU,
-		frame->_data[0], frame->_data[1], frame->_data[2], frame->_width, frame->_height,
-		frame->_width, frame->_width);
-
 	// Free the now un-needed frame data
 	frame->freeFrame();
 
-	// If there's any transparency data, decode it
-	if (_ctx._hasTransp)
-		decodeTransparency();
-
 	return 0;
 }
 
diff --git a/image/codecs/indeo/indeo.h b/image/codecs/indeo/indeo.h
index dcb7330318..d9740ecf61 100644
--- a/image/codecs/indeo/indeo.h
+++ b/image/codecs/indeo/indeo.h
@@ -398,6 +398,7 @@ public:
 
 	IVIHuffTab		_mbVlc;			///< current macroblock table descriptor
 	IVIHuffTab		_blkVlc;		///< current block table descriptor
+	IVIHuffTab		_transVlc;		///< current transparency table descriptor
 
 	uint8			_rvmapSel;
 	bool			_inImf;
@@ -419,6 +420,7 @@ public:
 	int				_bufInvalid[4];
 
 	bool			_isIndeo4;
+	uint32			_transKeyColor;
 
 	AVFrame *		_pFrame;
 	bool			_gotPFrame;
@@ -518,7 +520,7 @@ private:
 protected:
 	IVI45DecContext _ctx;
 	Graphics::PixelFormat _pixelFormat;
-	Graphics::Surface *_surface;
+	Graphics::Surface _surface;
 
 	/**
 	 *  Scan patterns shared between indeo4 and indeo5
@@ -566,7 +568,7 @@ protected:
 	/**
 	 * Decodes optional transparency data within Indeo frames
 	 */
-	virtual void decodeTransparency() {}
+	virtual int decodeTransparency() { return -1; }
 
 	/**
 	 * Decodes the Indeo frame from the bit reader already
diff --git a/image/codecs/indeo4.cpp b/image/codecs/indeo4.cpp
index ead1d3a814..a4eba85f49 100644
--- a/image/codecs/indeo4.cpp
+++ b/image/codecs/indeo4.cpp
@@ -26,7 +26,10 @@
  * written, produced, and directed by Alan Smithee
  */
 
+#include "common/algorithm.h"
+#include "common/debug.h"
 #include "common/memstream.h"
+#include "common/rect.h"
 #include "common/textconsole.h"
 #include "graphics/yuv_to_rgb.h"
 #include "image/codecs/indeo4.h"
@@ -86,7 +89,7 @@ const Graphics::Surface *Indeo4Decoder::decodeFrame(Common::SeekableReadStream &
 	_ctx._frameData = nullptr;
 	_ctx._frameSize = 0;
 
-	return (err < 0) ? nullptr : _surface;
+	return (err < 0) ? nullptr : &_surface;
 }
 
 int Indeo4Decoder::decodePictureHeader() {
@@ -109,6 +112,12 @@ int Indeo4Decoder::decodePictureHeader() {
 		_ctx._hasBFrames = true;
 
 	_ctx._hasTransp = _ctx._gb->getBit();
+	if (_ctx._hasTransp && _surface.format.aBits() == 0) {
+		// Surface is 4 bytes per pixel, but only RGB. So promote the
+		// surface to full RGBA, and convert all the existing pixels
+		_pixelFormat = Graphics::PixelFormat(4, 8, 8, 8, 8, 24, 16, 8, 0);
+		_surface.convertToInPlace(_pixelFormat);
+	}
 
 	// unknown bit: Mac decoder ignores this bit, XANIM returns error
 	if (_ctx._gb->getBit()) {
@@ -595,52 +604,221 @@ int Indeo4Decoder::decodeMbInfo(IVIBandDesc *band, IVITile *tile) {
 	return 0;
 }
 
-void Indeo4Decoder::decodeTransparency() {
-	// FIXME: Since I don't currently know how to decode the transparency layer,
-	// I'm currently doing a hack where I take the color of the top left corner,
-	// and mark the range of pixels of that color from the start and end of
-	// each line as transparent
-	assert(_surface->format.bytesPerPixel == 4);
-	byte r, g, b;
+int Indeo4Decoder::decodeRLETransparency(VLC_TYPE (*table)[2]) {
+	const uint32 startPos = _ctx._gb->pos();
 
-	if (_surface->format.aBits() == 0) {
-		// Surface is 4 bytes per pixel, but only RGB. So promote the
-		// surface to full RGBA, and convert all the existing pixels
-		Graphics::PixelFormat oldFormat = _pixelFormat;
-		_pixelFormat = Graphics::PixelFormat(4, 8, 8, 8, 8, 24, 16, 8, 0);
-		_surface->format = _pixelFormat;
+	_ctx._gb->align();
+
+	bool runIsOpaque = _ctx._gb->getBit();
+	bool nextRunIsOpaque = !runIsOpaque;
+
+	uint32 *pixel = (uint32 *)_surface.getPixels();
+	const int surfacePixelPitch = _surface.pitch / _surface.format.bytesPerPixel;
+	const int surfacePadding = surfacePixelPitch - _surface.w;
+	const uint32 *endOfVisibleRow = pixel + _surface.w;
+	const uint32 *endOfVisibleArea = pixel + surfacePixelPitch * _surface.h - surfacePadding;
+
+	const int codecAlignedWidth = (_surface.w + 31) & ~31;
+	const int codecPaddingSize = codecAlignedWidth - _surface.w;
+
+	int numPixelsToRead = codecAlignedWidth * _surface.h;
+	int numPixelsToSkip = 0;
+	while (numPixelsToRead > 0) {
+		int value = _ctx._gb->getVLC2<1>(table, IVI_VLC_BITS);
+
+		if (value == -1) {
+			warning("Transparency VLC code read failed");
+			return -1;
+		}
+
+		if (value == 0) {
+			value = 255;
+			nextRunIsOpaque = runIsOpaque;
+		}
+
+		numPixelsToRead -= value;
+
+		debugN(9, "%d%s ", value, runIsOpaque ? "O" : "T");
+
+		// The rest of the transparency data must be consumed but it will not
+		// participate in writing any more pixels
+		if (pixel == endOfVisibleArea) {
+			debug(5, "Indeo4: Done writing transparency, but still need to consume %d pixels", numPixelsToRead + value);
+			continue;
+		}
 
-		for (int y = 0; y < _surface->h; ++y) {
-			uint32 *lineP = (uint32 *)_surface->getBasePtr(0, y);
-			for (int x = 0; x < _surface->w; ++x, ++lineP) {
-				oldFormat.colorToRGB(*lineP, r, g, b);
-				*lineP = _pixelFormat.ARGBToColor(0xff, r, g, b);
+		// If a run ends in the padding area of a row, the next run needs to
+		// be partially consumed by the remaining pixels of the padding area
+		if (numPixelsToSkip) {
+			value -= numPixelsToSkip;
+			if (value < 0) {
+				numPixelsToSkip = -value;
+				value = 0;
+			} else {
+				numPixelsToSkip = 0;
 			}
 		}
-	} else {
-		// Working on a frame when the surface is already RGBA. In which case,
-		// start of by defaulting all pixels of the frame to fully opaque
-		for (int y = 0; y < _surface->h; ++y) {
-			uint32 *lineP = (uint32 *)_surface->getBasePtr(0, y);
-			for (int x = 0; x < _surface->w; ++x, ++lineP)
-				*lineP |= 0xff;
+
+		while (value > 0) {
+			const int length = MIN<int>(value, endOfVisibleRow - pixel);
+			if (!runIsOpaque) {
+				Common::fill(pixel, pixel + length, _ctx._transKeyColor);
+			}
+			value -= length;
+			pixel += length;
+
+			if (pixel == endOfVisibleRow) {
+				pixel += surfacePadding;
+				endOfVisibleRow += surfacePixelPitch;
+				value -= codecPaddingSize;
+
+				if (value < 0) {
+					numPixelsToSkip = -value;
+					break;
+				}
+
+				if (pixel == endOfVisibleArea) {
+					break;
+				}
+			}
+		}
+
+		runIsOpaque = nextRunIsOpaque;
+		nextRunIsOpaque = !runIsOpaque;
+	}
+
+	debugN(9, "\n");
+
+	if (numPixelsToRead != 0) {
+		warning("Wrong number of transparency pixels read; delta = %d", numPixelsToRead);
+	}
+
+	_ctx._gb->align();
+
+	return (_ctx._gb->pos() - startPos) / 8;
+}
+
+int Indeo4Decoder::decodeTransparency() {
+	if (_ctx._gb->getBits(2) != 3 || _ctx._gb->getBits(3) != 0) {
+		warning("Invalid transparency marker");
+		return -1;
+	}
+
+	Common::Rect drawRect;
+
+	for (int numRects = _ctx._gb->getBits(8); numRects; --numRects) {
+		const int x1 = _ctx._gb->getBits(16);
+		const int y1 = _ctx._gb->getBits(16);
+		const int x2 = x1 + _ctx._gb->getBits(16);
+		const int y2 = y1 + _ctx._gb->getBits(16);
+		drawRect.extend(Common::Rect(x1, y1, x2, y2));
+	}
+
+	debug(4, "Indeo4: Transparency rect is (%d, %d, %d, %d)", drawRect.left, drawRect.top, drawRect.right, drawRect.bottom);
+
+	if (_ctx._gb->getBit()) { /* @350 */
+		/* @358 */
+		_ctx._transKeyColor = _surface.format.ARGBToColor(0, _ctx._gb->getBits(8), _ctx._gb->getBits(8), _ctx._gb->getBits(8));
+		debug(4, "Indeo4: Key color is %08x", _ctx._transKeyColor);
+		/* @477 */
+	}
+
+	if (_ctx._gb->getBit() == 0) { /* @4D9 */
+		warning("Invalid transparency band?");
+		return -1;
+	}
+
+	IVIHuffDesc huffDesc;
+
+	const int numHuffRows = huffDesc._numRows = _ctx._gb->getBits(4);
+	if (numHuffRows == 0 || numHuffRows > IVI_VLC_BITS - 1) {
+		warning("Invalid codebook row count %d", numHuffRows);
+		return -1;
+	}
+
+	for (int i = 0; i < numHuffRows; ++i) {
+		huffDesc._xBits[i] = _ctx._gb->getBits(4);
+	}
+
+	/* @5E2 */
+	_ctx._gb->align();
+
+	IVIHuffTab &huffTable = _ctx._transVlc;
+
+	if (huffDesc.huffDescCompare(&huffTable._custDesc) || !huffTable._custTab._table) {
+		if (huffTable._custTab._table) {
+			huffTable._custTab.freeVlc();
+		}
+
+		huffTable._custDesc = huffDesc;
+		huffTable._tabSel = 7;
+		huffTable._tab = &huffTable._custTab;
+		if (huffTable._custDesc.createHuffFromDesc(huffTable._tab, false)) {
+			// reset faulty description
+			huffTable._custDesc._numRows = 0;
+			warning("Error while initializing transparency VLC table");
+			return -1;
 		}
 	}
 
-	// Use the top-left pixel as the key color, and figure out the
-	// equivalent value as fully transparent
-	uint32 keyColor = *(const uint32 *)_surface->getPixels();
-	uint32 transColor = keyColor & ~0xff;
+	// FIXME: The transparency plane can be split, apparently for local decoding
+	// mode (y459.avi in Titanic has the scalable flag and its transparency
+	// plane seems to be decoded successfully, so the split transparency plane
+	// does not seem to be related to scaling mode). This adds complexity to the
+	// implementation, so avoid supporting unless it turns out to actually be
+	// necessary for correct decoding of game videos.
+	assert(!_ctx._usesTiling);
+
+	assert(_surface.format.bytesPerPixel == 4);
+	assert((_surface.pitch % 4) == 0);
+
+	const uint32 startByte = _ctx._gb->pos() / 8;
+
+	/* @68D */
+	const bool useFillTransparency = _ctx._gb->getBit();
+	if (useFillTransparency) {
+		/* @6F2 */
+		const bool runIsOpaque = _ctx._gb->getBit();
+		if (!runIsOpaque) {
+			// It should only be necessary to draw transparency here since the
+			// data from the YUV planes gets drawn to the output surface on each
+			// frame, which resets the surface pixels to be fully opaque
+			_surface.fillRect(Common::Rect(_surface.w, _surface.h), _ctx._transKeyColor);
+		}
+
+		// No alignment here
+	} else {
+		/* @7BF */
+		const bool hasDataSize = _ctx._gb->getBit();
+		if (hasDataSize) { /* @81A */
+			/* @822 */
+			int expectedSize = _ctx._gb->getBits(8);
+			if (expectedSize == 0xFF) {
+				expectedSize = _ctx._gb->getBits(24);
+			}
+
+			expectedSize -= ((_ctx._gb->pos() + 7) / 8) - startByte;
 
-	for (int y = 0; y < _surface->h; ++y) {
-		uint32 *startP = (uint32 *)_surface->getBasePtr(0, y);
-		uint32 *endP = (uint32 *)_surface->getBasePtr(_surface->w - 1, y);
+			const int bytesRead = decodeRLETransparency(huffTable._tab->_table);
+			if (bytesRead == -1) {
+				// A more specific warning should have been emitted already
+				return -1;
+			} else if (bytesRead != expectedSize) {
+				warning("Mismatched read %u != %u", bytesRead, expectedSize);
+				return -1;
+			}
+		} else {
+			/* @95B */
+			if (decodeRLETransparency(huffTable._tab->_table) == -1) {
+				warning("Transparency data read failure");
+				return -1;
+			}
+		}
 
-		while (startP <= endP && *startP == keyColor)
-			*startP++ = transColor;
-		while (endP > startP && *endP == keyColor)
-			*endP-- = transColor;
+		_ctx._gb->align();
 	}
+
+	return 0;
 }
 
 int Indeo4Decoder::scaleTileSize(int defSize, int sizeFactor) {
diff --git a/image/codecs/indeo4.h b/image/codecs/indeo4.h
index 8b7fdab3f9..2f3fa8d816 100644
--- a/image/codecs/indeo4.h
+++ b/image/codecs/indeo4.h
@@ -91,9 +91,14 @@ protected:
 	virtual int decodeMbInfo(IVIBandDesc *band, IVITile *tile);
 
 	/**
-	 * Decodes optional transparency data within Indeo frames
+	 * Decodes huffman + RLE-coded transparency data within Indeo4 frames
 	 */
-	virtual void decodeTransparency();
+	int decodeRLETransparency(VLC_TYPE (*table)[2]);
+
+	/**
+	 * Decodes optional transparency data within Indeo4 frames
+	 */
+	virtual int decodeTransparency();
 private:
 	int scaleTileSize(int defSize, int sizeFactor);
 
diff --git a/image/codecs/indeo5.cpp b/image/codecs/indeo5.cpp
index c4e98d4ac7..790bdec87a 100644
--- a/image/codecs/indeo5.cpp
+++ b/image/codecs/indeo5.cpp
@@ -97,7 +97,7 @@ const Graphics::Surface *Indeo5Decoder::decodeFrame(Common::SeekableReadStream &
 	_ctx._frameData = nullptr;
 	_ctx._frameSize = 0;
 
-	return (err < 0) ? nullptr : _surface;
+	return (err < 0) ? nullptr : &_surface;
 }
 
 int Indeo5Decoder::decodePictureHeader() {