From 245cc68afabefbc9290bd5a13ec327a59fe23b6d Mon Sep 17 00:00:00 2001
From: arch1t3cht <arch1t3cht@gmail.com>
Date: Tue, 1 Nov 2022 11:26:57 +0100
Subject: [PATCH] Fix FrameAtTime computation for CFR

The new formula is just the inverse function of the CFR part of
the TimeAtFrame function.

To see how the previous implementation was faulty, see either the added
tests, or
- In Aegisub, open a dummy video with a frame rate of 23.976
- Make a subtitle event with start time 04:44.41
- Double-click the line to (supposedly) seek to its first frame
- This will seek one frame earlier than it should, and the event will
  not be displayed on the resulting frame.
---
 libaegisub/common/vfr.cpp |  2 +-
 tests/tests/vfr.cpp       | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/libaegisub/common/vfr.cpp b/libaegisub/common/vfr.cpp
index c82fcdbee..377e29b34 100644
--- a/libaegisub/common/vfr.cpp
+++ b/libaegisub/common/vfr.cpp
@@ -225,7 +225,7 @@ int Framerate::FrameAtTime(int ms, Time type) const {
 		return int((ms * numerator / denominator - 999) / 1000);
 
 	if (ms > timecodes.back())
-		return int((ms * numerator - last + denominator - 1) / denominator / 1000) + (int)timecodes.size() - 1;
+		return int((ms * numerator - numerator / 2 - last + numerator - 1) / denominator / 1000) + (int)timecodes.size() - 1;
 
 	return (int)distance(lower_bound(timecodes.rbegin(), timecodes.rend(), ms, std::greater<int>()), timecodes.rend()) - 1;
 }
diff --git a/tests/tests/vfr.cpp b/tests/tests/vfr.cpp
index a9b8f3acf..90a02d4c6 100644
--- a/tests/tests/vfr.cpp
+++ b/tests/tests/vfr.cpp
@@ -149,6 +149,12 @@ TEST(lagi_vfr, cfr_round_trip_exact) {
 	for (int i = -10; i < 11; i++) {
 		EXPECT_EQ(i, fps.FrameAtTime(fps.TimeAtFrame(i)));
 	}
+
+	ASSERT_NO_THROW(fps = Framerate(24000, 1001));
+	int frames[] = {-100, -10, -1, 0, 1, 10, 100, 6820};
+	for (int i : frames) {
+		EXPECT_EQ(i, fps.FrameAtTime(fps.TimeAtFrame(i)));
+	}
 }
 
 TEST(lagi_vfr, cfr_round_trip_start) {
@@ -157,6 +163,12 @@ TEST(lagi_vfr, cfr_round_trip_start) {
 	for (int i = -10; i < 11; i++) {
 		EXPECT_EQ(i, fps.FrameAtTime(fps.TimeAtFrame(i, START), START));
 	}
+
+	ASSERT_NO_THROW(fps = Framerate(24000, 1001));
+	int frames[] = {-100, -10, -1, 0, 1, 10, 100, 6820};
+	for (int i : frames) {
+		EXPECT_EQ(i, fps.FrameAtTime(fps.TimeAtFrame(i, START), START));
+	}
 }
 
 TEST(lagi_vfr, cfr_round_trip_end) {
@@ -165,6 +177,12 @@ TEST(lagi_vfr, cfr_round_trip_end) {
 	for (int i = -10; i < 11; i++) {
 		EXPECT_EQ(i, fps.FrameAtTime(fps.TimeAtFrame(i, END), END));
 	}
+
+	ASSERT_NO_THROW(fps = Framerate(24000, 1001));
+	int frames[] = {-100, -10, -1, 0, 1, 10, 100, 6820};
+	for (int i : frames) {
+		EXPECT_EQ(i, fps.FrameAtTime(fps.TimeAtFrame(i, END), END));
+	}
 }
 
 TEST(lagi_vfr, vfr_round_trip_exact) {