From 495fd7d8c4351f7efd3c6edde3e82fabf0910643 Mon Sep 17 00:00:00 2001 From: Dave Patrick Caberto Date: Fri, 29 Mar 2024 18:36:31 +0800 Subject: [PATCH] refactor: drop FramerateOption --- Cargo.lock | 1 - Cargo.toml | 1 - src/framerate.rs | 86 +++++++++++++++++++++ src/framerate_option.rs | 159 -------------------------------------- src/main.rs | 2 +- src/preferences_dialog.rs | 36 ++++----- src/window/mod.rs | 7 +- 7 files changed, 108 insertions(+), 184 deletions(-) create mode 100644 src/framerate.rs delete mode 100644 src/framerate_option.rs diff --git a/Cargo.lock b/Cargo.lock index 62a121a5..1c802bcd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1029,7 +1029,6 @@ dependencies = [ "gstreamer", "gtk4", "libadwaita", - "num-traits", "once_cell", "serde", "serde_yaml", diff --git a/Cargo.toml b/Cargo.toml index 03854ca4..571c57d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,6 @@ gst-plugin-gtk4 = { version = "0.12", features = [ "x11glx", ] } gtk = { package = "gtk4", version = "0.8", features = ["gnome_46"] } -num-traits = "0.2" once_cell = "1.19.0" serde_yaml = "0.9.31" serde = { version = "1.0.196", features = ["derive"] } diff --git a/src/framerate.rs b/src/framerate.rs new file mode 100644 index 00000000..f4704501 --- /dev/null +++ b/src/framerate.rs @@ -0,0 +1,86 @@ +use std::sync::OnceLock; + +use gtk::{gio, glib::BoxedAnyObject}; + +use crate::settings::Settings; + +/// Built-in framerates. +fn builtins() -> &'static [gst::Fraction] { + static BUILTINS: OnceLock> = OnceLock::new(); + + BUILTINS.get_or_init(|| { + vec![ + gst::Fraction::from_integer(10), + gst::Fraction::from_integer(20), + gst::Fraction::from_integer(24), + gst::Fraction::from_integer(25), + gst::Fraction::new(30_000, 1001), // 29.97 + gst::Fraction::from_integer(30), + gst::Fraction::from_integer(48), + gst::Fraction::from_integer(50), + gst::Fraction::new(60_000, 1001), // 59.94 + gst::Fraction::from_integer(60), + ] + }) +} + +/// Returns a model of type `BoxedAnyObject`. +/// +/// This appends the current framerate in the settings if it does not match any built-in ones. +pub fn builtins_model(settings: &Settings) -> gio::ListStore { + let list_store = gio::ListStore::new::(); + + let items = builtins() + .iter() + .map(|fraction| BoxedAnyObject::new(*fraction)) + .collect::>(); + list_store.splice(0, 0, &items); + + let other = settings.framerate(); + if !builtins().contains(&other) { + list_store.append(&BoxedAnyObject::new(other)); + } + + list_store +} + +/// Formats a framerate in a human-readable format. +pub fn format(framerate: gst::Fraction) -> String { + let reduced = framerate.reduced(); + + if reduced.is_integer() { + return reduced.numer().to_string(); + } + + let float = *reduced.numer() as f64 / *reduced.denom() as f64; + format!("{:.2}", float) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[track_caller] + fn assert_simplified(framerate: gst::Fraction) { + let reduced = framerate.0.reduced(); + + assert_eq!( + (framerate.numer(), framerate.denom()), + (*reduced.numer(), *reduced.denom()) + ); + } + + #[test] + fn simplified() { + for framerate in builtins() { + assert_simplified(*framerate); + } + } + + #[test] + fn test_format() { + assert_eq!(format(gst::Fraction::from_integer(24)), "24"); + assert_eq!(format(gst::Fraction::new(30_000, 1001)), "29.97"); + assert_eq!(format(gst::Fraction::new(60_000, 1001)), "59.94"); + } +} diff --git a/src/framerate_option.rs b/src/framerate_option.rs deleted file mode 100644 index f2dba874..00000000 --- a/src/framerate_option.rs +++ /dev/null @@ -1,159 +0,0 @@ -use std::fmt; - -use gtk::{gio, glib::BoxedAnyObject}; -use num_traits::Signed; - -use crate::settings::Settings; - -/// The available options for the framerate. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum FramerateOption { - _10, - _20, - _24, - _25, - _29_97, - _30, - _48, - _50, - _59_94, - _60, - Other(gst::Fraction), -} - -impl FramerateOption { - fn all_except_other() -> [Self; 10] { - [ - Self::_10, - Self::_20, - Self::_24, - Self::_25, - Self::_29_97, - Self::_30, - Self::_48, - Self::_50, - Self::_59_94, - Self::_60, - ] - } - - /// Returns a model of type `BoxedAnyObject`. This contains `Other` if the current settings framerate - /// does not match any of the predefined options. - pub fn model(settings: &Settings) -> gio::ListStore { - let list_store = gio::ListStore::new::(); - - let items = Self::all_except_other() - .into_iter() - .map(BoxedAnyObject::new) - .collect::>(); - list_store.splice(0, 0, &items); - - if let other @ Self::Other(_) = Self::from_framerate(settings.framerate()) { - list_store.append(&BoxedAnyObject::new(other)); - } - - list_store - } - - /// Returns the corresponding `FramerateOption` for the given framerate. - pub fn from_framerate(framerate: gst::Fraction) -> Self { - // This must be updated if an option is added or removed. - let epsilon = gst::Fraction::new_raw(1, 100); - - Self::all_except_other() - .into_iter() - .find(|o| (o.as_framerate() - framerate).abs() < epsilon.0) - .unwrap_or(Self::Other(framerate)) - } - - /// Converts a `FramerateOption` to a framerate. - pub const fn as_framerate(self) -> gst::Fraction { - let (numerator, denominator) = match self { - Self::_10 => (10, 1), - Self::_20 => (20, 1), - Self::_24 => (24, 1), - Self::_25 => (25, 1), - Self::_29_97 => (30_000, 1001), - Self::_30 => (30, 1), - Self::_48 => (48, 1), - Self::_50 => (50, 1), - Self::_59_94 => (60_000, 1001), - Self::_60 => (60, 1), - Self::Other(framerate) => return framerate, - }; - gst::Fraction::new_raw(numerator, denominator) - } -} - -impl fmt::Display for FramerateOption { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(match self { - Self::_10 => "10", - Self::_20 => "20", - Self::_24 => "24", - Self::_25 => "25", - Self::_29_97 => "29.97", - Self::_30 => "30", - Self::_48 => "48", - Self::_50 => "50", - Self::_59_94 => "59.94", - Self::_60 => "60", - Self::Other(framerate) => return write!(f, "{}", framerate), - }) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[track_caller] - fn assert_simplified(framerate: gst::Fraction) { - let reduced = framerate.0.reduced(); - - assert_eq!( - (framerate.numer(), framerate.denom()), - (*reduced.numer(), *reduced.denom()) - ); - } - - #[test] - fn simplified() { - for option in FramerateOption::all_except_other() { - assert_simplified(option.as_framerate()); - } - } - - #[track_caller] - fn test_framerate(framerate: gst::Fraction, expected: FramerateOption) { - assert_eq!(FramerateOption::from_framerate(framerate), expected); - } - - #[test] - fn equivalence() { - test_framerate( - gst::Fraction::from_integer(5), - FramerateOption::Other(gst::Fraction::from_integer(5)), - ); - test_framerate(gst::Fraction::from_integer(10), FramerateOption::_10); - test_framerate(gst::Fraction::from_integer(20), FramerateOption::_20); - test_framerate(gst::Fraction::from_integer(24), FramerateOption::_24); - test_framerate(gst::Fraction::from_integer(25), FramerateOption::_25); - test_framerate( - gst::Fraction::approximate_f64(29.97).unwrap(), - FramerateOption::_29_97, - ); - test_framerate(gst::Fraction::from_integer(30), FramerateOption::_30); - test_framerate(gst::Fraction::from_integer(48), FramerateOption::_48); - test_framerate(gst::Fraction::from_integer(50), FramerateOption::_50); - test_framerate( - gst::Fraction::approximate_f64(59.94).unwrap(), - FramerateOption::_59_94, - ); - test_framerate(gst::Fraction::from_integer(60), FramerateOption::_60); - test_framerate( - gst::Fraction::from_integer(120), - FramerateOption::Other(gst::Fraction::from_integer(120)), - ); - } -} diff --git a/src/main.rs b/src/main.rs index 7ecd0d47..fd544f49 100644 --- a/src/main.rs +++ b/src/main.rs @@ -31,7 +31,7 @@ mod config; mod device; mod experimental; mod format_time; -mod framerate_option; +mod framerate; mod help; mod i18n; mod item_row; diff --git a/src/preferences_dialog.rs b/src/preferences_dialog.rs index e165c329..e70c332f 100644 --- a/src/preferences_dialog.rs +++ b/src/preferences_dialog.rs @@ -8,8 +8,7 @@ use gtk::{ }; use crate::{ - experimental::Feature, framerate_option::FramerateOption, item_row::ItemRow, profile::Profile, - settings::Settings, + experimental::Feature, framerate, item_row::ItemRow, profile::Profile, settings::Settings, }; const ROW_SELECTED_ITEM_NOTIFY_HANDLER_ID_KEY: &str = "kooha-row-selected-item-notify-handler-id"; @@ -114,12 +113,11 @@ mod imp { self.framerate_row .connect_selected_item_notify(clone!(@weak obj => move |row| { if let Some(item) = row.selected_item() { - let framerate_option = item + let framerate = item .downcast_ref::() .unwrap() - .borrow::(); - obj.settings() - .set_framerate(framerate_option.as_framerate()); + .borrow::(); + obj.settings().set_framerate(*framerate); } })); } @@ -182,18 +180,18 @@ impl PreferencesDialog { let imp = self.imp(); let settings = self.settings(); - let framerate_option = FramerateOption::from_framerate(settings.framerate()); + let framerate = settings.framerate(); let model = imp.framerate_row.model().unwrap(); let position = model .iter::() - .position(|item| *item.unwrap().borrow::() == framerate_option); + .position(|item| *item.unwrap().borrow::() == framerate); if let Some(position) = position { imp.framerate_row.set_selected(position as u32); } else { tracing::error!( "Active framerate `{:?}` was not found on framerate model", - framerate_option + framerate ); } } @@ -210,11 +208,11 @@ impl PreferencesDialog { let item = list_item.item().unwrap(); let item_row = list_item.child().unwrap().downcast::().unwrap(); - let framerate_option = item + let framerate = item .downcast_ref::() .unwrap() - .borrow::(); - item_row.set_title(framerate_option.to_string()); + .borrow::(); + item_row.set_title(framerate::format(*framerate)); unsafe { list_item.set_data( @@ -239,7 +237,7 @@ impl PreferencesDialog { }), ))); - let framerate_model = FramerateOption::model(&settings); + let framerate_model = framerate::builtins_model(&settings); imp.framerate_row.set_model(Some(&framerate_model)); imp.profile_row.set_factory(Some(&row_factory( @@ -361,14 +359,16 @@ fn update_framerate_row_shows_warning_icon(settings: &Settings, list_item: >k: let item_row = list_item.child().unwrap().downcast::().unwrap(); let item = list_item.item().unwrap(); - let framerate_option = item + let framerate = item .downcast_ref::() .unwrap() - .borrow::(); + .borrow::(); - item_row.set_shows_warning_icon(settings.profile().is_some_and(|profile| { - framerate_option.as_framerate() > profile.suggested_max_framerate() - })); + item_row.set_shows_warning_icon( + settings + .profile() + .is_some_and(|profile| *framerate > profile.suggested_max_framerate()), + ); } /// Returns `Some` if the object is a `Profile`, otherwise `None`, if the object is a `NoneProfile`. diff --git a/src/window/mod.rs b/src/window/mod.rs index 67b714ac..22964ed4 100644 --- a/src/window/mod.rs +++ b/src/window/mod.rs @@ -15,8 +15,7 @@ use self::{progress_icon::ProgressIcon, toggle_button::ToggleButton}; use crate::{ cancelled::Cancelled, config::PROFILE, - format_time, - framerate_option::FramerateOption, + format_time, framerate, help::ContextWithHelp, preferences_dialog::PreferencesDialog, recording::{NoProfileError, Recording, RecordingState}, @@ -533,10 +532,10 @@ impl Window { let profile_text = settings .profile() .map_or_else(|| gettext("None"), |profile| profile.name().to_string()); - let framerate_option = FramerateOption::from_framerate(settings.framerate()); + let framerate_text = framerate::format(settings.framerate()); imp.title - .set_subtitle(&format!("{} • {} FPS", profile_text, framerate_option)); + .set_subtitle(&format!("{} • {} FPS", profile_text, framerate_text)); } fn update_audio_actions(&self) {