[pitivi] timeline: Consider the clip inpoint when toggling keyframes

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

commit 2ad7d95c0f030f565c7fa4d884d0207b62077459
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Tue Dec 13 21:33:59 2016 +0100

    timeline: Consider the clip inpoint when toggling keyframes
    
    Fixes https://phabricator.freedesktop.org/T7627
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1554

 pitivi/timeline/elements.py     |    7 +-
 pitivi/timeline/timeline.py     |    1 +
 tests/test_timeline_elements.py |  126 ++++++++++++++++++++++-----------------
 tests/test_timeline_timeline.py |   14 +++-
 4 files changed, 85 insertions(+), 63 deletions(-)
---
diff --git a/pitivi/timeline/elements.py b/pitivi/timeline/elements.py
index 90272f7..37ceae2 100644
--- a/pitivi/timeline/elements.py
+++ b/pitivi/timeline/elements.py
@@ -509,10 +509,9 @@ class TimelineElement(Gtk.Layout, Zoomable, Loggable):
             val = float(self.__controlledProperty.default_value) / \
                 (self.__controlledProperty.maximum -
                  self.__controlledProperty.minimum)
-            source.set(self._ges_elem.props.in_point, val)
-            source.set(
-                self._ges_elem.props.duration + self._ges_elem.props.in_point,
-                val)
+            inpoint = self._ges_elem.props.in_point
+            assert source.set(inpoint, val)
+            assert source.set(inpoint + self._ges_elem.props.duration, val)
 
         self.__removeKeyframes()
         self.keyframe_curve = KeyframeCurve(self.timeline, binding)
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index d19d865..6ad33fc 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -1724,6 +1724,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         offset = self._project.pipeline.getPosition() - ges_clip.props.start
         if offset <= 0 or offset >= ges_clip.props.duration:
             return
+        offset += ges_clip.props.in_point
 
         for ges_track_element in ges_track_elements:
             keyframe_curve = ges_track_element.ui.keyframe_curve
diff --git a/tests/test_timeline_elements.py b/tests/test_timeline_elements.py
index 4478c7b..371cc68 100644
--- a/tests/test_timeline_elements.py
+++ b/tests/test_timeline_elements.py
@@ -17,7 +17,7 @@
 # Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
 # Boston, MA 02110-1301, USA.
 """Tests for the timeline.elements module."""
-# pylint: disable=protected-access,no-self-use
+# pylint: disable=protected-access,no-self-use,too-many-locals
 from unittest import mock
 from unittest import TestCase
 
@@ -37,61 +37,77 @@ class TestKeyframeCurve(BaseTestTimeline):
         """Checks keyframes toggling at the playhead position."""
         timeline_container = create_timeline_container()
         timeline = timeline_container.timeline
-        pipeline = timeline._project.pipeline
-        self.addClipsSimple(timeline, 2)
-        ges_layer = timeline.ges_timeline.get_layers()[0]
+        ges_layer = timeline.ges_timeline.append_layer()
+        ges_clip1 = self.add_clip(ges_layer, 0)
+        ges_clip2 = self.add_clip(ges_layer, 10)
+        ges_clip3 = self.add_clip(ges_layer, 20, inpoint=100)
         # For variety, add TitleClip to the list of clips.
-        ges_clip = 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_container._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_container._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_container._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_container._keyframe_cb(None, None)
-                values = [item.timestamp for item in control_source.get_all()]
-                self.assertEqual(values, [0, ges_clip.props.duration])
+        ges_clip4 = create_test_clip(GES.TitleClip)
+        ges_clip4.props.start = 30
+        ges_clip4.props.duration = 4.5
+        ges_layer.add_clip(ges_clip4)
+
+        self.check_keyframe_toggle(ges_clip1, timeline_container)
+        self.check_keyframe_toggle(ges_clip2, timeline_container)
+        self.check_keyframe_toggle(ges_clip3, timeline_container)
+        self.check_keyframe_toggle(ges_clip4, timeline_container)
+
+    def check_keyframe_toggle(self, ges_clip, timeline_container):
+        """Checks keyframes toggling on the specified clip."""
+        timeline = timeline_container.timeline
+        pipeline = timeline._project.pipeline
+
+        start = ges_clip.props.start
+        inpoint = ges_clip.props.in_point
+        duration = ges_clip.props.duration
+        offsets = (1, int(duration / 2), int(duration) - 1)
+        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
+
+        values = [item.timestamp for item in control_source.get_all()]
+        self.assertEqual(values, [inpoint, inpoint + duration])
+
+        # Add keyframes.
+        for offset in offsets:
+            position = start + offset
+            pipeline.getPosition = mock.Mock(return_value=position)
+            timeline_container._keyframe_cb(None, None)
+            values = [item.timestamp for item in control_source.get_all()]
+            self.assertIn(inpoint + offset, values)
+
+        # Remove keyframes.
+        for offset in offsets:
+            position = start + offset
+            pipeline.getPosition = mock.Mock(return_value=position)
+            timeline_container._keyframe_cb(None, None)
+            values = [item.timestamp for item in control_source.get_all()]
+            self.assertNotIn(inpoint + offset, values, offset)
+
+        # Make sure the keyframes at the start and end of the clip
+        # cannot be toggled.
+        for offset in [0, duration]:
+            position = start + offset
+            pipeline.getPosition = mock.Mock(return_value=position)
+            values = [item.timestamp for item in control_source.get_all()]
+            self.assertIn(inpoint + offset, values)
+            timeline_container._keyframe_cb(None, None)
+            values = [item.timestamp for item in control_source.get_all()]
+            self.assertIn(inpoint + offset, values)
+
+        # Test out of clip range.
+        for offset in [-1, duration + 1]:
+            position = min(max(0, start + offset),
+                           timeline.ges_timeline.props.duration)
+            pipeline.getPosition = mock.Mock(return_value=position)
+            timeline_container._keyframe_cb(None, None)
+            values = [item.timestamp for item in control_source.get_all()]
+            self.assertEqual(values, [inpoint, inpoint + duration])
 
     def test_no_clip_selected(self):
         """Checks nothing happens when no clip is selected."""
diff --git a/tests/test_timeline_timeline.py b/tests/test_timeline_timeline.py
index ee51193..37d0f7b 100644
--- a/tests/test_timeline_timeline.py
+++ b/tests/test_timeline_timeline.py
@@ -32,18 +32,24 @@ THICK = LAYER_HEIGHT
 
 
 class BaseTestTimeline(common.TestCase):
+    """Test case with tools for setting up a timeline."""
 
-    def addClipsSimple(self, timeline, num_clips):
-        layer = timeline.ges_timeline.append_layer()
+    def add_clip(self, layer, start, inpoint=0, duration=10):
+        """Creates a clip on the specified 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)]
+        return layer.add_asset(asset, start, inpoint, duration, GES.TrackType.UNKNOWN)
+
+    def addClipsSimple(self, timeline, num_clips):
+        """Creates a number of clips on a new layer."""
+        layer = timeline.ges_timeline.append_layer()
+        clips = [self.add_clip(layer, i * 10) for i in range(num_clips)]
         self.assertEqual(len(clips), num_clips)
         return clips
 
 
 class TestLayers(BaseTestTimeline):
+    """Tests for the layers."""
 
     def testDraggingLayer(self):
         self.checkGetLayerAt([THIN, THIN, THIN], 1, True,