From 5c2d745302f27c9824f6bcf4b7e50c944d36f0f3 Mon Sep 17 00:00:00 2001
From: Brandon W <bwaggone@umich.edu>
Date: Thu, 27 Mar 2025 17:58:30 -0400
Subject: [PATCH] Fix looping logic when the final frame can't display.

Additionally, adjust the destruction order of the movie texture and
improve logging.
---
 src/arch/MovieTexture/MovieTexture_FFMpeg.cpp | 24 +++++++------------
 .../MovieTexture/MovieTexture_Generic.cpp     | 10 ++++----
 src/arch/MovieTexture/MovieTexture_Generic.h  |  2 +-
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/arch/MovieTexture/MovieTexture_FFMpeg.cpp b/src/arch/MovieTexture/MovieTexture_FFMpeg.cpp
index ad53ab1777..8e0e25d7be 100644
--- a/src/arch/MovieTexture/MovieTexture_FFMpeg.cpp
+++ b/src/arch/MovieTexture/MovieTexture_FFMpeg.cpp
@@ -388,8 +388,8 @@ int MovieDecoder_FFMpeg::DecodePacketToFrame() {
 
 		if (avcodec_return != 0)
 		{
-			LOG->Warn(
-				"Frame %i saw nonzero avcodec_receive_frame status: %i",
+			LOG->Trace(
+				"Frame %i saw nonzero avcodec_receive_frame status: %i, this is likely not fatal.",
 				static_cast<int>(packet_buffer_.size() - 1),
 				avcodec_return);
 
@@ -446,41 +446,35 @@ int MovieDecoder_FFMpeg::GetFrame(RageSurface* surface_out)
 		if (av_sws_context_ == nullptr)
 		{
 			LOG->Warn("Cannot initialize sws conversion context for (%d,%d) %d->%d", GetWidth(), GetHeight(), av_stream_codec_->pix_fmt, av_pixel_format_);
-			return false;
+			return -1;
 		}
 	}
 
 	std::size_t display_frame_in_buffer = (display_frame_num_ + offset_) % frame_buffer_.size();
 	std::lock_guard<std::mutex> lock(frame_buffer_[display_frame_in_buffer]->lock);
+	int scale_status = 0;
 
 	// Sanity check.
 	if (frame_buffer_[display_frame_in_buffer]->packet_num == display_frame_num_) {
-		int ret = avcodec::sws_scale(av_sws_context_,
+		scale_status = avcodec::sws_scale(av_sws_context_,
 			frame_buffer_[display_frame_in_buffer]->frame->data, frame_buffer_[display_frame_in_buffer]->frame->linesize, 0, GetHeight(),
 			pict.data, pict.linesize);
-
-		// If the texture couldn't scale, then it means there's an issue with the
-		// frame. Return an error status here.
-		if (ret <= 0) {
-			display_frame_num_++;
-			frame_buffer_[display_frame_in_buffer]->displayed = true;
-			return -1;
-		}
 	}
 	else {
 		LOG->Warn("Unexpected frame trying to display! display_frame_num_ = %zu, packet_num = %zu", display_frame_num_, frame_buffer_[display_frame_in_buffer]->packet_num);
 	}
 
+	// Even if scale_status returns a failure, we mark displayed as true. The
+	// frame won't be displayed, but instead skipped over.
 	frame_buffer_[display_frame_in_buffer]->displayed = true;
 
-	// Set the end of movie flag if this is the final frame.
 	if (LastFrame()) {
 		end_of_movie_ = true;
-		return 0;
+		return scale_status;
 	}
 	end_of_movie_ = false;
 	display_frame_num_++;
-	return 0;
+	return scale_status;
 }
 
 static RString averr_ssprintf(int err, const char* fmt, ...)
diff --git a/src/arch/MovieTexture/MovieTexture_Generic.cpp b/src/arch/MovieTexture/MovieTexture_Generic.cpp
index 0ac492da5b..e086a8021d 100644
--- a/src/arch/MovieTexture/MovieTexture_Generic.cpp
+++ b/src/arch/MovieTexture/MovieTexture_Generic.cpp
@@ -41,9 +41,11 @@ MovieTexture_Generic::MovieTexture_Generic(RageTextureID ID, MovieDecoder* pDeco
 
 RString MovieTexture_Generic::Init()
 {
-	RString sError = decoder_->Open(GetID().filename);
-	if (sError != "")
-		return sError;
+	RString err = decoder_->Open(GetID().filename);
+	if (err != "") {
+		LOG->Warn("MovieTexture_Generic::Init: failed to open decoder for file: %s, with error:\n%s", GetID().filename.c_str(), err.c_str());
+		return err;
+	}
 
 	CreateTexture();
 	CreateFrameRects();
@@ -80,10 +82,10 @@ MovieTexture_Generic::~MovieTexture_Generic()
 
 	/* sprite_ may reference the texture; delete it before DestroyTexture. */
 	delete sprite_;
+	delete decoder_;
 
 	DestroyTexture();
 
-	delete decoder_;
 }
 
 /* Delete the surface and texture.  The decoding thread must be stopped, and this
diff --git a/src/arch/MovieTexture/MovieTexture_Generic.h b/src/arch/MovieTexture/MovieTexture_Generic.h
index 1642f4b0f5..aebaaca795 100644
--- a/src/arch/MovieTexture/MovieTexture_Generic.h
+++ b/src/arch/MovieTexture/MovieTexture_Generic.h
@@ -108,7 +108,7 @@ public:
 	static EffectMode GetEffectMode( MovieDecoderPixelFormatYCbCr fmt );
 
 private:
-	MovieDecoder *decoder_;
+	MovieDecoder* decoder_;
 
 	std::unique_ptr<std::thread> decoding_thread_;