[pitivi] timeline: Shortcut K to toggle a keyframe

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

commit 7b082cacf063648070cfa2a0c4e47ec31ef7c189
Author: Jakub Brindza <jakub brindza gmail com>
Date:   Thu Mar 24 23:39:41 2016 +0000

    timeline: Shortcut K to toggle a keyframe
    
    Differential Revision: https://phabricator.freedesktop.org/D898

 pitivi/timeline/elements.py     |   61 ++++++++++++++++---------
 pitivi/timeline/timeline.py     |   36 ++++++---------
 pitivi/utils/timeline.py        |    3 +
 pre-commit.hook                 |    1 +
 tests/Makefile.am               |    1 +
 tests/test_timeline_elements.py |   94 +++++++++++++++++++++++++++++++++++++++
 tests/test_timeline_timeline.py |    6 +--
 7 files changed, 153 insertions(+), 49 deletions(-)
---
diff --git a/pitivi/timeline/elements.py b/pitivi/timeline/elements.py
index 4e39767..a5f3935 100644
--- a/pitivi/timeline/elements.py
+++ b/pitivi/timeline/elements.py
@@ -213,6 +213,21 @@ class KeyframeCurve(FigureCanvas, Loggable):
             with self.__timeline.app.action_log.started("Keyframe added"):
                 self.__source.set(event.xdata, value)
 
+    def toggle_keyframe(self, offset):
+        """
+        Set or unset the keyframe at the given offset.
+        """
+        items = self.__source.get_all()
+        if offset in (items[0].timestamp, items[-1].timestamp):
+            return
+
+        if offset in [item.timestamp for item in items]:
+            self.__source.unset(offset)
+        else:
+            res, value = self.__source.control_source_get_value(offset)
+            assert res
+            self.__source.set(offset, value)
+
     # Callbacks
     def __controlSourceChangedCb(self, unused_control_source, unused_timed_value):
         self.__updatePlots()
@@ -423,7 +438,7 @@ class TimelineElement(Gtk.Layout, timelineUtils.Zoomable, Loggable):
         if self.__background:
             self.add(self.__background)
 
-        self.__keyframeCurve = None
+        self.keyframe_curve = None
         self.show_all()
 
         # We set up the default mixing property right here, if a binding was
@@ -446,8 +461,8 @@ class TimelineElement(Gtk.Layout, timelineUtils.Zoomable, Loggable):
         if self.__background:
             self.__background.set_size_request(width, height)
 
-        if self.__keyframeCurve:
-            self.__keyframeCurve.set_size_request(width, height)
+        if self.keyframe_curve:
+            self.keyframe_curve.set_size_request(width, height)
 
         self.__width = width
         self.__height = height
@@ -471,18 +486,18 @@ class TimelineElement(Gtk.Layout, timelineUtils.Zoomable, Loggable):
         self.emit("curve-leave")
 
     def __removeKeyframes(self):
-        if not self.__keyframeCurve:
+        if not self.keyframe_curve:
             # Nothing to remove.
             return
 
-        self.__keyframeCurve.disconnect_by_func(
+        self.keyframe_curve.disconnect_by_func(
             self.__keyframePlotChangedCb)
-        self.__keyframeCurve.disconnect_by_func(self.__curveEnterCb)
-        self.__keyframeCurve.disconnect_by_func(self.__curveLeaveCb)
-        self.remove(self.__keyframeCurve)
+        self.keyframe_curve.disconnect_by_func(self.__curveEnterCb)
+        self.keyframe_curve.disconnect_by_func(self.__curveLeaveCb)
+        self.remove(self.keyframe_curve)
 
-        self.__keyframeCurve.release()
-        self.__keyframeCurve = None
+        self.keyframe_curve.release()
+        self.keyframe_curve = None
 
     # Private methods
     def __createKeyframeCurve(self, binding):
@@ -500,14 +515,14 @@ class TimelineElement(Gtk.Layout, timelineUtils.Zoomable, Loggable):
                 val)
 
         self.__removeKeyframes()
