Implement sliding window between AVPackets and AVFrames, to save memory.

src/arch/MovieTexture/MovieTexture_FFMpeg.cpp

@@ -116,9 +116,12 @@ MovieDecoder_FFMpeg::MovieDecoder_FFMpeg()
 
 	av_format_context_ = nullptr;
 	av_stream_ = nullptr;
-	current_packet_offset_ = -1;
+	total_frames_ = 0;
-
+	end_of_file_ = 0;
-	Init();
+	// Hardcoded frame buffer size of 50. Roughly translates to 100mb of ram.
+	for (int i = 0; i < 50; i++) {
+		frame_buffer_.emplace_back(std::make_unique<FrameHolder>());
+	}
 }
 
 MovieDecoder_FFMpeg::~MovieDecoder_FFMpeg()
@@ -143,6 +146,7 @@ MovieDecoder_FFMpeg::~MovieDecoder_FFMpeg()
 	{
 		avcodec::avcodec_free_context(&av_stream_codec_);
 	}
+	packet_buffer_.clear();
 	frame_buffer_.clear();
 }
 
@@ -150,7 +154,6 @@ void MovieDecoder_FFMpeg::Init()
 {
 	end_of_file_ = 0;
 	display_frame_num_ = 0;
-	total_frames_ = 0;
 	av_sws_context_ = nullptr;
 	av_io_context_ = nullptr;
 	av_buffer_ = nullptr;
@@ -164,75 +167,113 @@ float MovieDecoder_FFMpeg::GetTimestamp() const
 	}
 
 	// In a logical situation, this means that display is outpacing decoding.
