Don't load the FPS from Aegisub keyframes. The format breaks vfr and setting the fps should be done with timecode files.

Originally committed to SVN as r5900.

aegisub/libaegisub/common/keyframe.cpp

@@ -32,21 +32,19 @@
 #include "libaegisub/keyframe.h"
 #include "libaegisub/vfr.h"
 
-static std::pair<std::vector<int>, double> agi_keyframes(std::istream &file) {
+namespace {
+std::vector<int> agi_keyframes(std::istream &file) {
 	double fps;
 	std::string fps_str;
 	file >> fps_str;
 	file >> fps;
 
-	if (!file.good() || fps_str != "fps")
-		throw agi::keyframe::Error("FPS not found");
-
 	std::vector<int> ret;
-	std::copy(std::istream_iterator<int>(file), std::istream_iterator<int>(), std::back_inserter(ret));
+	copy(std::istream_iterator<int>(file), std::istream_iterator<int>(), back_inserter(ret));
-	return make_pair(ret, fps);
+	return ret;
 }
 
-static std::pair<std::vector<int>, double> other_keyframes(std::istream &file, char (*func)(std::string const&)) {
+std::vector<int> other_keyframes(std::istream &file, char (*func)(std::string const&)) {
 	int count = 0;
 	std::vector<int> ret;
 	agi::line_iterator<std::string> end;
@@ -59,7 +57,7 @@ static std::pair<std::vector<int>, double> other_keyframes(std::istream &file, c
 			++count;
 		}
 	}
-	return std::make_pair(ret, 0);
+	return ret;
 }
 
 char xvid(std::string const& line) {
@@ -83,23 +81,22 @@ char x264(std::string const& line) {
 }
 
 template<int N>
-static bool starts_with(std::string const& str, const char (&test)[N]) {
+bool starts_with(std::string const& str, const char (&test)[N]) {
 	if (str.size() < N) return false;
 	return std::mismatch(str.begin(), str.begin() + N - 1, test).first == str.begin() + N - 1;
 }
-
+}
 
 namespace agi { namespace keyframe {
-
+	void Save(std::string const& filename, std::vector<int> const& keyframes) {
-	void Save(std::string const& filename, std::vector<int> const& keyframes, vfr::Framerate const& fps) {
 	io::Save file(filename);
 	std::ofstream& of = file.Get();
 	of << "# keyframe format v1" << std::endl;
-	of << "fps " << fps.FPS() << std::endl;
+	of << "fps " << 0 << std::endl;
-	std::copy(keyframes.begin(), keyframes.end(), std::ostream_iterator<int>(of, "\n"));
+	copy(keyframes.begin(), keyframes.end(), std::ostream_iterator<int>(of, "\n"));
 }
 
-std::pair<std::vector<int>, double> Load(std::string const& filename) {
+std::vector<int> Load(std::string const& filename) {
 	std::auto_ptr<std::ifstream> file(io::Open(filename));
 	std::istream &is(*file.get());
 

aegisub/libaegisub/include/libaegisub/keyframe.h

@@ -30,13 +30,12 @@ namespace agi {
 	namespace keyframe {
 		/// @brief Load a keyframe file
 		/// @param filename File to load
-		/// @return Pair of frame numbers which are keyframes and fps
+		/// @return List of frame numbers which are keyframes
-		std::pair<std::vector<int>, double> Load(std::string const& filename);
+		std::vector<int> Load(std::string const& filename);
 		/// @brief Save keyframes to a file
 		/// @param filename File to save to
 		/// @param keyframes List of keyframes to save
-		/// @param fps Current fps that goes with the keyframes
+		void Save(std::string const& filename, std::vector<int> const& keyframes);
-		void Save(std::string const& filename, std::vector<int> const& keyframes, vfr::Framerate const& fps);
 
 		DEFINE_SIMPLE_EXCEPTION_NOINNER(Error, Exception, "keyframe/error")
 	}

aegisub/src/video_context.cpp

@@ -482,16 +482,9 @@ void VideoContext::SetAspectRatio(int type, double value) {
 void VideoContext::LoadKeyframes(wxString filename) {
 	if (filename == keyFramesFilename || filename.empty()) return;
 	try {
-		std::pair<std::vector<int>, double> kf = agi::keyframe::Load(STD_STR(filename));
+		keyFrames = agi::keyframe::Load(STD_STR(filename));
-		keyFrames = kf.first;
 		keyFramesFilename = filename;
 		KeyframesOpen(keyFrames);
-		if (kf.second != 0.) {
-			ovrFPS = agi::vfr::Framerate(kf.second);
-			ovrTimecodeFile.clear();
-			OnSubtitlesCommit();
-			TimecodesOpen(ovrFPS);
-		}
 		config::mru->Add("Keyframes", STD_STR(filename));
 	}
 	catch (agi::keyframe::Error const& err) {
@@ -505,18 +498,16 @@ void VideoContext::LoadKeyframes(wxString filename) {
 }
 
 void VideoContext::SaveKeyframes(wxString filename) {
-	agi::keyframe::Save(STD_STR(filename), GetKeyFrames(), FPS());
+	agi::keyframe::Save(STD_STR(filename), GetKeyFrames());
 	config::mru->Add("Keyframes", STD_STR(filename));
 }
 
 void VideoContext::CloseKeyframes() {
 	keyFramesFilename.clear();
-	if (videoProvider.get()) {
+	if (videoProvider)
 		keyFrames = videoProvider->GetKeyFrames();
-	}
+	else
-	else {
 		keyFrames.clear();
-	}
 	KeyframesOpen(keyFrames);
 }
 

aegisub/tests/libaegisub_keyframe.cpp

@@ -20,7 +20,6 @@
 
 #include <libaegisub/access.h>
 #include <libaegisub/keyframe.h>
-#include <libaegisub/vfr.h>
 
 #include <fstream>
 #include <iterator>
@@ -33,15 +32,13 @@ using namespace util;
 
 TEST(lagi_keyframe, save) {
 	std::vector<int> kf = make_vector<int>(5, 0, 5, 70, 180, 300);
-	agi::vfr::Framerate fps(80.);
 
-	ASSERT_NO_THROW(Save("data/keyframe/out.txt", kf, fps));
+	ASSERT_NO_THROW(Save("data/keyframe/out.txt", kf));
 
-	std::pair<std::vector<int>, double> res;
+	std::vector<int> res;
 	ASSERT_NO_THROW(res = Load("data/keyframe/out.txt"));
 
-	EXPECT_TRUE(kf == res.first);
+	EXPECT_TRUE(kf == res);
-	EXPECT_EQ(80., res.second);
 }
 
 TEST(lagi_keyframe, bad_files) {
@@ -118,11 +115,10 @@ TEST(lagi_keyframe, xvid) {
 		33880, 33923, 33969, 33982, 33991, 34010, 34025, 34043, 34055, 34065,
 		34082, 34102, 34112, 34122, 34142, 34158, 34170, 34187, 34211);
 
-	std::pair<std::vector<int>, double> res;
+	std::vector<int> res;
 	ASSERT_NO_THROW(res = Load("data/keyframe/xvid.txt"));
 
-	EXPECT_TRUE(expected == res.first);
+	EXPECT_TRUE(expected == res);
-	EXPECT_EQ(0., res.second);
 
 }
 
@@ -130,20 +126,18 @@ TEST(lagi_keyframe, x264) {
 	std::vector<int> expected(41);
 	for (size_t i = 0; i < 41; ++i) expected[i] = i * 250;
 
-	std::pair<std::vector<int>, double> res;
+	std::vector<int> res;
 	ASSERT_NO_THROW(res = Load("data/keyframe/x264.log"));
 
-	EXPECT_TRUE(expected == res.first);
+	EXPECT_TRUE(expected == res);
-	EXPECT_EQ(0., res.second);
 }
 
 TEST(lagi_keyframe, aegi) {
 	std::vector<int> expected(135);
 	for (size_t i = 0; i < 135; ++i) expected[i] = i * 250;
 
-	std::pair<std::vector<int>, double> res;
+	std::vector<int> res;
 	ASSERT_NO_THROW(res = Load("data/keyframe/aegi.txt"));
 
-	EXPECT_TRUE(expected == res.first);
+	EXPECT_TRUE(expected == res);
-	EXPECT_EQ(25., res.second);
 }