[pitivi/1.0] previewers: Cache the positions and the images size

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

commit 11176ad7090ac075d83169248a6e6b55aad625e9
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Thu Dec 7 01:47:24 2017 +0100

    previewers: Cache the positions and the images size
    
    Improvements per 1000 runs, in millis, on i7, using the same asset:
    - in           8.956 -> 1.327
    - image_size  63.847 -> 2.127
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1906

 pitivi/medialibrary.py        |    2 +-
 pitivi/timeline/previewers.py |   91 ++++++++++++++++++++++-------------------
 pre-commit.hook               |    1 -
 tests/common.py               |   10 ++++-
 tests/test_previewers.py      |   63 ++++++++++++++++++++++------
 5 files changed, 108 insertions(+), 59 deletions(-)
---
diff --git a/pitivi/medialibrary.py b/pitivi/medialibrary.py
index cc6ed9b..9af5968 100644
--- a/pitivi/medialibrary.py
+++ b/pitivi/medialibrary.py
@@ -232,7 +232,7 @@ class AssetThumbnail(Loggable):
                 else:
                     # Build or reuse a ThumbnailCache.
                     thumb_cache = ThumbnailCache.get(self.__asset)
-                    small_thumb = thumb_cache.getPreviewThumbnail()
+                    small_thumb = thumb_cache.get_preview_thumbnail()
                     if not small_thumb:
                         small_thumb, large_thumb = self.__get_icons("video-x-generic")
                     else:
diff --git a/pitivi/timeline/previewers.py b/pitivi/timeline/previewers.py
index cdf7bf6..28abf8c 100644
--- a/pitivi/timeline/previewers.py
+++ b/pitivi/timeline/previewers.py
@@ -446,7 +446,7 @@ class VideoPreviewer(Previewer, Zoomable, Loggable):
         self.thumbs = {}
         self.thumb_cache = ThumbnailCache.get(self.uri)
         self._ensure_proxy_thumbnails_cache()
-        self.thumb_width, unused_height = self.thumb_cache.getImagesSize()
+        self.thumb_width, unused_height = self.thumb_cache.image_size
 
         self.cpu_usage_tracker = CPUUsageTracker()
         self.interval = 500  # Every 0.5 second, reevaluate the situation
@@ -753,12 +753,12 @@ class Thumbnail(Gtk.Image):
 
 
 class ThumbnailCache(Loggable):
-    """Caches an asset's thumbnails by key, using LRU policy.
+    """Cache for the thumbnails of an asset.
 
