[pitivi/1.0] ui: Add formatting functions and refactor models

[Alexandru Băluț <alexbalut src gnome org>]

commit 0914d2e44b535b3afe1936ffab90794b24e19976
Author: yatinmaan <yatinmaan1 gmail com>
Date:   Fri Mar 2 13:26:08 2018 +0530

    ui: Add formatting functions and refactor models
    
    Add formatting functions for framerates, audiorates & audiochannels
    
    Generate models using formatting functions to ensure a consistent UI.
    Remove display_aspect_ratios and pixel_aspect_ratio models, they were
    not used anywhere.
    
    Fixes https://gitlab.gnome.org/GNOME/pitivi/issues/2160

 pitivi/dialogs/clipmediaprops.py |  55 ++++++-------
 pitivi/project.py                |   2 -
 pitivi/utils/ui.py               | 171 ++++++++++++++++++++-------------------
 tests/test_utils.py              |  48 ++++++++++-
 4 files changed, 155 insertions(+), 121 deletions(-)
---
diff --git a/pitivi/dialogs/clipmediaprops.py b/pitivi/dialogs/clipmediaprops.py
index 1017ee29..13248392 100644
--- a/pitivi/dialogs/clipmediaprops.py
+++ b/pitivi/dialogs/clipmediaprops.py
@@ -22,11 +22,9 @@ from gi.repository import Gst
 from gi.repository import Gtk
 
 from pitivi.configure import get_ui_dir
-from pitivi.utils.ui import audio_channels
-from pitivi.utils.ui import audio_rates
-from pitivi.utils.ui import frame_rates
-from pitivi.utils.ui import get_value_from_model
-from pitivi.utils.ui import pixel_aspect_ratios
+from pitivi.utils.ui import format_audiochannels
+from pitivi.utils.ui import format_audiorate
+from pitivi.utils.ui import format_framerate
 
 
 class ClipMediaPropsDialog(object):
@@ -80,10 +78,8 @@ class ClipMediaPropsDialog(object):
         # in the UI, instead of acting as if there was only one. But that means
         # dynamically creating checkboxes and labels in a table and such.
         for stream in self.audio_streams:
-            self.channels.set_text(
-                get_value_from_model(audio_channels, stream.get_channels()))
-            self.sample_rate.set_text(
-                get_value_from_model(audio_rates, stream.get_sample_rate()))
+            self.channels.set_text(format_audiochannels(stream))
+            self.sample_rate.set_text(format_audiorate(stream))
             self.has_audio = True
             break
 
@@ -92,35 +88,32 @@ class ClipMediaPropsDialog(object):
             self.size_height.set_text(str(stream.get_height()))
             self.is_image = stream.is_image()
             if not self.is_image:
-                # When gst returns a crazy framerate such as 0/1, that either
-                # means it couldn't determine it, or it is a variable framerate
-                framerate_num = stream.get_framerate_num()
-                framerate_denom = stream.get_framerate_denom()
-                if framerate_num != 0 and framerate_denom != 0:
-                    self.frame_rate.set_text(
-                        get_value_from_model(frame_rates,
-                            Gst.Fraction(framerate_num, framerate_denom)
-                        ))
-                    if (framerate_num / framerate_denom) > 500:
-                        # Sometimes you have "broken" 1000fps clips (WebM files
-                        # from YouTube, for example), but it could also be a
-                        # real framerate, so just uncheck instead of disabling:
-                        self.framerate_checkbutton.set_active(False)
+                num = stream.get_framerate_num()
+                denom = stream.get_framerate_denom()
+                if denom != 0:
+                    fps = num / denom
                 else:
-                    foo = str(framerate_num) + "/" + str(framerate_denom)
-                    # Translators: a label showing an invalid framerate value
-                    self.frame_rate.set_text(_("invalid (%s fps)") % foo)
+                    fps = 0
+
+                if fps > 500:
+                    # Sometimes you have "broken" 1000fps clips (WebM files
+                    # from YouTube, for example), but it could also be a
+                    # real framerate, so just uncheck instead of disabling:
+                    self.framerate_checkbutton.set_active(False)
+                elif fps == 0:
+                    self.frame_rate.set_text(_("Variable"))
                     self.framerate_checkbutton.set_active(False)
                     # For consistency, insensitize the checkbox AND value
                     # labels
                     self.framerate_checkbutton.set_sensitive(False)
                     self.frame_rate.set_sensitive(False)