-        self.__keyframeCurve = KeyframeCurve(self.timeline, binding)
-        self.__keyframeCurve.connect("plot-changed",
+        self.keyframe_curve = KeyframeCurve(self.timeline, binding)
+        self.keyframe_curve.connect("plot-changed",
                                      self.__keyframePlotChangedCb)
-        self.__keyframeCurve.connect("enter", self.__curveEnterCb)
-        self.__keyframeCurve.connect("leave", self.__curveLeaveCb)
-        self.add(self.__keyframeCurve)
-        self.__keyframeCurve.set_size_request(self.__width, self.__height)
-        self.__keyframeCurve.props.visible = bool(self._ges_elem.selected)
+        self.keyframe_curve.connect("enter", self.__curveEnterCb)
+        self.keyframe_curve.connect("leave", self.__curveLeaveCb)
+        self.add(self.keyframe_curve)
+        self.keyframe_curve.set_size_request(self.__width, self.__height)
+        self.keyframe_curve.props.visible = bool(self._ges_elem.selected)
         self.queue_draw()
 
     def __createControlBinding(self, element):
@@ -535,11 +550,11 @@ class TimelineElement(Gtk.Layout, timelineUtils.Zoomable, Loggable):
         if self.timeline.app.project_manager.current_project.pipeline.getState() == Gst.State.PLAYING:
             return False
 
-        if not self.__keyframeCurve:
+        if not self.keyframe_curve:
             return False
 
         # We do not show keyframes while a clip is being moved on the timeline
-        if self.timeline.draggingElement and not self.__keyframeCurve.handling_motion:
+        if self.timeline.draggingElement and not self.keyframe_curve.handling_motion:
             return False
 
         # We do not show keyframes when there are several clips selected
@@ -555,19 +570,19 @@ class TimelineElement(Gtk.Layout, timelineUtils.Zoomable, Loggable):
             self.propagate_draw(self.__previewer, cr)
 
         if self.__showKeyframes():
-            self.propagate_draw(self.__keyframeCurve, cr)
+            self.propagate_draw(self.keyframe_curve, cr)
 
     def do_show_all(self):
         for child in self.get_children():
-            if bool(self._ges_elem.selected) or child != self.__keyframeCurve:
+            if bool(self._ges_elem.selected) or child != self.keyframe_curve:
                 child.show_all()
 
         self.show()
 
     # Callbacks
     def __selectedChangedCb(self, unused_ges_elem, selected):
-        if self.__keyframeCurve:
-            self.__keyframeCurve.props.visible = selected
+        if self.keyframe_curve:
+            self.keyframe_curve.props.visible = selected
 
         if self.__previewer:
             self.__previewer.setSelected(selected)
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index f3b0311..d0d51c7 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -1494,7 +1494,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         self.split_action.set_enabled(True)
 
         self.keyframe_action = Gio.SimpleAction.new("keyframe_selected_clips", None)
-        self.keyframe_action.connect("activate", self._keyframeCb)
+        self.keyframe_action.connect("activate", self._keyframe_cb)
         group.add_action(self.keyframe_action)
         self.app.add_accelerator("K", "timeline.keyframe_selected_clips", None)
 
@@ -1698,30 +1698,24 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         if not splitted and splitting_selection:
             self._splitElements()
 
-    def _keyframeCb(self, unused_action, unused_parameter):
+    def _keyframe_cb(self, unused_action, unused_parameter):
         """
         Add or remove a keyframe at the current position of the selected clip.
         """
-        selected = self.timeline.selection.getSelectedTrackElements()
+        ges_clip = self.timeline.selection.getSingleClip(GES.Clip)
+        if ges_clip is None:
+            return
 
-        for obj in selected:
-            keyframe_exists = False
-            position = self._project.pipeline.getPosition()
-            position_in_obj = (position - obj.props.start) + obj.props.in_point
-            interpolators = obj.getInterpolators()
-            for value in interpolators:
-                interpolator = obj.getInterpolator(value)
-                keyframes = interpolator.getInteriorKeyframes()
-                for kf in keyframes:
-                    if kf.getTime() == position_in_obj:
-                        keyframe_exists = True
-                        self.app.action_log.begin("remove volume point")
-                        interpolator.removeKeyframe(kf)
-                        self.app.action_log.commit()
-                if keyframe_exists is False:
-                    self.app.action_log.begin("add volume point")
-                    interpolator.newKeyframe(position_in_obj)
-                    self.app.action_log.commit()
+        ges_track_elements = ges_clip.find_track_elements(None, GES.TrackType.VIDEO, GES.Source)
+        ges_track_elements += ges_clip.find_track_elements(None, GES.TrackType.AUDIO, GES.Source)
+
+        offset = self._project.pipeline.getPosition() - ges_clip.props.start
+        if offset <= 0 or offset >= ges_clip.props.duration:
+            return
+
+        for ges_track_element in ges_track_elements:
+            keyframe_curve = ges_track_element.ui.keyframe_curve
+            keyframe_curve.toggle_keyframe(offset)
 
     def _playPauseCb(self, unused_action, unused_parameter):
         self._project.pipeline.togglePlayback()
diff --git a/pitivi/utils/timeline.py b/pitivi/utils/timeline.py
index 316166d..9f02c76 100644
--- a/pitivi/utils/timeline.py
+++ b/pitivi/utils/timeline.py
@@ -152,6 +152,9 @@ class Selection(GObject.Object, Loggable):
                     element.selected.selected = True
         self.emit("selection-changed")
 
+    def select(self, objs):
+        self.setSelection(objs, SELECT)
+
     def unselect(self, objs):
         self.setSelection(objs, UNSELECT)
 
diff --git a/pre-commit.hook b/pre-commit.hook
index 83e4243..bc49404 100755
--- a/pre-commit.hook
+++ b/pre-commit.hook
@@ -23,6 +23,7 @@ tests/test_misc.py \
 tests/test_widgets.py \
 tests/test_common.py \
 tests/common.py \
+tests/test_timeline_elements.py \
 tests/test_timeline_timeline.py \
 tests/test_pipeline.py \
 tests/test_application.py \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4fd1946..8f576e8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -17,6 +17,7 @@ tests =       \
        test_previewers.py \
        test_project.py \
        test_system.py \
+       test_timeline_elements.py \
        test_timeline_layer.py \
        test_timeline_timeline.py \
        test_undo.py \
diff --git a/tests/test_timeline_elements.py b/tests/test_timeline_elements.py
new file mode 100644
index 0000000..079a87a
--- /dev/null
+++ b/tests/test_timeline_elements.py
@@ -0,0 +1,94 @@
+# -*- coding: utf-8 -*-
+# Pitivi video editor
+#
+#       tests/test_timeline_elements.py
+#
+# Copyright (c) 2016, Jakub Brindza <jakub brindza gmail com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the
+# Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+# Boston, MA 02110-1301, USA.
+from unittest import mock
+
+from gi.repository import GES
+
+from tests import common
+from tests import test_timeline_timeline
+
+
+class TestKeyframeCurve(test_timeline_timeline.BaseTestTimeline):
+
+    def test_keyframe_toggle(self):
+        timeline = self.createTimeline()
+        pipeline = timeline._project.pipeline
+        self.addClipsSimple(timeline, 2)
+        ges_layer = timeline.ges_timeline.get_layers()[0]
+        # For variety, add TitleClip to the list of clips.
+        ges_clip = common.create_test_clip(GES.TitleClip)
+        ges_clip.props.duration = 4.5
+        ges_layer.add_clip(ges_clip)
+
+        for ges_clip in ges_layer.get_clips():
+            start = ges_clip.props.start
+            offsets = list(range(1, int(ges_clip.props.duration)))
+            timeline.selection.select([ges_clip])
+
+            ges_video_source = None
+            for child in ges_clip.get_children(recursive=False):
+                if isinstance(child, GES.VideoSource):
+                    ges_video_source = child
+            binding = ges_video_source.get_control_binding("alpha")
+            control_source = binding.props.control_source
+
+            # Test adding of keyframes.
+            for offset in offsets:
+                position = start + offset
+                pipeline.getPosition = mock.Mock(return_value=position)
+                timeline.parent._keyframe_cb(None, None)
+                values = [item.timestamp for item in control_source.get_all()]
+                self.assertIn(offset, values)
+
+            # Test removing of keyframes.
+            for offset in offsets:
+                position = start + offset
+                pipeline.getPosition = mock.Mock(return_value=position)
+                timeline.parent._keyframe_cb(None, None)
+                values = [item.timestamp for item in control_source.get_all()]
+                self.assertNotIn(offset, values)
+
+            # Make sure the keyframes at the start and end of the clip
+            # cannot be toggled.
+            for offset in [0, ges_clip.props.duration]:
+                position = start + offset
+                pipeline.getPosition = mock.Mock(return_value=position)
+                values = [item.timestamp for item in control_source.get_all()]
+                self.assertIn(offset, values)
+                timeline.parent._keyframe_cb(None, None)
+                values = [item.timestamp for item in control_source.get_all()]
+                self.assertIn(offset, values)
+
+            # Test out of clip range.
+            for offset in [-1, ges_clip.props.duration + 1]:
+                position = min(max(0, start + offset),
+                               timeline.ges_timeline.props.duration)
+                pipeline.getPosition = mock.Mock(return_value=position)
+                timeline.parent._keyframe_cb(None, None)
+                values = [item.timestamp for item in control_source.get_all()]
+                self.assertEqual(values, [0, ges_clip.props.duration])
+
+    def test_no_clip_selected(self):
+        # When no clip is selected, pressing key should yield no action.
+        # Make sure this does not raise any exception
+        timeline = self.createTimeline()
+        timeline.parent._keyframe_cb(None, None)
diff --git a/tests/test_timeline_timeline.py b/tests/test_timeline_timeline.py
index c1519a7..c6f1661 100644
--- a/tests/test_timeline_timeline.py
+++ b/tests/test_timeline_timeline.py
@@ -53,15 +53,11 @@ class BaseTestTimeline(common.TestCase):
 
     def addClipsSimple(self, timeline, num_clips):
         layer = timeline.ges_timeline.append_layer()
-
         asset = GES.UriClipAsset.request_sync(
             common.get_sample_uri("tears_of_steel.webm"))
-
         clips = [layer.add_asset(asset, i * 10, 0, 10, GES.TrackType.UNKNOWN)
-                for i in range(num_clips)]
-
+                 for i in range(num_clips)]
         self.assertEqual(len(clips), num_clips)
-
         return clips