-    Uses a two stage caching mechanism. A limited number of elements are
-    held in memory, the rest is being cached on disk in an SQLite db.
+    Uses a separate sqlite3 database for each asset.
     """
 
+    # The cache of caches.
     caches_by_uri = {}
 
     def __init__(self, uri):
@@ -767,10 +767,18 @@ class ThumbnailCache(Loggable):
         thumbs_cache_dir = get_dir(os.path.join(xdg_cache_home(), "thumbs"))
         self._dbfile = os.path.join(thumbs_cache_dir, self._filehash)
         self._db = sqlite3.connect(self._dbfile)
-        self._cur = self._db.cursor()  # Use this for normal db operations
-        self._cur.execute("CREATE TABLE IF NOT EXISTS Thumbs\
-                          (Time INTEGER NOT NULL PRIMARY KEY,\
-                          Jpeg BLOB NOT NULL)")
+        self._cur = self._db.cursor()
+        self._cur.execute("CREATE TABLE IF NOT EXISTS Thumbs "
+                          "(Time INTEGER NOT NULL PRIMARY KEY, "
+                          " Jpeg BLOB NOT NULL)")
+        # The cached (width, height) of the images.
+        self._image_size = (None, None)
+        # The cached positions available in the database.
+        self.positions = self.__existing_positions()
+
+    def __existing_positions(self):
+        self._cur.execute("SELECT Time FROM Thumbs")
+        return set([row[0] for row in self._cur.fetchall()])
 
     @classmethod
     def get(cls, obj):
@@ -810,70 +818,69 @@ class ThumbnailCache(Loggable):
             pass
         os.symlink(self._dbfile, dbfile)
 
-    def getImagesSize(self):
+    @property
+    def image_size(self):
         """Gets the image size.
 
         Returns:
             List[int]: The width and height of the images in the cache.
         """
-        self._cur.execute("SELECT * FROM Thumbs LIMIT 1")
-        row = self._cur.fetchone()
-        if not row:
-            return None, None
-
-        pixbuf = self.__getPixbufFromRow(row)
-        return pixbuf.get_width(), pixbuf.get_height()
-
-    def getPreviewThumbnail(self):
+        if self._image_size[0] is None:
+            self._cur.execute("SELECT * FROM Thumbs LIMIT 1")
+            row = self._cur.fetchone()
+            if row:
+                pixbuf = self.__pixbuf_from_row(row)
+                self._image_size = (pixbuf.get_width(), pixbuf.get_height())
+        return self._image_size
+
+    def get_preview_thumbnail(self):
         """Gets a thumbnail contained 'at the middle' of the cache."""
-        self._cur.execute("SELECT Time FROM Thumbs")
-        timestamps = self._cur.fetchall()
-        if not timestamps:
+        if not self.positions:
             return None
 
-        return self[timestamps[int(len(timestamps) / 2)][0]]
+        middle = int(len(self.positions) / 2)
+        position = sorted(list(self.positions))[middle]
+        return self[position]
 
-    # pylint: disable=no-self-use
-    def __getPixbufFromRow(self, row):
+    @staticmethod
+    def __pixbuf_from_row(row):
+        """Returns the GdkPixbuf.Pixbuf from the specified row."""
         jpeg = row[1]
         loader = GdkPixbuf.PixbufLoader.new()
-        # TODO: what do to if any of the following calls fails?
         loader.write(jpeg)
         loader.close()
         pixbuf = loader.get_pixbuf()
         return pixbuf
 
-    def __contains__(self, key):
-        # check if item is present in on disk cache
-        self._cur.execute("SELECT Time FROM Thumbs WHERE Time = ?", (key,))
-        if self._cur.fetchone():
-            return True
-        return False
+    def __contains__(self, position):
+        """Returns whether a row for the specified position exists in the DB."""
+        return position in self.positions
 
-    def __getitem__(self, key):
-        self._cur.execute("SELECT * FROM Thumbs WHERE Time = ?", (key,))
+    def __getitem__(self, position):
+        """Gets the GdkPixbuf.Pixbuf for the specified position."""
+        self._cur.execute("SELECT * FROM Thumbs WHERE Time = ?", (position,))
         row = self._cur.fetchone()
         if not row:
-            raise KeyError(key)
-        return self.__getPixbufFromRow(row)
+            raise KeyError(position)
+        return self.__pixbuf_from_row(row)
 
-    def __setitem__(self, key, value):
-        success, jpeg = value.save_to_bufferv(
+    def __setitem__(self, position, pixbuf):
+        """Sets a GdkPixbuf.Pixbuf for the specified position."""
+        success, jpeg = pixbuf.save_to_bufferv(
             "jpeg", ["quality", None], ["90"])
         if not success:
             self.warning("JPEG compression failed")
             return
         blob = sqlite3.Binary(jpeg)
         # Replace if a row with the same time already exists.
-        self._cur.execute("DELETE FROM Thumbs WHERE  time=?", (key,))
-        self._cur.execute("INSERT INTO Thumbs VALUES (?,?)", (key, blob,))
+        self._cur.execute("DELETE FROM Thumbs WHERE  time=?", (position,))
+        self._cur.execute("INSERT INTO Thumbs VALUES (?,?)", (position, blob,))
+        self.positions.add(position)
 
     def commit(self):
         """Saves the cache on disk (in the database)."""
         self._db.commit()
-        self.log("Saved thumbnail cache file: %s" % self._filehash)
-
-        return False
+        self.log("Saved thumbnail cache file: %s", self._filehash)
 
 
 def get_wavefile_location_for_uri(uri):
diff --git a/pre-commit.hook b/pre-commit.hook
index 11076e9..49d2d64 100755
--- a/pre-commit.hook
+++ b/pre-commit.hook
@@ -55,7 +55,6 @@ tests/test_log.py
 tests/test_media_library.py
 tests/test_prefs.py
 tests/test_preset.py
-tests/test_previewers.py
 tests/test_project.py
 tests/test_system.py
 tests/test_timeline_layer.py
diff --git a/tests/common.py b/tests/common.py
index cccc31d..999c8b6 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -110,14 +110,14 @@ def create_main_loop():
     mainloop = GLib.MainLoop()
     timed_out = False
 
-    def quit_cb(unused):
+    def timeout_cb(unused):
         nonlocal timed_out
         timed_out = True
         mainloop.quit()
 
     def run(timeout_seconds=5):
         source = GLib.timeout_source_new_seconds(timeout_seconds)
-        source.set_callback(quit_cb)
+        source.set_callback(timeout_cb)
         source.attach()
         GLib.MainLoop.run(mainloop)
         source.destroy()
@@ -252,6 +252,12 @@ def get_sample_uri(sample):
     return Gst.filename_to_uri(os.path.join(tests_dir, "samples", sample))
 
 
+def get_clip_children(ges_clip, *track_types, recursive=False):
+    for ges_timeline_element in ges_clip.get_children(recursive):
+        if not track_types or ges_timeline_element.get_track_type() in track_types:
+            yield ges_timeline_element
+
+
 def clean_proxy_samples():
     _dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), "samples")
     proxy_manager = ProxyManager(mock.MagicMock())
diff --git a/tests/test_previewers.py b/tests/test_previewers.py
index f9924c0..9e855f1 100644
--- a/tests/test_previewers.py
+++ b/tests/test_previewers.py
@@ -16,12 +16,14 @@
 # License along with this program; if not, write to the
 # Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
 # Boston, MA 02110-1301, USA.
+"""Tests for the timeline.previewers module."""
+# pylint: disable=protected-access
 import os
 import tempfile
 from unittest import mock
-from unittest import TestCase
 
 import numpy
+from gi.repository import GdkPixbuf
 from gi.repository import GES
 from gi.repository import Gst
 
@@ -66,28 +68,23 @@ SIMPSON_WAVFORM_VALUES = [
     3.6256085412966614, 0.0]
 
 
-class TestPreviewers(BaseTestMediaLibrary):
+class TestAudioPreviewer(BaseTestMediaLibrary):
+    """Tests for the `AudioPreviewer` class."""
 
-    def testCreateThumbnailBin(self):
+    def test_create_thumbnail_bin(self):
+        """Checks our `waveformbin` element is usable."""
         pipeline = Gst.parse_launch("uridecodebin name=decode uri=file:///some/thing"
                                     " waveformbin name=wavebin ! fakesink qos=false name=faked")
         self.assertTrue(pipeline)
         wavebin = pipeline.get_by_name("wavebin")
         self.assertTrue(wavebin)
 
-    def testWaveFormAndThumbnailCreated(self):
+    def test_waveform_creation(self):
+        """Checks the waveform generation."""
         sample_name = "1sec_simpsons_trailer.mp4"
         self.runCheckImport([sample_name])
 
         sample_uri = common.get_sample_uri(sample_name)
-        asset = GES.UriClipAsset.request_sync(sample_uri)
-
-        thumb_cache = ThumbnailCache.get(asset)
-        width, height = thumb_cache.getImagesSize()
-        self.assertEqual(height, THUMB_HEIGHT)
-        self.assertTrue(thumb_cache[0] is not None)
-        self.assertTrue(thumb_cache[Gst.SECOND / 2] is not None)
-
         wavefile = get_wavefile_location_for_uri(sample_uri)
         self.assertTrue(os.path.exists(wavefile), wavefile)
 
@@ -97,9 +94,11 @@ class TestPreviewers(BaseTestMediaLibrary):
         self.assertEqual(samples, SIMPSON_WAVFORM_VALUES)
 
 
-class TestThumbnailCache(TestCase):
+class TestThumbnailCache(BaseTestMediaLibrary):
+    """Tests for the ThumbnailCache class."""
 
     def test_get(self):
+        """Checks the `get` method returns the same thing for asset and URI."""
         with self.assertRaises(ValueError):
             ThumbnailCache.get(1)
         with mock.patch("pitivi.timeline.previewers.xdg_cache_home") as xdg_config_home,\
@@ -111,3 +110,41 @@ class TestThumbnailCache(TestCase):
 
             asset = GES.UriClipAsset.request_sync(sample_uri)
             self.assertEqual(ThumbnailCache.get(asset), cache)
+
+    def test_image_size(self):
+        """Checks the `image_size` property."""
+        with tempfile.TemporaryDirectory() as tmpdirname:
+            with mock.patch("pitivi.timeline.previewers.xdg_cache_home") as xdg_cache_home:
+                xdg_cache_home.return_value = tmpdirname
+                sample_uri = common.get_sample_uri("1sec_simpsons_trailer.mp4")
+                thumb_cache = ThumbnailCache(sample_uri)
+                self.assertEqual(thumb_cache.image_size, (None, None))
+
+                pixbuf = GdkPixbuf.Pixbuf.new(GdkPixbuf.Colorspace.RGB,
+                                              False, 8, 20, 10)
+                thumb_cache[0] = pixbuf
+                self.assertEqual(thumb_cache.image_size, (20, 10))
+
+    def test_containment(self):
+        """Checks the __contains/getitem/setitem__ methods."""
+        with tempfile.TemporaryDirectory() as tmpdirname:
+            with mock.patch("pitivi.timeline.previewers.xdg_cache_home") as xdg_cache_home:
+                xdg_cache_home.return_value = tmpdirname
+                sample_uri = common.get_sample_uri("1sec_simpsons_trailer.mp4")
+                thumb_cache = ThumbnailCache(sample_uri)
+                self.assertFalse(Gst.SECOND in thumb_cache)
+                with self.assertRaises(KeyError):
+                    # pylint: disable=pointless-statement
+                    thumb_cache[Gst.SECOND]
+
+                pixbuf = GdkPixbuf.Pixbuf.new(GdkPixbuf.Colorspace.RGB,
+                                              False, 8,
+                                              int(THUMB_HEIGHT * 1280 / 544), THUMB_HEIGHT)
+                thumb_cache[Gst.SECOND] = pixbuf
+                self.assertTrue(Gst.SECOND in thumb_cache)
+                self.assertIsNotNone(thumb_cache[Gst.SECOND])
+                thumb_cache.commit()
+
+                thumb_cache = ThumbnailCache(sample_uri)
+                self.assertTrue(Gst.SECOND in thumb_cache)
+                self.assertIsNotNone(thumb_cache[Gst.SECOND])