+                else:
+                    self.frame_rate.set_text(format_framerate(stream))
 
-                # Aspect ratio (probably?) doesn't need such a check:
-                self.aspect_ratio.set_text(
-                    get_value_from_model(pixel_aspect_ratios, Gst.Fraction(
-                        stream.get_par_num(),
-                        stream.get_par_denom())))
+                par_num = stream.get_par_num()
+                par_denom = stream.get_par_denom()
+                aspect_ratio = "{0:n}:{1:n}".format(par_num, par_denom)
+                self.aspect_ratio.set_text(aspect_ratio)
 
             self.has_video = True
             break
diff --git a/pitivi/project.py b/pitivi/project.py
index fa0a4417..bad39d9f 100644
--- a/pitivi/project.py
+++ b/pitivi/project.py
@@ -51,10 +51,8 @@ from pitivi.utils.ripple_update_group import RippleUpdateGroup
 from pitivi.utils.ui import audio_channels
 from pitivi.utils.ui import audio_rates
 from pitivi.utils.ui import beautify_time_delta
-from pitivi.utils.ui import display_aspect_ratios
 from pitivi.utils.ui import frame_rates
 from pitivi.utils.ui import get_combo_value
-from pitivi.utils.ui import pixel_aspect_ratios
 from pitivi.utils.ui import set_combo_value
 from pitivi.utils.ui import SPACING
 from pitivi.utils.validate import create_monitor
