SCI32: Refactor DuckPlayer to use common video playback code

This lets DuckPlayer support configurable black-lined video and
configurable high-quality scaling.

3 files changed, 71 insertions, 186 deletions

diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index 506187659a..de751846d0 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -3117,10 +3117,11 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #ifdef ENABLE_SCI3_GAMES
 
 // TODO: Correct GUIOs
-#define GUIO_PHANTASMAGORIA2 GUIO4(GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
-                                   GUIO_NOASPECT, \
+#define GUIO_PHANTASMAGORIA2 GUIO5(GUIO_NOASPECT, \
                                    GUIO_NOMIDI, \
-                                   GAMEOPTION_ORIGINAL_SAVELOAD)
+                                   GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                                   GAMEOPTION_ORIGINAL_SAVELOAD, \
+                                   GAMEOPTION_HQ_VIDEO)
 
 	// Some versions of Phantasmagoria 2 were heavily censored.
 	// Censored versions (data files are currently unknown to us): UK, Australia, first English release in Germany
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index ec37d1c292..22be48c0f8 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -969,21 +969,12 @@ void VMDPlayer::restrictPalette(const uint8 startColor, const int16 endColor) {
 #pragma mark -
 #pragma mark DuckPlayer
 
-DuckPlayer::DuckPlayer(SegManager *segMan, EventManager *eventMan) :
-	VideoPlayer(eventMan),
-	_decoder(new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
+DuckPlayer::DuckPlayer(EventManager *eventMan, SegManager *segMan) :
+	VideoPlayer(eventMan, new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
 	_plane(nullptr),
 	_status(kDuckClosed),
-	_drawRect(),
 	_volume(Audio::Mixer::kMaxChannelVolume),
-	_doFrameOut(false),
-	_pixelDouble(false),
-	_scaleBuffer(nullptr) {}
-
-DuckPlayer::~DuckPlayer() {
-	close();
-	delete _decoder;
-}
+	_doFrameOut(false) {}
 
 void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, const int16 x, const int16 y) {
 	if (_status != kDuckClosed) {
@@ -991,19 +982,22 @@ void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, con
 	}
 
 	const Common::String fileName = Common::String::format("%u.duk", resourceId);
-	if (!_decoder->loadFile(fileName)) {
-		error("Can't open %s", fileName.c_str());
+
+	if (!VideoPlayer::open(fileName)) {
+		return;
 	}
 
 	_decoder->setVolume(_volume);
-	_pixelDouble = displayMode != 0;
 
-	const int16 scale = _pixelDouble ? 2 : 1;
+	_doublePixels = displayMode != 0;
+	_blackLines = ConfMan.getBool("enable_black_lined_video") &&
+				 (displayMode == 1 || displayMode == 3);
+
 	// SSCI seems to incorrectly calculate the draw rect by scaling the origin
 	// in addition to the width/height for the BR point
 	_drawRect = Common::Rect(x, y,
-							 x + _decoder->getWidth() * scale,
-							 y + _decoder->getHeight() * scale);
+							 x + (_decoder->getWidth() << _doublePixels),
+							 y + (_decoder->getHeight() << _doublePixels));
 
 	g_sci->_gfxCursor32->hide();
 
@@ -1013,59 +1007,33 @@ void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, con
 		g_sci->_gfxFrameout->frameOut(true);
 	}
 
-	const Graphics::PixelFormat format = _decoder->getPixelFormat();
-
-	if (_pixelDouble) {
-		assert(_scaleBuffer == nullptr);
-		_scaleBuffer = new byte[_drawRect.width() * _drawRect.height() * format.bytesPerPixel];
+	if (!startHQVideo() && _decoder->getPixelFormat().bytesPerPixel != 1) {
+		g_sci->_gfxFrameout->setPixelFormat(_decoder->getPixelFormat());
 	}
 
-	g_sci->_gfxFrameout->setPixelFormat(format);
-
 	_status = kDuckOpen;
 }
 
 void DuckPlayer::play(const int lastFrameNo) {
-	flushEvents(_eventMan);
+	// This status check does not exist in the original interpreter, but is
+	// necessary to avoid a crash if the engine cannot find or render the video
+	// for playback. Game scripts receive no feedback from the kernel regarding
+	// whether or not an attempt to open a Duck video actually succeeded, so
+	// they can only assume it always succeeds (and so always call to `play`
+	// even if they shouldn't).
+	if (_status == kDuckClosed) {
+		return;
+	}
 
 	if (_status != kDuckPlaying) {
 		_status = kDuckPlaying;
-		_decoder->start();
 	}
 
-	while (!g_engine->shouldQuit()) {
-		if (_decoder->endOfVideo() || (lastFrameNo != -1 && _decoder->getCurFrame() >= lastFrameNo)) {
-			break;
-		}
-
-		g_sci->sleep(_decoder->getTimeToNextFrame());
-		while (_decoder->needsUpdate()) {
-			renderFrame();
-		}
-
-		SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
-		if (event.type == SCI_EVENT_MOUSE_PRESS) {
-			flushEvents(_eventMan);
-			break;
-		}
-
-		event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
-		if (event.type == SCI_EVENT_KEYBOARD) {
-			bool stop = false;
-			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
-				   event.type != SCI_EVENT_NONE) {
-				if (event.character == SCI_KEY_ESC) {
-					stop = true;
-					break;
-				}
-			}
-
-			if (stop) {
-				flushEvents(_eventMan);
-				break;
-			}
-		}
+	if (lastFrameNo != -1) {
+		_decoder->setEndFrame(lastFrameNo);
 	}
+
+	playUntilEvent(kEventFlagMouseDown | kEventFlagEscapeKey);
 }
 
 void DuckPlayer::close() {
@@ -1075,8 +1043,7 @@ void DuckPlayer::close() {
 
 	_decoder->close();
 
-	const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
-	g_sci->_gfxFrameout->setPixelFormat(format);
+	endHQVideo();
 
 	g_sci->_gfxCursor32->unhide();
 
@@ -1086,109 +1053,28 @@ void DuckPlayer::close() {
 		_plane = nullptr;
 	}
 
-	_pixelDouble = false;
-	delete[] _scaleBuffer;
-	_scaleBuffer = nullptr;
-
+	_drawRect = Common::Rect();
 	_status = kDuckClosed;
+	_volume = Audio::Mixer::kMaxChannelVolume;
+	_doFrameOut = false;
 }
 
-static inline uint16 interpolate(const Graphics::PixelFormat &format, const uint16 p1, const uint16 p2) {
-	uint8 r1, g1, b1, r2, g2, b2;
-	format.colorToRGB(p1, r1, g1, b1);
-	format.colorToRGB(p2, r2, g2, b2);
-	return format.RGBToColor((r1 + r2) >> 1, (g1 + g2) >> 1, (b1 + b2) >> 1);
-}
-
-void DuckPlayer::renderFrame() const {
-	const Graphics::Surface *surface = _decoder->decodeNextFrame();
-
-	// Audio-only or non-updated frame
-	if (surface == nullptr) {
+void DuckPlayer::renderFrame(const Graphics::Surface &nextFrame) const {
+#ifdef USE_RGB_COLOR
+	if (_hqVideoMode) {
+		VideoPlayer::renderFrame(nextFrame);
 		return;
 	}
-
-	assert(surface->format.bytesPerPixel == 2);
-
-	if (_pixelDouble) {
-		const uint16 *source = (const uint16 *)surface->getPixels();
-		const Graphics::PixelFormat &format = surface->format;
-		uint16 *target = (uint16 *)_scaleBuffer;
-
-#ifndef SCI_DUCK_NO_INTERPOLATION
-		// divide by 2 gets pixel pitch instead of byte pitch for source
-		const uint16 sourcePitch = surface->pitch >> 1;
-#endif
-
-		const uint16 targetPitch = surface->pitch;
-		const bool blackLined = ConfMan.getBool("enable_black_lined_video");
-		for (int y = 0; y < surface->h - 1; ++y) {
-			for (int x = 0; x < surface->w - 1; ++x) {
-#ifndef SCI_DUCK_NO_INTERPOLATION
-				const uint16 a = source[0];
-				const uint16 b = source[1];
-				const uint16 c = source[sourcePitch];
-				const uint16 d = source[sourcePitch + 1];
-
-				target[0] = a;
-				target[1] = interpolate(format, a, b);
-#else
-				const uint16 value = *source;
-				target[0] = value;
-				target[1] = value;
-#endif
-				if (!blackLined) {
-#ifndef SCI_DUCK_NO_INTERPOLATION
-					target[targetPitch] = interpolate(format, a, c);
-					target[targetPitch + 1] = interpolate(format, target[1], interpolate(format, c, d));
-#else
-					target[targetPitch] = value;
-					target[targetPitch + 1] = value;
 #endif
-				}
-
-				target += 2;
-				++source;
-			}
-
-			const uint16 value = *source++;
-			target[0] = value;
-			target[1] = value;
-			if (!blackLined) {
-				target[targetPitch] = value;
-				target[targetPitch + 1] = value;
-			}
-			target += 2;
-
-			if (blackLined) {
-				memset(target, 0, targetPitch * format.bytesPerPixel);
-			}
-
-			target += targetPitch;
-		}
 
-		for (int x = 0; x < surface->w; ++x) {
-			const uint16 lastValue = *source++;
-			target[0] = lastValue;
-			target[1] = lastValue;
-
-			if (!blackLined) {
-				target[targetPitch] = lastValue;
-				target[targetPitch + 1] = lastValue;
-				target += 2;
-			}
-		}
-
-		if (blackLined) {
-			memset(target, 0, targetPitch);
-		}
-
-		g_system->copyRectToScreen(_scaleBuffer, surface->pitch * 2, _drawRect.left, _drawRect.top, _drawRect.width(), _drawRect.height());
-	} else {
-		g_system->copyRectToScreen(surface->getPixels(), surface->pitch, _drawRect.left, _drawRect.top, surface->w, surface->h);
+	Graphics::Surface out;
+	out.create(_drawRect.width(), _drawRect.height(), nextFrame.format);
+	renderLQToSurface<uint16>(out, nextFrame, _doublePixels, _blackLines);
+	if (out.format != g_system->getScreenFormat()) {
+		out.convertToInPlace(g_system->getScreenFormat());
 	}
-
-	g_sci->_gfxFrameout->updateScreen();
+	g_system->copyRectToScreen(out.getPixels(), out.pitch, _drawRect.left, _drawRect.top, out.w, out.h);
+	out.free();
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index bf3cc84123..5c213484d3 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -608,6 +608,10 @@ private:
 #pragma mark -
 #pragma mark DuckPlayer
 
+/**
+ * DuckPlayer is used to play Duck TrueMotion videos.
+ * Used by Phantasmagoria 2.
+ */
 class DuckPlayer : public VideoPlayer {
 public:
 	enum DuckStatus {
@@ -617,9 +621,7 @@ public:
 		kDuckPaused  = 3
 	};
 
-	DuckPlayer(SegManager *segMan, EventManager *eventMan);
-
-	~DuckPlayer();
+	DuckPlayer(EventManager *eventMan, SegManager *segMan);
 
 	/**
 	 * Opens a stream to a Duck resource.
@@ -637,9 +639,8 @@ public:
 	void play(const int lastFrameNo);
 
 	/**
-	 * Sets a flag indicating that an opaque plane should be added
-	 * to the graphics manager underneath the video surface during
-	 * playback.
+	 * Sets a flag indicating that an opaque plane should be added to the
+	 * graphics manager underneath the video surface during playback.
 	 */
 	void setDoFrameOut(const bool value) { _doFrameOut = value; }
 
@@ -647,17 +648,24 @@ public:
 	 * Sets the volume of the decoder.
 	 */
 	void setVolume(const uint8 value) {
-		_volume = (uint)value * Audio::Mixer::kMaxChannelVolume / Audio32::kMaxVolume;
+		_volume = value * Audio::Mixer::kMaxChannelVolume / Audio32::kMaxVolume;
 		_decoder->setVolume(_volume);
 	}
 
-private:
-	EventManager *_eventMan;
-	Video::AVIDecoder *_decoder;
+protected:
+	virtual bool shouldStartHQVideo() const override {
+		if (!VideoPlayer::shouldStartHQVideo() || _blackLines) {
+			return false;
+		}
 
+		return true;
+	}
+
+	virtual void renderFrame(const Graphics::Surface &nextFrame) const override;
+
+private:
 	/**
-	 * An empty plane drawn behind the video when the doFrameOut
-	 * flag is true.
+	 * An empty plane drawn behind the video when the doFrameOut flag is true.
 	 */
 	Plane *_plane;
 
@@ -667,11 +675,6 @@ private:
 	DuckStatus _status;
 
 	/**
-	 * The screen rect where the video should be drawn.
-	 */
-	Common::Rect _drawRect;
-
-	/**
 	 * The playback volume for the player.
 	 */
 	uint8 _volume;
@@ -683,19 +686,14 @@ private:
 	bool _doFrameOut;
 
 	/**
-	 * If true, the video will be pixel doubled during playback.
-	 */
-	bool _pixelDouble;
-
-	/**
-	 * The buffer used to perform scaling of the video.
+	 * Whether or not the video should be pixel doubled.
 	 */
-	byte *_scaleBuffer;
+	bool _doublePixels;
 
 	/**
-	 * Renders the current frame to the system video buffer.
+	 * Whether or not black lines should be rendered across the video.
 	 */
-	void renderFrame() const;
+	bool _blackLines;
 };
 
 #pragma mark -
@@ -711,7 +709,7 @@ public:
 	_AVIPlayer(eventMan),
 	_VMDPlayer(eventMan, segMan),
 	_robotPlayer(segMan),
-	_duckPlayer(segMan, eventMan) {}
+	_duckPlayer(eventMan, segMan) {}
 
 	void beforeSaveLoadWithSerializer(Common::Serializer &ser);
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);