Use an enum for the predefined aspect ratio types

aegisub/src/command/video.cpp

@@ -100,12 +100,12 @@ struct video_aspect_cinematic : public validator_video_loaded {
 	CMD_TYPE(COMMAND_VALIDATE | COMMAND_RADIO)
 
 	bool IsActive(const agi::Context *c) {
-		return c->videoController->GetAspectRatioType() == 3;
+		return c->videoController->GetAspectRatioType() == AspectRatio::Cinematic;
 	}
 
 	void operator()(agi::Context *c) {
 		c->videoController->Stop();
-		c->videoController->SetAspectRatio(3);
+		c->videoController->SetAspectRatio(AspectRatio::Cinematic);
 		wxGetApp().frame->SetDisplayMode(1,-1);
 	}
 };
@@ -119,7 +119,7 @@ struct video_aspect_custom : public validator_video_loaded {
 	CMD_TYPE(COMMAND_VALIDATE | COMMAND_RADIO)
 
 	bool IsActive(const agi::Context *c) {
-		return c->videoController->GetAspectRatioType() == 4;
+		return c->videoController->GetAspectRatioType() == AspectRatio::Custom;
 	}
 
 	void operator()(agi::Context *c) {
@@ -148,7 +148,7 @@ struct video_aspect_custom : public validator_video_loaded {
 		if (numval < 0.5 || numval > 5.0)
 			wxMessageBox(_("Invalid value! Aspect ratio must be between 0.5 and 5.0."),_("Invalid Aspect Ratio"),wxOK | wxICON_ERROR | wxCENTER);
 		else {
-			c->videoController->SetAspectRatio(4, numval);
+			c->videoController->SetAspectRatio(numval);
 			wxGetApp().frame->SetDisplayMode(1,-1);
 		}
 	}
@@ -163,12 +163,12 @@ struct video_aspect_default : public validator_video_loaded {
 	CMD_TYPE(COMMAND_VALIDATE | COMMAND_RADIO)
 
 	bool IsActive(const agi::Context *c) {
-		return c->videoController->GetAspectRatioType() == 0;
+		return c->videoController->GetAspectRatioType() == AspectRatio::Default;
 	}
 
 	void operator()(agi::Context *c) {
 		c->videoController->Stop();
-		c->videoController->SetAspectRatio(0);
+		c->videoController->SetAspectRatio(AspectRatio::Default);
 		wxGetApp().frame->SetDisplayMode(1,-1);
 	}
 };
@@ -182,12 +182,12 @@ struct video_aspect_full : public validator_video_loaded {
 	CMD_TYPE(COMMAND_VALIDATE | COMMAND_RADIO)
 
 	bool IsActive(const agi::Context *c) {
-		return c->videoController->GetAspectRatioType() == 1;
+		return c->videoController->GetAspectRatioType() == AspectRatio::Fullscreen;
 	}
 
 	void operator()(agi::Context *c) {
 		c->videoController->Stop();
-		c->videoController->SetAspectRatio(1);
+		c->videoController->SetAspectRatio(AspectRatio::Fullscreen);
 		wxGetApp().frame->SetDisplayMode(1,-1);
 	}
 };
@@ -201,12 +201,12 @@ struct video_aspect_wide : public validator_video_loaded {
 	CMD_TYPE(COMMAND_VALIDATE | COMMAND_RADIO)
 
 	bool IsActive(const agi::Context *c) {
-		return c->videoController->GetAspectRatioType() == 2;
+		return c->videoController->GetAspectRatioType() == AspectRatio::Widescreen;
 	}
 
 	void operator()(agi::Context *c) {
 		c->videoController->Stop();
-		c->videoController->SetAspectRatio(2);
+		c->videoController->SetAspectRatio(AspectRatio::Widescreen);
 		wxGetApp().frame->SetDisplayMode(1,-1);
 	}
 };

aegisub/src/frame_main.cpp

@@ -722,17 +722,17 @@ void FrameMain::OnSubtitlesOpen() {
 		if (context->videoController->IsLoaded()) {
 			context->videoController->JumpToFrame(context->ass->GetScriptInfoAsInt("Video Position"));
 
-			int videoAr = 0;
-			double videoArValue = 0.;
 			std::string arString = context->ass->GetScriptInfo("Video Aspect Ratio");
 			if (boost::starts_with(arString, "c")) {
-				videoAr = 4;
+				double ar = 0.;
-				agi::util::try_parse(arString.substr(1), &videoArValue);
+				agi::util::try_parse(arString.substr(1), &ar);
+				context->videoController->SetAspectRatio(ar);
+			}
+			else {
+				int ar = 0;
+				if (agi::util::try_parse(arString.substr(1), &ar) && ar >= 0 && ar < 4)
+					context->videoController->SetAspectRatio((AspectRatio)ar);
 			}
-			else
-				agi::util::try_parse(arString.substr(1), &videoAr);
-
-			context->videoController->SetAspectRatio(videoAr, videoArValue);
 
 			double videoZoom = 0.;
 			if (agi::util::try_parse(context->ass->GetScriptInfo("Video Zoom Percent"), &videoZoom))

aegisub/src/video_context.cpp

@@ -63,7 +63,7 @@ VideoContext::VideoContext()
 , end_frame(0)
 , frame_n(0)
 , ar_value(1.)
-, ar_type(0)
+, ar_type(AspectRatio::Default)
 , has_subtitles(false)
 , playAudioOnStep(OPT_GET("Audio/Plays When Stepping Video"))
 , no_amend(false)
@@ -183,7 +183,7 @@ void VideoContext::SetVideo(const agi::fs::path &filename) {
 		// Set aspect ratio
 		double dar = video_provider->GetDAR();
 		if (dar > 0)
-			SetAspectRatio(4, dar);
+			SetAspectRatio(dar);
 
 		// Set filename
 		config::mru->Add("Video", filename);
@@ -253,10 +253,10 @@ void VideoContext::OnSubtitlesSave() {
 	}
 
 	std::string ar;
-	if (ar_type == 4)
+	if (ar_type == AspectRatio::Custom)
 		ar = "c" + std::to_string(ar_value);
 	else
-		ar = "c" + std::to_string(ar_type);
+		ar = std::to_string((int)ar_type);
 
 	context->ass->SetScriptInfo("Video File", config::path->MakeRelative(video_filename, "?script").generic_string());
 	context->ass->SetScriptInfo("YCbCr Matrix", video_provider->GetColorSpace());
@@ -371,22 +371,28 @@ void VideoContext::OnPlayTimer(wxTimerEvent &) {
 	}
 }
 
-double VideoContext::GetARFromType(int type) const {
+double VideoContext::GetARFromType(AspectRatio type) const {
-	if (type == 0) return (double)GetWidth()/(double)GetHeight();
+	switch (type) {
-	if (type == 1) return 4.0/3.0;
+		case AspectRatio::Default: return (double)GetWidth()/(double)GetHeight();
-	if (type == 2) return 16.0/9.0;
+		case AspectRatio::Fullscreen: return 4.0/3.0;
-	if (type == 3) return 2.35;
+		case AspectRatio::Widescreen: return 16.0/9.0;
-	return 1.0;  //error
+		case AspectRatio::Cinematic: return 2.35;
+	}
+	throw agi::InternalError("Bad AR type", nullptr);
 }
 
-void VideoContext::SetAspectRatio(int type, double value) {
+void VideoContext::SetAspectRatio(double value) {
-	if (type != 4) value = GetARFromType(type);
+	ar_type = AspectRatio::Custom;
-
-	ar_type = type;
 	ar_value = mid(.5, value, 5.);
 	ARChange(ar_type, ar_value);
 }
 
+void VideoContext::SetAspectRatio(AspectRatio type) {
+	ar_value = mid(.5, GetARFromType(type), 5.);
+	ar_type = type;
+	ARChange(ar_type, ar_value);
+}
+
 void VideoContext::LoadKeyframes(agi::fs::path const& filename) {
 	if (filename == keyframes_filename || filename.empty()) return;
 	try {

aegisub/src/video_context.h

@@ -57,6 +57,14 @@ namespace agi {
 	class OptionValue;
 }
 
+enum class AspectRatio {
+	Default = 0,
+	Fullscreen,
+	Widescreen,
+	Cinematic,
+	Custom
+};
+
 /// @class VideoContext
 /// @brief Manage a bunch of things vaguely related to video playback
 ///
@@ -73,7 +81,7 @@ class VideoContext : public wxEvtHandler {
 	/// New timecodes opened (new timecode data)
 	agi::signal::Signal<agi::vfr::Framerate const&> TimecodesOpen;
 	/// Aspect ratio was changed (type, value)
-	agi::signal::Signal<int, double> ARChange;
+	agi::signal::Signal<AspectRatio, double> ARChange;
 
 	agi::Context *context;
 
@@ -115,11 +123,8 @@ class VideoContext : public wxEvtHandler {
 	/// overridden by the user
 	double ar_value;
 
-	/// @brief The current AR type
+	/// The current AR type
-	///
+	AspectRatio ar_type;
-	/// 0 is square pixels; 1-3 are predefined ARs; 4 is custom, where the real
-	/// AR is in arValue
-	int ar_type;
 
 	/// Does the currently loaded video file have subtitles muxed into it?
 	bool has_subtitles;
@@ -202,15 +207,17 @@ public:
 	int GetFrameN() const { return frame_n; }
 
 	/// Get the actual aspect ratio from a predefined AR type
-	double GetARFromType(int type) const;
+	double GetARFromType(AspectRatio type) const;
 
 	/// Override the aspect ratio of the currently loaded video
-	/// @param type Aspect ratio type from 0-4
+	void SetAspectRatio(double value);
-	/// @param value If type is 4 (custom), the aspect ratio to use
+
-	void SetAspectRatio(int type, double value=1.0);
+	/// Override the aspect ratio of the currently loaded video
+	/// @param type Predefined type to set the AR to. Must not be Custom.
+	void SetAspectRatio(AspectRatio type);
 
 	/// Get the current AR type
-	int GetAspectRatioType() const { return ar_type; }
+	AspectRatio GetAspectRatioType() const { return ar_type; }
 
 	/// Get the current aspect ratio of the video
 	double GetAspectRatioValue() const { return ar_value; }

aegisub/src/video_display.cpp

@@ -281,9 +281,9 @@ void VideoDisplay::PositionVideo() {
 		int vidW = con->videoController->GetWidth();
 		int vidH = con->videoController->GetHeight();
 
-		int arType = con->videoController->GetAspectRatioType();
+		AspectRatio arType = con->videoController->GetAspectRatioType();
 		double displayAr = double(viewport_width) / viewport_height;
-		double videoAr = arType == 0 ? double(vidW) / vidH : con->videoController->GetAspectRatioValue();
+		double videoAr = arType == AspectRatio::Default ? double(vidW) / vidH : con->videoController->GetAspectRatioValue();
 
 		// Window is wider than video, blackbox left/right
 		if (displayAr - videoAr > 0.01f) {
@@ -310,7 +310,7 @@ void VideoDisplay::UpdateSize() {
 
 	videoSize.Set(con->videoController->GetWidth(), con->videoController->GetHeight());
 	videoSize *= zoomValue;
-	if (con->videoController->GetAspectRatioType() != 0)
+	if (con->videoController->GetAspectRatioType() != AspectRatio::Default)
 		videoSize.SetWidth(videoSize.GetHeight() * con->videoController->GetAspectRatioValue());
 
 	wxEventBlocker blocker(this);