diff --git a/pitivi/utils/ui.py b/pitivi/utils/ui.py
index 3a6ab023..1461b489 100644
--- a/pitivi/utils/ui.py
+++ b/pitivi/utils/ui.py
@@ -160,14 +160,62 @@ TIMELINE_CSS = """
         'trimbar_focused': os.path.join(get_pixmap_dir(), "trimbar-focused.png")})
 
 
-def get_framerate(stream):
-    """Formats the framerate of the stream for display."""
-    num = stream.get_framerate_num()
-    denom = stream.get_framerate_denom()
-    if num == 0 or denom == 0:
-        # Gst returns 0/1 if unable to determine it or in case of an image.
+def format_framerate_value(framerate):
+    """Formats the framerate or returns 0 if unable to determine it."""
+    if isinstance(framerate, DiscovererVideoInfo):
+        num = framerate.get_framerate_num()
+        denom = framerate.get_framerate_denom()
+        framerate = Gst.Fraction(num, denom)
+
+    if framerate.denom == 0:
         return "0"
-    return "{0:.5n}".format(num / denom)
+
+    value = framerate.num / framerate.denom
+    # Keep maximum 3 decimals.
+    value = value * 1000 // 1 / 1000
+    return "{0:n}".format(value)
+
+
+def format_framerate(framerate):
+    """Formats the framerate for display."""
+    # Translators: 'fps' is for 'frames per second'
+    return _("{0:s} fps").format(format_framerate_value(framerate))
+
+
+def format_audiorate(rate):
+    """Formats the audiorate (in kHz) for display."""
+    if isinstance(rate, DiscovererAudioInfo):
+        rate = rate.get_sample_rate()
+
+    # We need to use "n" to format the number according to the locale:
+    # https://www.python.org/dev/peps/pep-3101/#id20
+    # It's tricky specifying the "precision". For example:
+    # "{0:.1n}".format(44.1) == "4e+01"
+    # Seems the only way to control the number of decimals is by massaging
+    # the value.
+    if rate // 100 % 10:
+        # Show one (significant) decimal.
+        rate = rate // 100 / 10
+    else:
+        # Show no decimals.
+        rate = rate // 1000
+    return _("{0:n} kHz").format(rate)
+
+
+def format_audiochannels(channels):
+    """Formats the audio channels for display."""
+    if isinstance(channels, DiscovererAudioInfo):
+        channels = channels.get_channels()
+
+    unique_vals = {
+        1: _("Mono"),
+        2: _("Stereo"),
+        6: _("6 (5.1)"),
+        8: _("8 (7.1)")}
+    try:
+        return unique_vals[channels]
+    except KeyError:
+        return str(channels)
 
 
 # ---------------------- ARGB color helper-------------------------------------#
@@ -355,7 +403,7 @@ def beautify_stream(stream):
         width = stream.get_width()
         height = stream.get_height()
         if not stream.is_image():
-            fps = get_framerate(stream)
+            fps = format_framerate_value(stream)
             templ = _("<b>Video:</b> %d×%d <i>pixels</i> at %s <i>fps</i>")
             return templ % (par * width, height, fps)
         else:
@@ -522,22 +570,6 @@ def get_combo_value(combo):
     return combo.props.model.get_value(active_iter, 1)
 
 
-def get_value_from_model(model, key):
-    """Searches a key in a model's second column.
-
-    Returns:
-        str: The first column element on the matching row. If no row matches,
-            and the key is a `Gst.Fraction`, returns a beautified form.
-            Otherwise returns the key.
-    """
-    for row in model:
-        if row[1] == key:
-            return str(row[0])
-    if isinstance(key, Gst.Fraction):
-        return "%.3f" % decimal.Decimal(float(key.num) / key.denom)
-    return str(key)
-
-
 def alter_style_class(style_class, target_widget, css_style):
     css_provider = Gtk.CssProvider()
     css = "%s { %s }" % (style_class, css_style)
@@ -581,63 +613,34 @@ def fix_infobar(infobar):
     infobar.forall(make_sure_revealer_does_nothing)
 
 
-# ----------------------- encoding datas --------------------------------------#
-# FIXME This should into a special file
-frame_rates = model((str, object), (
-    # Translators: fps is for frames per second
-    (_("%d fps") % 12, Gst.Fraction(12, 1)),
-    (_("%d fps") % 15, Gst.Fraction(15, 1)),
-    (_("%d fps") % 20, Gst.Fraction(20, 1)),
-    (_("%.3f fps") % 23.976, Gst.Fraction(24000, 1001)),
-    (_("%d fps") % 24, Gst.Fraction(24, 1)),
-    (_("%d fps") % 25, Gst.Fraction(25, 1)),
-    (_("%.2f fps") % 29.97, Gst.Fraction(30000, 1001)),
-    (_("%d fps") % 30, Gst.Fraction(30, 1)),
-    (_("%d fps") % 50, Gst.Fraction(50, 1)),
-    (_("%.2f fps") % 59.94, Gst.Fraction(60000, 1001)),
-    (_("%d fps") % 60, Gst.Fraction(60, 1)),
-    (_("%d fps") % 120, Gst.Fraction(120, 1)),
-))
-
-audio_rates = model((str, int), (
-    (_("%d kHz") % 8, 8000),
-    (_("%d kHz") % 11, 11025),
-    (_("%d kHz") % 12, 12000),
-    (_("%d kHz") % 16, 16000),
-    (_("%d kHz") % 22, 22050),
-    (_("%d kHz") % 24, 24000),
-    (_("%.1f kHz") % 44.1, 44100),
-    (_("%d kHz") % 48, 48000),
-    (_("%d kHz") % 96, 96000)))
-
-audio_channels = model((str, int), (
-    (_("6 Channels (5.1)"), 6),
-    (_("4 Channels (4.0)"), 4),
-    (_("Stereo"), 2),
-    (_("Mono"), 1)))
-
-# FIXME: are we sure the following tables correct?
-
-pixel_aspect_ratios = model((str, object), (
-    (_("Square"), Gst.Fraction(1, 1)),
-    (_("480p"), Gst.Fraction(10, 11)),
-    (_("480i"), Gst.Fraction(8, 9)),
-    (_("480p Wide"), Gst.Fraction(40, 33)),
-    (_("480i Wide"), Gst.Fraction(32, 27)),
-    (_("576p"), Gst.Fraction(12, 11)),
-    (_("576i"), Gst.Fraction(16, 15)),
-    (_("576p Wide"), Gst.Fraction(16, 11)),
-    (_("576i Wide"), Gst.Fraction(64, 45)),
-))
-
-display_aspect_ratios = model((str, object), (
-    (_("Standard (4:3)"), Gst.Fraction(4, 3)),
-    (_("DV (15:11)"), Gst.Fraction(15, 11)),
-    (_("DV Widescreen (16:9)"), Gst.Fraction(16, 9)),
-    (_("Cinema (1.37)"), Gst.Fraction(11, 8)),
-    (_("Cinema (1.66)"), Gst.Fraction(166, 100)),
-    (_("Cinema (1.85)"), Gst.Fraction(185, 100)),
-    (_("Anamorphic (2.35)"), Gst.Fraction(235, 100)),
-    (_("Anamorphic (2.39)"), Gst.Fraction(239, 100)),
-    (_("Anamorphic (2.4)"), Gst.Fraction(24, 10)),
-))
+audio_channels = model((str, int),
+    [(format_audiochannels(ch), ch) for ch in (8, 6, 4, 2, 1)])
+
+frame_rates = model((str, object),
+    [(format_framerate(Gst.Fraction(*fps)), Gst.Fraction(*fps)) for fps in (
+        (12, 1),
+        (15, 1),
+        (20, 1),
+        (24000, 1001),
+        (24, 1),
+        (25, 1),
+        (30000, 1001),
+        (30, 1),
+        (50, 1),
+        (60000, 1001),
+        (60, 1),
+        (120, 1)
+    )])
+
+audio_rates = model((str, int),
+    [(format_audiorate(rate), rate) for rate in (
+        8000,
+        11025,
+        12000,
+        16000,
+        22050,
+        24000,
+        44100,
+        48000,
+        96000
+    )])
diff --git a/tests/test_utils.py b/tests/test_utils.py
index c6b9c2f2..97a029cc 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -22,6 +22,7 @@ from unittest.mock import Mock
 
 from gi.repository import GLib
 from gi.repository import Gst
+from gi.repository import GstPbutils
 
 from pitivi.check import CairoDependency
 from pitivi.check import ClassicDependency
@@ -29,7 +30,9 @@ from pitivi.check import GstDependency
 from pitivi.check import GtkDependency
 from pitivi.utils.misc import fixate_caps_with_default_values
 from pitivi.utils.ui import beautify_length
-from pitivi.utils.ui import get_framerate
+from pitivi.utils.ui import format_audiochannels
+from pitivi.utils.ui import format_audiorate
+from pitivi.utils.ui import format_framerate_value
 
 second = Gst.SECOND
 minute = second * 60
@@ -62,13 +65,17 @@ class TestBeautifyLength(TestCase):
         self.assertEqual(beautify_length(Gst.CLOCK_TIME_NONE), "")
 
 
-class TestGetFramerate(TestCase):
+class TestFormatFramerateValue(TestCase):
 
     def __check(self, num, denom, expected):
-        stream = Mock()
+        stream = Mock(spec=GstPbutils.DiscovererVideoInfo)
+        fraction = Mock(num=num, denom=denom)
+
         stream.get_framerate_num = Mock(return_value=num)
         stream.get_framerate_denom = Mock(return_value=denom)
-        self.assertEqual(get_framerate(stream), expected)
+
+        self.assertEqual(format_framerate_value(stream), expected)
+        self.assertEqual(format_framerate_value(fraction), expected)
 
     def test_invalid_fps(self):
         self.__check(0, 1, "0")
@@ -89,6 +96,39 @@ class TestGetFramerate(TestCase):
         self.__check(120, 1, "120")
 
 
+class TestFormatAudiorate(TestCase):
+
+    def __check(self, rate, expected):
+        stream = Mock(spec=GstPbutils.DiscovererAudioInfo)
+        stream.get_sample_rate = Mock(return_value=rate)
+
+        self.assertEqual(format_audiorate(stream), expected)
+        self.assertEqual(format_audiorate(rate), expected)
+
+    def test_audiorates(self):
+        self.__check(8000, "8 kHz")
+        self.__check(11025, "11 kHz")
+        self.__check(22050, "22 kHz")
+        self.__check(44100, "44.1 kHz")
+        self.__check(96000, "96 kHz")
+        self.__check(960000, "960 kHz")
+
+
+class TestFormatAudiochannels(TestCase):
+
+    def __check(self, channels, expected):
+        stream = Mock(spec=GstPbutils.DiscovererAudioInfo)
+        stream.get_channels = Mock(return_value=channels)
+
+        self.assertEqual(format_audiochannels(stream), expected)
+        self.assertEqual(format_audiochannels(channels), expected)
+
+    def test_audiochannels(self):
+        self.__check(1, "Mono")
+        self.__check(2, "Stereo")
+        self.__check(6, "6 (5.1)")
+
+
 class TestDependencyChecks(TestCase):
 
     def testDependencies(self):