-	if (display_frame_num_ >= static_cast<int>(frame_buffer_.size())) {
+	if (display_frame_num_ >= static_cast<int>(packet_buffer_.size())) {
 		return 0;
 	}
-	return frame_buffer_[display_frame_num_]->frame_timestamp;
+
+	PacketHolder* packet = packet_buffer_[display_frame_num_].get();
+
+	// Sanity check.
+	if (packet == nullptr) {
+		return 0;
+	}
+
+	std::lock_guard<std::mutex> lock(packet->lock);
+	return packet->frame_timestamp;
 }
 
 bool MovieDecoder_FFMpeg::IsCurrentFrameReady() {
-	// We're displaying faster than decoding. Do not even try to display the frame.
+	// We're displaying faster than decoding. Do not even try to display the
-	if (display_frame_num_ >= static_cast<int>(frame_buffer_.size())) {
+	// frame.
+	if (display_frame_num_ >= static_cast<int>(packet_buffer_.size())) {
 		return false;
 	}
-	// If the whole movie is decoded, then the frame is definitely ready.
-	if (end_of_file_) {
-		return true;
-	}
 
+	FrameHolder* frame = frame_buffer_[(display_frame_num_ + offset_) % frame_buffer_.size()].get();
+	// To make sure the frame doesn't change from under us.
+	std::lock_guard<std::mutex> lock(frame->lock);
 
-	std::lock_guard<std::mutex> lock(frame_buffer_[display_frame_num_]->lock);
+	// In terms of how the sliding window works, displayed and ready are
-	if (frame_buffer_[display_frame_num_]->skip) {
+	// opposites. If the frame hasn't been displayed, it's ready. If it has
-		LOG->Info("Frame %i not decoded, skipping...", display_frame_num_);
+	// been displayed, then it hasn't been overwritten and reset yet.
-		return true;
+	if (frame->displayed) {
+		LOG->Info("Frame %i not decoded, total frames: %i", display_frame_num_, total_frames_);
 	}
-	if (!frame_buffer_[display_frame_num_]->decoded) {
+	return !frame->displayed;
-		LOG->Info("Frame %i not decoded and was not skipped, total frames: %i", display_frame_num_, total_frames_);
-	}
-	return frame_buffer_[display_frame_num_]->decoded;
 }
 
-int MovieDecoder_FFMpeg::DecodeNextFrame()
+int MovieDecoder_FFMpeg::HandleNextPacket() {
-{
+	// If the decoder hit the end of the file, then that means the packet
-	// Add in a new FrameBuffer entry, and lock it immediately
+	// buffer is complete.
-	frame_buffer_.emplace_back(std::make_unique<FrameHolder>());
+	if (!end_of_file_) {
-	std::unique_lock<std::mutex> lock(frame_buffer_.back()->lock);
+		// Add in a new FrameBuffer entry, and lock it immediately.
+		packet_buffer_.emplace_back(std::make_unique<PacketHolder>());
+		std::unique_lock<std::mutex> lock(packet_buffer_.back()->lock);
 		int status = SendPacketToBuffer();
 		if (status < 0) {
+			lock.unlock();
 			return status;
 		}
+
+		// If the decoded packet is the end of file.
 		if (end_of_file_) {
 			// Release the mutex.
 			lock.unlock();
 
-		frame_buffer_.pop_back(); // Don't display an EoF frame.
+			packet_buffer_.pop_back(); // Don't display an EoF frame.
 			// If we had to approximate the number of frames, set the actual
 			// total number of frames. This is benign even if we did have an
 			// accurate frame count at the start.
-		total_frames_ = frame_buffer_.size();
+			total_frames_ = packet_buffer_.size();
+			return 1;
 		}
-	status = DecodePacketInBuffer();
+		lock.unlock();
-	if (first_frame_) {
-		first_frame_ = false;
 	}
+	return 0;
+}
+
+int MovieDecoder_FFMpeg::DecodeFrame()
+{
+	int status = HandleNextPacket();
+	if (status != 0) {
 		return status;
+	}
+
+	status = DecodePacketToFrame();
+	frame_buffer_position_ = (frame_buffer_position_ + 1) % frame_buffer_.size();
+	packet_buffer_position_ = (packet_buffer_position_ + 1) % total_frames_;
+	return status;
+}
+
+void MovieDecoder_FFMpeg::HandleReset() {
+	reset_ = false;
+
+	for (std::unique_ptr<FrameHolder>& frame : frame_buffer_) {
+		frame->displayed = true;
+	}
+	// If not end of file, reset the decoding.
+	if (!end_of_file_) {
+		avcodec::av_seek_frame(av_format_context_, -1, 0, 0);
+		OpenCodec();
+		packet_buffer_.clear();
+	}
+	offset_ = 0;
+	next_offset_ = 0;
+	display_frame_num_ = 0;
+	packet_buffer_position_ = 0;
+	frame_buffer_position_ = 0;
 }
 
 int MovieDecoder_FFMpeg::DecodeMovie()
 {
-	using std::chrono::operator""ms;
+	// Never exit when the movie is looping. Otherwise exit when the last frame
+	// is added to the FrameBuffer.
+	while (looping_ || (!looping_ && display_frame_num_ < total_frames_)) {
+		if (reset_) {
+			HandleReset();
+		}
 
-	// The first frame expected to be decoded and drawn already,
+		int status = DecodeFrame();
-	// that is handled by MovieTexture_Generic::Init().
-	int frame_num = 0;
-	while (!end_of_file_) {
-		// This wake up time could be tied to the RageTimer, but as it doesn't
-		// need to sync with other parts of ITGm, using chrono is fine.
-		// The 1ms time here is arbitrary, and means that the game will decode
-		// at a maximum speed of 1 frame per ms (or 1000 fps).
-		auto wake_up = std::chrono::steady_clock::now() + 1ms;
-
-		int status = DecodeNextFrame();
 
 		// If cancelled (quitting a song, scrolling the banner), or fatal error,
 		// stop decoding.
@@ -240,17 +281,14 @@ int MovieDecoder_FFMpeg::DecodeMovie()
 			return status;
 		}
 
-		frame_num++;
-
 		// This means when opening the file, less frames were detected than
 		// there actually are. Increment to keep up so we don't end the video
 		// early during display.
-		if (frame_num - 1 > total_frames_) {
+		if (packet_buffer_position_ - 1 > total_frames_ && !end_of_file_) {
 			total_frames_++;
 		}
-
-		std::this_thread::sleep_until(wake_up);
 	}
+
 	return 0;
 }
 
@@ -265,49 +303,76 @@ int MovieDecoder_FFMpeg::SendPacketToBuffer()
 
 	while (true)
 	{
-		int ret = avcodec::av_read_frame(av_format_context_, frame_buffer_.back()->packet);
+		int ret = avcodec::av_read_frame(av_format_context_, packet_buffer_.back()->packet);
 		/* XXX: why is avformat returning AVERROR_NOMEM on EOF? */
 		if (ret < 0)
 		{
-			/* EOF. */
 			end_of_file_ = 1;
 			return 0;
 		}
 
-		if (frame_buffer_.back()->packet->stream_index == av_stream_->index)
+		if (packet_buffer_.back()->packet->stream_index == av_stream_->index)
 		{
-			current_packet_offset_ = 0;
 			return 1;
 		}
 		/* It's not for the video stream; ignore it. */
-		avcodec::av_packet_unref(frame_buffer_.back()->packet);
+		avcodec::av_packet_unref(packet_buffer_.back()->packet);
 	}
 }
 
-int MovieDecoder_FFMpeg::DecodePacketInBuffer() {
+int MovieDecoder_FFMpeg::DecodePacketToFrame() {
 	if (cancel_) {
 		return -2;
 	}
-	if (end_of_file_ == 0 && current_packet_offset_ == -1) {
+
-		return 0; /* no packet */
+	frame_buffer_position_ %= frame_buffer_.size();
+	packet_buffer_position_ %= total_frames_;
+	FrameHolder* frame = frame_buffer_[frame_buffer_position_].get();
+	PacketHolder* packet = packet_buffer_[packet_buffer_position_].get();
+
+	// Packet buffer is bigger than the frame buffer, don't overwrite frames that
+	// haven't been displayed.
+	if (packet_buffer_.size() > frame_buffer_.size()) {
+		while (!frame->displayed) {
+			// Sleep so the CPU performance stays happy.
+			usleep(1000);  // 1ms
+			if (cancel_) {
+				return -2;
+			}
+			if (reset_) {
+				return 0;
+			}
+		}
 	}
 
-	while (end_of_file_ == 0 && current_packet_offset_ <= frame_buffer_.back()->packet->size)
+	std::lock_guard<std::mutex> frame_lock(frame->lock);
+	std::lock_guard<std::mutex> packet_lock(packet->lock);
+
+	// If the movie is looping naturally, next_offset_ is decided where
+	// packet_buffer_position_ zero is decoded.
+	if (packet->decoded && packet_buffer_position_ == 0) {
+		next_offset_ = frame_buffer_position_;
+	}
+
+	int packet_offset = 0;
+	while (packet_offset <= packet->packet->size)
 	{
 		/* If we have no data on the first frame, just return EOF; passing an empty packet
 		 * to avcodec_decode_video in this case is crashing it.  However, passing an empty
 		 * packet is normal with B-frames, to flush.  This may be unnecessary in newer
 		 * versions of avcodec, but I'm waiting until a new stable release to upgrade. */
-		if (frame_buffer_.back()->packet->size == 0 && first_frame_) {
+		if (packet->packet->size == 0 && packet_buffer_position_ == 0) {
 			return 0; /* eof */
 		}
 
 		/* Hack: we need to send size = 0 to flush frames at the end, but we have
 		 * to give it a buffer to read from since it tries to read anyway. */
-		frame_buffer_.back()->packet->data = frame_buffer_.back()->packet->size ? frame_buffer_.back()->packet->data : nullptr;
+		packet->packet->data = packet->packet->size ? packet->packet->data : nullptr;
-		int len = frame_buffer_.back()->packet->size;
+		int len = packet->packet->size;
-		avcodec::avcodec_send_packet(av_stream_codec_, frame_buffer_.back()->packet);
+		avcodec::avcodec_send_packet(av_stream_codec_, packet->packet);
-		int avcodec_return = avcodec::avcodec_receive_frame(av_stream_codec_, frame_buffer_.back()->frame);
+		int avcodec_return = avcodec::avcodec_receive_frame(av_stream_codec_, frame_buffer_[frame_buffer_position_]->frame);
+		frame->displayed = false;
+		frame->packet_num = packet_buffer_position_;
 
 		if (len < 0)
 		{
@@ -315,64 +380,50 @@ int MovieDecoder_FFMpeg::DecodePacketInBuffer() {
 			return -1;
 		}
 
-		current_packet_offset_ += len;
+		packet_offset += len;
 
 		if (avcodec_return != 0)
 		{
 			LOG->Warn(
-				"Frame number %i not successfully decoded into buffer. avcodec_receive_frame status: %i",
+				"Frame %i saw nonzero avcodec_receive_frame status: %i",
-				static_cast<int>(frame_buffer_.size() - 1),
+				static_cast<int>(packet_buffer_.size() - 1),
 				avcodec_return);
+
+			// Not a fatal decoding error, and the FFMpeg code is robust enough to handle displaying
+			// somewhat mangled frames.
+			if (packet_offset <= packet->packet->size) {
 				continue;
 			}
+		}
 
-		if (frame_buffer_.back()->frame->pkt_dts != AV_NOPTS_VALUE)
+		if (frame->frame->pkt_dts != AV_NOPTS_VALUE)
 		{
-			frame_buffer_.back()->frame_timestamp = (float)(frame_buffer_.back()->frame->pkt_dts * av_q2d(av_stream_->time_base));
+			packet->frame_timestamp = (float)(frame->frame->pkt_dts * av_q2d(av_stream_->time_base));
 		}
 		else
 		{
 			/* If the timestamp is zero, this frame is to be played at the
 			 * time of the last frame plus the length of the last frame. */
-			if (!first_frame_) {
+			if (packet_buffer_position_ != 0) {
-				frame_buffer_.back()->frame_timestamp += frame_buffer_[frame_buffer_.size() - 2]->frame_delay;
+				packet->frame_timestamp += packet_buffer_[packet_buffer_.size() - 2]->frame_delay;
 			}
 			else {
-				frame_buffer_.back()->frame_timestamp = 0;
+				packet->frame_timestamp = 0;
 			}
 		}
 
 		// Length of this frame, only used as a fallback for getting the frame
 		// timestamp above.
-		frame_buffer_.back()->frame_delay = (float)av_q2d(av_stream_->time_base);
+		packet->frame_delay = (float)av_q2d(av_stream_->time_base);
-		frame_buffer_.back()->frame_delay += frame_buffer_.back()->frame->repeat_pict * (frame_buffer_.back()->frame_delay * 0.5f);
+		packet->frame_delay += frame_buffer_[frame_buffer_position_]->frame->repeat_pict * (packet->frame_delay * 0.5f);
-		frame_buffer_.back()->decoded = true;
+		packet->decoded = true;
-
 		return 1;
 	}
 
-	// This if statement means the packet did not decode correctly. This is not
-	// necessarily fatal for video playback, but out of caution the frame should
-	// be skipped.
-	if (!frame_buffer_.back()->decoded) {
-		frame_buffer_.back()->skip = true;
-	}
-
 	return 0; /* packet done */
 }
 
-bool MovieDecoder_FFMpeg::SkipNextFrame() {
+int MovieDecoder_FFMpeg::GetFrame(RageSurface* pSurface)
-	if (display_frame_num_ > (total_frames_ - 1)) {
-		return true;
-	}
-	if (frame_buffer_[display_frame_num_]->skip) {
-		display_frame_num_++;
-		return true;
-	}
-	return false;
-}
-
-bool MovieDecoder_FFMpeg::GetFrame(RageSurface* pSurface)
 {
 	avcodec::AVFrame pict;
 	pict.data[0] = (unsigned char*)pSurface->pixels;
@@ -395,18 +446,37 @@ bool MovieDecoder_FFMpeg::GetFrame(RageSurface* pSurface)
 		}
 	}
 
-	avcodec::sws_scale(av_sws_context_,
+	int display_frame_in_buffer = (display_frame_num_ + offset_) % frame_buffer_.size();
-		frame_buffer_[display_frame_num_]->frame->data, frame_buffer_[display_frame_num_]->frame->linesize, 0, GetHeight(),
+	std::lock_guard<std::mutex> lock(frame_buffer_[display_frame_in_buffer]->lock);
+
+	// Sanity check.
+	if (frame_buffer_[display_frame_in_buffer]->packet_num == display_frame_num_) {
+		int ret = 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);
 
-	// Don't advance the frame number past the (potential) end of the buffer.
+		// If the texture couldn't scale, then it means there's an issue with the
-	// This can happen if display is outpacing decoding, or if we're at the
+		// frame. Return an error status here.
-	// end of file.
+		if (ret <= 0) {
-	if (display_frame_num_ >= (total_frames_ - 1)) {
-		return end_of_file_;
-	}
 			display_frame_num_++;
-	return false;
+			frame_buffer_[display_frame_in_buffer]->displayed = true;
+			return -1;
+		}
+	}
+	else {
+		LOG->Warn("Unexpected frame trying to display! display_frame_num_ = %d, packet_num = %d", display_frame_num_, frame_buffer_[display_frame_in_buffer]->packet_num);
+	}
+
+	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;
+	}
+	end_of_movie_ = false;
+	display_frame_num_++;
+	return 0;
 }
 
 static RString averr_ssprintf(int err, const char* fmt, ...)
@@ -549,13 +619,19 @@ void MovieDecoder_FFMpeg::Close()
 		avcodec::avformat_close_input(&av_format_context_);
 		av_format_context_ = nullptr;
 	}
-
-	Init();
 }
 
 void MovieDecoder_FFMpeg::Rewind()
 {
 	display_frame_num_ = 0;
+	reset_ = true;
+}
+
+void MovieDecoder_FFMpeg::Rollover()
+{
+	display_frame_num_ = 0;
+	offset_ = next_offset_;
+	next_offset_ = 0;
 }
 
 RageSurface* MovieDecoder_FFMpeg::CreateCompatibleSurface(int iTextureWidth, int iTextureHeight, bool bPreferHighColor, MovieDecoderPixelFormatYCbCr& fmtout)

src/arch/MovieTexture/MovieTexture_FFMpeg.h

@@ -26,31 +26,29 @@ static const int kSwsFlags = SWS_BICUBIC; // XXX: Reasonable default?
 
 struct FrameHolder {
 	avcodec::AVFrame* frame = avcodec::av_frame_alloc();
+	bool displayed = false;
+	int packet_num = -1; // Used as a sanity check during display.
+	std::mutex lock; // Protects the frame as it's being initialized.
+	~FrameHolder() {
+		if (frame != nullptr) {
+			avcodec::av_frame_free(&frame);
+		}
+	}
+};
+
+struct PacketHolder {
 	avcodec::AVPacket* packet = avcodec::av_packet_alloc();
 	float frame_timestamp = 0;
 	float frame_delay = 0;
 	bool decoded = false;
-	bool skip = false;
+	std::mutex lock; // Protects the packet as it's being initialized.
-	std::mutex lock; // Protects the frame as it's being initialized.
 
-	FrameHolder() = default;
+	PacketHolder() = default;
 
-	FrameHolder(const FrameHolder& fh) {
+	~PacketHolder() {
-		avcodec::av_frame_ref(frame, fh.frame);
-		avcodec::av_packet_ref(packet, fh.packet);
-		frame_timestamp = fh.frame_timestamp;
-		frame_delay = fh.frame_delay;
-		decoded = fh.decoded;
-		skip = fh.skip;
-	}
-
-	~FrameHolder() {
 		if (packet != nullptr) {
 			avcodec::av_packet_free(&packet);
 		}
-		if (frame != nullptr) {
-			avcodec::av_frame_free(&frame);
-		}
 	}
 };
 
@@ -77,27 +75,37 @@ public:
 
 	RString Open(RString sFile);
 	void Close();
+
+	// Rewind sends the reset signal to DecodeMovie. See DecodeMovie
+	// and HandleReset for more information.
 	void Rewind();
 
-	// This draws a frame from the buffer onto the provided RageSurface.
+	// Like rewind, but handles the case that a looping video reached the end,
-	// Returns true if returning the last frame in the movie.
+	// and the next frame to display is the first one of the movie.
-	bool GetFrame(RageSurface* pOut);
+	void Rollover();
-	int DecodeFrame(float fTargetTime);
 
-	// Decode a single frame.  Return -2 on cancel, -1 on error, 0 on EOF, 1 if we have a frame.
+	// This draws a frame from the buffer onto the provided RageSurface.
-	int DecodeNextFrame();
+	// Returns 1 if the last frame of the movie, -1 if there's an issue
+	// with the frame and we should skip.
+	int GetFrame(RageSurface* pOut);
+
+	// Handles the next packet in decoding.
+	int HandleNextPacket();
+
+	// Decode a single frame.
+	// Return -2 on cancel
+	//        -1 on error
+	//         0 on success
+	//         1 on success and end_of_file_ set
+	int DecodeFrame();
 
 	// Decode the entire movie.
-	// If we let this decode as fast as possible, it could come at the expense
+	// Works via a sliding window between packet_buffer_ and frame_buffer_.
-	// of gameplay performance. Since dropping frames is undesirable, an
+	// The frame_buffer_, when full, will not reuse a FrameHolder until the
-	// artificial rate limit is introduced.
+	// frame at frame_buffer_position_ has been displayed.
 	//
-	// Given that most movies will display at 30fps or 60fps, decoding at a
+	// Returns 0 on success, -1 on fatal error, -2 on cancel. Looping movies
-	// speed of 1000fps should be more than sufficient to ensure we never
+	// never exit until destruction.
-	// run behind. This rate limiting is only needed in case itgmania is
-	// running on a low performance machine, or the movie is REALLY long.
-	//
-	// Returns 0 on success, -1 on fatal error, -2 on cancel.
 	int DecodeMovie();
 	bool IsCurrentFrameReady();
 
@@ -108,10 +116,17 @@ public:
 
 	float GetTimestamp() const;
 
+	// Cancel decoding.
 	void Cancel() { cancel_ = true; };
 
-	// If the next frame to display had an issue decoding, skip it.
+	// Called by the MovieTexture to tell the decoder if the movie loops.
-	bool SkipNextFrame();
+	void SetLooping(bool loop) { looping_ = loop; }
+
+	// Are we displaying the last frame?
+	bool LastFrame() { return display_frame_num_ == (total_frames_ - 1); }
+
+	// The signal if the final frame of the movie was just displayed.
+	bool EndOfMovie() { return end_of_movie_; }
 
 private:
 	void Init();
@@ -121,34 +136,51 @@ private:
 	// Returns -2 on cancel, -1 on error, 0 on EOF, 1 on OK.
 	int SendPacketToBuffer();
 
-	// Decode frame data from the packet in the buffer.
+	// Send the packet at packet_buffer_position_ to the frame buffer
+	// at the next open position.
 	// Returns -2 on cancel, -1 on error, 0 if the packet is finished.
-	int DecodePacketInBuffer();
+	int DecodePacketToFrame();
+	void HandleReset();
 
 	avcodec::AVStream* av_stream_;
 	avcodec::AVPixelFormat av_pixel_format_;	/* pixel format of output surface */
 	avcodec::SwsContext* av_sws_context_;
 	avcodec::AVCodecContext* av_stream_codec_;
-
 	avcodec::AVFormatContext* av_format_context_;
-	int display_frame_num_;
 	int total_frames_; // Total number of frames in the movie.
 
 	unsigned char* av_buffer_;
 	avcodec::AVIOContext* av_io_context_;
 
-	// The movie buffer.
+	// The movie's buffers. This uses a sliding window from FrameBuffer to
+	// PacketBuffer. AVPackets are small enough that keeping the whole movie's
+	// packets in memory is trivial, but AVFrames can quickly overwhelm RAM.
+	// Therefore, the FrameBuffer represents a sliding window along the PacketBuffer.
+	std::vector<std::unique_ptr<PacketHolder>> packet_buffer_;
 	std::vector<std::unique_ptr<FrameHolder>> frame_buffer_;
+	int frame_buffer_position_ = 0;
+	int packet_buffer_position_ = 0;
 
-	int current_packet_offset_;
+	// Offset for the frame_buffer_ when a looping movie goes back to
+	// the zeroeth frame. next_offset_ is written when the zeroeth frame
+	// is decoded, and when the last frame is displayed, it is applied to
+	// offset_.
+	int offset_ = 0;
+	int next_offset_ = 0;
+
+	// display_frame_num_ will often be the start of the sliding window,
+	// or the oldest Frame that is currently decoded.
+	int display_frame_num_ = 0;
 
 	// 0 = no EOF
 	// 1 = EOF while decoding
 	int end_of_file_;
 
-	// If true, received a cancel signal from the MovieTexture.
+	// The various flags used to signal the decoding thread to do something.
 	bool cancel_ = false;
-	bool first_frame_ = true;
+	bool looping_ = false;
+	bool reset_ = false;
+	bool end_of_movie_ = false;
 };
 
 static struct AVPixelFormat_t

src/arch/MovieTexture/MovieTexture_Generic.cpp

@@ -47,16 +47,7 @@ RString MovieTexture_Generic::Init()
 
 	CreateTexture();
 	CreateFrameRects();
-
+	m_pDecoder->SetLooping(m_bLoop);
-
-	// Draw the first frame immediately, to guarantee that the texture is drawn
-	// when this function returns--if possible.
-	if (m_pDecoder->DecodeNextFrame() < 0) {
-		LOG->Trace("Failure to decode first frame of video file \"%s\"", GetID().filename.c_str());
-		m_failure = true;
-		return RString("Failure to display movie.");
-	};
-	UpdateMovie(0);
 
 	decoding_thread = std::make_unique<std::thread>([this]() {
 		LOG->Trace("Beginning to decode video file \"%s\"", GetID().filename.c_str());
@@ -339,23 +330,26 @@ void MovieTexture_Generic::UpdateMovie(float fSeconds)
 
 void MovieTexture_Generic::UpdateFrame()
 {
-	/* Just in case we were invalidated: */
+	if (finished_) {
-	CreateTexture();
-
-	if (m_pDecoder->SkipNextFrame()) {
 		return;
 	}
 
+	/* Just in case we were invalidated: */
+	CreateTexture();
+
 	if(m_pTextureLock != nullptr)
 	{
 		std::uintptr_t iHandle = m_pTextureIntermediate != nullptr ? m_pTextureIntermediate->GetTexHandle(): this->GetTexHandle();
 		m_pTextureLock->Lock(iHandle, m_pSurface);
 	}
 
-	/* Are we looping? */
+	int frame_ret = m_pDecoder->GetFrame(m_pSurface);
-	if (m_pDecoder->GetFrame(m_pSurface) && m_bLoop) {
+
+	// Are we looping?
+	if (m_pDecoder->EndOfMovie() && m_bLoop) {
 		LOG->Trace("File \"%s\" looping", GetID().filename.c_str());
-		m_pDecoder->Rewind();
+		m_pDecoder->Rollover();
+
 		// There's a gap in the audio when the music preview loops. This value
 		// is dynamic based on the ending and starting beats (see
 		// GameSoundManager.cpp::StartMusic).
@@ -365,7 +359,20 @@ void MovieTexture_Generic::UpdateFrame()
 		// Until it does, we can either freeze at the end of the video banner,
 		// or give it a best effort approximation (0.5 seconds).
 		m_fClock = 0.5;
-	};
+	}
+	else if (m_pDecoder->EndOfMovie()) {
+		// At the end of the movie, and not looping.
+		finished_ = true;
+	}
+
+	// There's an issue with the frame, make sure it does not get
+	// uploaded.
+	if (frame_ret == -1) {
+		if (m_pTextureLock != nullptr) {
+			m_pTextureLock->Unlock(m_pSurface, true);
+		}
+		return;
+	}
 
 	if (m_pTextureLock != nullptr) {
 		m_pTextureLock->Unlock(m_pSurface, true);
@@ -416,8 +423,9 @@ void MovieTexture_Generic::Reload()
 
 void MovieTexture_Generic::SetPosition(float fSeconds)
 {
-	// In theory, we can math out fSeconds and frame counts to seek the video,
+	// TODO: The only non-zero use case of this function would be practice mode.
-	// but there's likely no practical use case of this.
+	// Implement this by mathing out fSeconds and frame counts to seek the
+	// video.
 	if (fSeconds != 0)
 	{
 		LOG->Warn("MovieTexture_Generic::SetPosition(%f): non-0 seeking unsupported; ignored", fSeconds);

src/arch/MovieTexture/MovieTexture_Generic.h

@@ -28,10 +28,11 @@ public:
 	virtual RString Open( RString sFile ) = 0;
 	virtual void Close() = 0;
 	virtual void Rewind() = 0;
+	virtual void Rollover() = 0;
 
 	// Decode the next frame.
 	// Return 1 on success, 0 on EOF, -1 on fatal error, -2 on cancel.
-	virtual int DecodeNextFrame() = 0;
+	virtual int DecodeFrame() = 0;
 	virtual int DecodeMovie() = 0;
 
 	// Returns true if the frame we want to display has been decoded already.
@@ -40,10 +41,7 @@ public:
 	/*
 	 * Get the currently-decoded frame.
 	 */
-	virtual bool GetFrame( RageSurface *pOut ) = 0;
+	virtual int GetFrame( RageSurface *pOut ) = 0;
-
-	// Returns true if the frame should be skipped.
-	virtual bool SkipNextFrame() = 0;
 
 	/* Return the dimensions of the image, in pixels (before aspect ratio
 	 * adjustments). */
@@ -76,6 +74,12 @@ public:
 
 	// Cancels the decoding of the movie.
 	virtual void Cancel() = 0;
+
+	// Sets the looping property on the decoder.
+	virtual void SetLooping(bool loop) = 0;
+
+	// Returns true if the the final frame was displayed.
+	virtual bool EndOfMovie() = 0;
 };
 
 
@@ -110,6 +114,7 @@ private:
 
 	float m_fRate;
 	bool m_bLoop;
+	bool finished_ = false;
 
 	// If true, halts all decoding and display.
 	bool m_failure = false;
@@ -131,7 +136,6 @@ private:
 	void CreateTexture();
 	void DestroyTexture();
 
-	bool DecodeFrame();
 	float CheckFrameTime();
 };