Allow fractional frame rates in dummy video

The validation code for the dummy video dialog is kind of dirty but I've
had this lying around for months and just want to get it done...

src/dialog_dummy_video.cpp

@@ -15,6 +15,7 @@
 // Aegisub Project http://www.aegisub.org/
 
 #include "colour_button.h"
+#include "compat.h"
 #include "format.h"
 #include "help_button.h"
 #include "libresrc/libresrc.h"
@@ -40,7 +41,7 @@ namespace {
 struct DialogDummyVideo {
 	wxDialog d;
 
-	double fps       = OPT_GET("Video/Dummy/FPS")->GetDouble();
+	wxString fps     = OPT_GET("Video/Dummy/FPS String")->GetString();
 	int width        = OPT_GET("Video/Dummy/Last/Width")->GetInt();
 	int height       = OPT_GET("Video/Dummy/Last/Height")->GetInt();
 	int length       = OPT_GET("Video/Dummy/Last/Length")->GetInt();
@@ -54,7 +55,7 @@ struct DialogDummyVideo {
 	void AddCtrl(wxString const& label, T *ctrl);
 
 	void OnResolutionShortcut(wxCommandEvent &evt);
-	void UpdateLengthDisplay();
+	bool UpdateLengthDisplay();
 
 	DialogDummyVideo(wxWindow *parent);
 };
@@ -85,10 +86,6 @@ wxSpinCtrl *spin_ctrl(wxWindow *parent, int min, int max, int *value) {
 	return ctrl;
 }
 
-wxControl *spin_ctrl(wxWindow *parent, double min, double max, double *value) {
-	return new wxTextCtrl(parent, -1, "", wxDefaultPosition, wxSize(50, -1), 0, DoubleValidator(value, min, max));
-}
-
 wxComboBox *resolution_shortcuts(wxWindow *parent, int width, int height) {
 	wxComboBox *ctrl = new wxComboBox(parent, -1, "", wxDefaultPosition, wxDefaultSize, 0, nullptr, wxCB_READONLY);
 
@@ -120,7 +117,9 @@ DialogDummyVideo::DialogDummyVideo(wxWindow *parent)
 	AddCtrl(_("Video resolution:"), resolution_shortcuts(&d, width, height));
 	AddCtrl("", res_sizer);
 	AddCtrl(_("Color:"), color_sizer);
-	AddCtrl(_("Frame rate (fps):"), spin_ctrl(&d, .1, 1000.0, &fps));
+	wxTextValidator fpsVal(wxFILTER_INCLUDE_CHAR_LIST, &fps);
+	fpsVal.SetCharIncludes("0123456789./");
+	AddCtrl(_("Frame rate (fps):"), new wxTextCtrl(&d, -1, "", wxDefaultPosition, wxDefaultSize, 0, fpsVal));
 	AddCtrl(_("Duration (frames):"), spin_ctrl(&d, 2, 36000000, &length)); // Ten hours of 1k FPS
 	AddCtrl("", length_display = new wxStaticText(&d, -1, ""));
 
@@ -132,17 +131,19 @@ DialogDummyVideo::DialogDummyVideo(wxWindow *parent)
 	main_sizer->Add(new wxStaticLine(&d, wxHORIZONTAL), wxSizerFlags().HorzBorder().Expand());
 	main_sizer->Add(btn_sizer, wxSizerFlags().Expand().Border());
 
-	UpdateLengthDisplay();
+	btn_sizer->GetAffirmativeButton()->Enable(UpdateLengthDisplay());
 
 	d.SetSizerAndFit(main_sizer);
 	d.CenterOnParent();
 
 	d.Bind(wxEVT_COMBOBOX, &DialogDummyVideo::OnResolutionShortcut, this);
 	color_btn->Bind(EVT_COLOR, [=](ValueEvent<agi::Color>& e) { color = e.Get(); });
-	d.Bind(wxEVT_SPINCTRL, [&](wxCommandEvent&) {
+	auto on_update = [&, btn_sizer](wxCommandEvent&) {
 		d.TransferDataFromWindow();
-		UpdateLengthDisplay();
+		btn_sizer->GetAffirmativeButton()->Enable(UpdateLengthDisplay());
-	});
+	};
+	d.Bind(wxEVT_SPINCTRL, on_update);
+	d.Bind(wxEVT_TEXT, on_update);
 }
 
 static void add_label(wxWindow *parent, wxSizer *sizer, wxString const& label) {
@@ -166,8 +167,16 @@ void DialogDummyVideo::OnResolutionShortcut(wxCommandEvent &e) {
 	d.TransferDataToWindow();
 }
 
-void DialogDummyVideo::UpdateLengthDisplay() {
+bool DialogDummyVideo::UpdateLengthDisplay() {
-	length_display->SetLabel(fmt_tl("Resulting duration: %s", agi::Time(length / fps * 1000).GetAssFormatted(true)));
+	std::string dur = "-";
+	bool valid = false;
+	agi::vfr::Framerate fr;
+	if (DummyVideoProvider::TryParseFramerate(from_wx(fps), fr)) {
+		dur = agi::Time(fr.TimeAtFrame(length)).GetAssFormatted(true);
+		valid = true;
+	}
+	length_display->SetLabel(fmt_tl("Resulting duration: %s", dur));
+	return valid;
 }
 }
 
@@ -176,12 +185,12 @@ std::string CreateDummyVideo(wxWindow *parent) {
 	if (dlg.d.ShowModal() != wxID_OK)
 		return "";
 
-	OPT_SET("Video/Dummy/FPS")->SetDouble(dlg.fps);
+	OPT_SET("Video/Dummy/FPS String")->SetString(from_wx(dlg.fps));
 	OPT_SET("Video/Dummy/Last/Width")->SetInt(dlg.width);
 	OPT_SET("Video/Dummy/Last/Height")->SetInt(dlg.height);
 	OPT_SET("Video/Dummy/Last/Length")->SetInt(dlg.length);
 	OPT_SET("Colour/Video Dummy/Last Colour")->SetColor(dlg.color);
 	OPT_SET("Video/Dummy/Pattern")->SetBool(dlg.pattern);
 
-	return DummyVideoProvider::MakeFilename(dlg.fps, dlg.length, dlg.width, dlg.height, dlg.color, dlg.pattern);
+	return DummyVideoProvider::MakeFilename(from_wx(dlg.fps), dlg.length, dlg.width, dlg.height, dlg.color, dlg.pattern);
 }

src/libresrc/default_config.json

@@ -604,7 +604,7 @@
 			"Maximized" : false
 		},
 		"Dummy" : {
-			"FPS" : 23.975999999999999091,
+			"FPS String" : "24000/1001",
 			"Last" : {
 				"Height" : 720,
 				"Length" : 40000,

src/libresrc/osx/default_config.json

@@ -604,7 +604,7 @@
 			"Maximized" : false
 		},
 		"Dummy" : {
-			"FPS" : 23.975999999999999091,
+			"FPS String" : "24000/1001",
 			"Last" : {
 				"Height" : 720,
 				"Length" : 40000,

src/video_provider_dummy.cpp

@@ -38,6 +38,7 @@
 #include "video_frame.h"
 
 #include <libaegisub/color.h>
+#include <libaegisub/exception.h>
 #include <libaegisub/make_unique.h>
 #include <libaegisub/split.h>
 #include <libaegisub/util.h>
@@ -51,7 +52,7 @@
 #include <boost/gil/gil_all.hpp>
 #endif
 
-DummyVideoProvider::DummyVideoProvider(double fps, int frames, int width, int height, agi::Color colour, bool pattern)
+DummyVideoProvider::DummyVideoProvider(agi::vfr::Framerate fps, int frames, int width, int height, agi::Color colour, bool pattern)
 : framecount(frames)
 , fps(fps)
 , width(width)
@@ -91,8 +92,37 @@ DummyVideoProvider::DummyVideoProvider(double fps, int frames, int width, int he
 	}
 }
 
-std::string DummyVideoProvider::MakeFilename(double fps, int frames, int width, int height, agi::Color colour, bool pattern) {
+bool DummyVideoProvider::TryParseFramerate(std::string fps_string, agi::vfr::Framerate &fps) {
-	return agi::format("?dummy:%f:%d:%d:%d:%d:%d:%d:%s", fps, frames, width, height, (int)colour.r, (int)colour.g, (int)colour.b, (pattern ? "c" : ""));
+	using agi::util::try_parse;
+
+	double fps_double;
+	if (try_parse(fps_string, &fps_double)) {
+		try {
+			fps = fps_double;
+		} catch (agi::vfr::InvalidFramerate) {
+			return false;
+		}
+	} else {
+		std::vector<std::string> numden;
+		agi::Split(numden, fps_string, '/');
+		if (numden.size() != 2)
+			return false;
+
+		int num, den;
+		if (!try_parse(numden[0], &num)) return false;
+		if (!try_parse(numden[1], &den)) return false;
+
+		try {
+			fps = agi::vfr::Framerate(num, den);
+		} catch (agi::vfr::InvalidFramerate) {
+			return false;
+		}
+	}
+	return true;
+}
+
+std::string DummyVideoProvider::MakeFilename(std::string fps, int frames, int width, int height, agi::Color colour, bool pattern) {
+	return agi::format("?dummy:%s:%d:%d:%d:%d:%d:%d:%s", fps, frames, width, height, (int)colour.r, (int)colour.g, (int)colour.b, (pattern ? "c" : ""));
 }
 
 void DummyVideoProvider::GetFrame(int, VideoFrame &frame) {
@@ -105,21 +135,23 @@ void DummyVideoProvider::GetFrame(int, VideoFrame &frame) {
 
 namespace agi { class BackgroundRunner; }
 std::unique_ptr<VideoProvider> CreateDummyVideoProvider(agi::fs::path const& filename, std::string const&, agi::BackgroundRunner *) {
-	if (!boost::starts_with(filename.string(), "?dummy"))
+	// Use filename.generic_string here so forward slashes stay as they are
+	if (!boost::starts_with(filename.generic_string(), "?dummy"))
 		return {};
 
 	std::vector<std::string> toks;
-	auto const& fields = filename.string().substr(7);
+	auto const& fields = filename.generic_string().substr(7);
 	agi::Split(toks, fields, ':');
 	if (toks.size() != 8)
 		throw VideoOpenError("Too few fields in dummy video parameter list");
 
 	size_t i = 0;
-	double fps;
 	int frames, width, height, red, green, blue;
+	agi::vfr::Framerate fps;
 
 	using agi::util::try_parse;
-	if (!try_parse(toks[i++], &fps))    throw VideoOpenError("Unable to parse fps field in dummy video parameter list");
+	if (!DummyVideoProvider::TryParseFramerate(toks[i++], fps))
+	   throw VideoOpenError("Unable to parse fps field in dummy video parameter list");
 	if (!try_parse(toks[i++], &frames)) throw VideoOpenError("Unable to parse framecount field in dummy video parameter list");
 	if (!try_parse(toks[i++], &width))  throw VideoOpenError("Unable to parse width field in dummy video parameter list");
 	if (!try_parse(toks[i++], &height)) throw VideoOpenError("Unable to parse height field in dummy video parameter list");

src/video_provider_dummy.h

@@ -58,11 +58,12 @@ public:
 	/// @param height Height in pixels of the dummy video
 	/// @param colour Primary colour of the dummy video
 	/// @param pattern Use a checkerboard pattern rather than a solid colour
-	DummyVideoProvider(double fps, int frames, int width, int height, agi::Color colour, bool pattern);
+	DummyVideoProvider(agi::vfr::Framerate fps, int frames, int width, int height, agi::Color colour, bool pattern);
 
 	/// Make a fake filename which when passed to the constructor taking a
 	/// string will result in a video with the given parameters
-	static std::string MakeFilename(double fps, int frames, int width, int height, agi::Color colour, bool pattern);
+	static std::string MakeFilename(std::string fps, int frames, int width, int height, agi::Color colour, bool pattern);
+	static bool TryParseFramerate(std::string fps_string, agi::vfr::Framerate &fps);
 
 	void GetFrame(int n, VideoFrame &frame) override;
 	void SetColorSpace(std::string const&) override { }