[pitivi] Fixe bugs related to do/undo of effect properties

[Edward Hervey <edwardrv src gnome org>]

commit a92e2525f7e5cff54021c86cbfefc6ab3614493b
Author: Thibault Saunier <tsaunier gnome org>
Date:   Tue Sep 14 15:19:44 2010 +0200

    Fixe bugs related to do/undo of effect properties

 pitivi/effects.py                 |   19 ++++++++++++++++
 pitivi/pipeline.py                |    7 ++++++
 pitivi/timeline/timeline_undo.py  |   33 ++++++++++++++++++++++++---
 pitivi/timeline/track.py          |    6 ++--
 pitivi/ui/clipproperties.py       |    1 -
 pitivi/ui/effectsconfiguration.py |   22 +-----------------
 pitivi/ui/mainwindow.py           |    2 +
 tests/test_etree_formatter.py     |    2 +-
 tests/test_timeline_undo.py       |   44 ++----------------------------------
 9 files changed, 65 insertions(+), 71 deletions(-)
---
diff --git a/pitivi/effects.py b/pitivi/effects.py
index 2bdaae7..e37f122 100644
--- a/pitivi/effects.py
+++ b/pitivi/effects.py
@@ -326,6 +326,7 @@ class EffectGstElementPropertyChangeTracker:
     def __init__(self, action_log):
         self._tracked_effects = {}
         self.action_log = action_log
+        self.pipeline = None
 
     def addEffectElement(self, gst_element):
         properties = {}
@@ -337,10 +338,28 @@ class EffectGstElementPropertyChangeTracker:
                 properties[prop.name] = gst_element.get_property(prop.name)
         self._tracked_effects[gst_element] = properties
 
+    def getPropChangedFromTrackObj(self, track_effect):
+        prop_changed = []
+
+        for undo_stack in self.action_log.undo_stacks:
+            for done_prop_change in undo_stack.done_actions:
+                if isinstance(done_prop_change, EffectPropertyChanged):
+                    if done_prop_change.gst_element is track_effect.getElement():
+                        prop_changed.append(done_prop_change)
+
+        for redo_stack in self.action_log.redo_stacks:
+            for done_prop_change in redo_stack.done_actions:
+                if isinstance(done_prop_change, EffectPropertyChanged):
+                    if done_prop_change.gst_element is track_effect.getElement():
+                        prop_changed.append(done_prop_change)
+
+        return prop_changed
+
     def _propertyChangedCb(self, gst_element, pspec, unused):
         old_value = self._tracked_effects[gst_element][pspec.name]
         new_value = gst_element.get_property(pspec.name)
         action = EffectPropertyChanged(gst_element, pspec.name, old_value,
                                        new_value)
         self._tracked_effects[gst_element][pspec.name] = new_value
+        self.pipeline.flushSeekVideo()
         self.action_log.push(action)
diff --git a/pitivi/pipeline.py b/pitivi/pipeline.py
index 9b1dd0c..9770986 100644
--- a/pitivi/pipeline.py
+++ b/pitivi/pipeline.py
@@ -199,6 +199,13 @@ class Pipeline(Signallable, Loggable):
         self.debug("Returning")
         return action
 
+    def flushSeekVideo(self):
+        self.pause()
+        try:
+            self.seekRelative(0)
+        except PipelineError:
+            pass
+
     def setAction(self, action):
         """
         Set the given L{Action} on the L{Pipeline}.
diff --git a/pitivi/timeline/timeline_undo.py b/pitivi/timeline/timeline_undo.py
index 5859a5c..292fd67 100644
--- a/pitivi/timeline/timeline_undo.py
+++ b/pitivi/timeline/timeline_undo.py
@@ -160,12 +160,21 @@ class TimelineObjectRemoved(UndoableAction):
         self._undone()
 
 class TrackEffectAdded(UndoableAction):
-    def __init__(self, timeline_object, track_object):
+    # Note: We have a bug if we just remove the TrackEffect from the timeline
+    # and keep it saved here and then readd it to corresponding timeline (it
+    # freezes everything). So what we are doing is  to free the TrackEffect,
+    # keep its settings here when undoing, and instanciate a new one when
+    # doing again. We have to keep all EffectPropertyChanged object that refers
+    # to the TrackEffect when undoing so we reset theirs gst_element when
+    # doing it again. The way of doing it is the same with TrackEffectRemoved
+    def __init__(self, timeline_object, track_object, properties_watcher):
         self.timeline_object = timeline_object
         self.track_object = track_object
         self.factory = track_object.factory
         self.effect_props = []
         self.gnl_obj_props = []
+        self._properties_watcher = properties_watcher
+        self._props_changed = []
 
     def do(self):
         timeline = self.timeline_object.timeline
@@ -178,6 +187,9 @@ class TrackEffectAdded(UndoableAction):
             element.set_property(prop_name, prop_value)
         for prop_name, prop_value in self.gnl_obj_props:
             self.track_object.gnl_object.set_property(prop_name, prop_value)
+        for prop_changed in self._props_changed:
+            prop_changed.gst_element = self.track_object.getElement()
+        self._props_changed = []
 
         self._done()
 
@@ -196,18 +208,22 @@ class TrackEffectAdded(UndoableAction):
 
         self.timeline_object.removeTrackObject(self.track_object)
         self.track_object.track.removeTrackObject(self.track_object)
+        self._props_changed =\
+            self._properties_watcher.getPropChangedFromTrackObj(self.track_object)
         del self.track_object
         self.track_object = None
 
         self._undone()
 
 class TrackEffectRemoved(UndoableAction):
-    def __init__(self, timeline_object, track_object):
+    def __init__(self, timeline_object, track_object, properties_watcher):
         self.track_object = track_object
         self.timeline_object = timeline_object
         self.factory = track_object.factory
         self.effect_props = []
         self.gnl_obj_props = []
+        self._properties_watcher = properties_watcher
+        self._props_changed = []
 
     def do(self):
         element = self.track_object.getElement()
@@ -225,6 +241,8 @@ class TrackEffectRemoved(UndoableAction):
 
         self.timeline_object.removeTrackObject(self.track_object)
         self.track_object.track.removeTrackObject(self.track_object)
+        self._props_changed =\
+            self._properties_watcher.getPropChangedFromTrackObj(self.track_object)
         del self.track_object
         self.track_object = None
 
@@ -241,6 +259,9 @@ class TrackEffectRemoved(UndoableAction):
             element.set_property(prop_name, prop_value)
         for prop_name, prop_value in self.gnl_obj_props:
             self.track_object.gnl_object.set_property(prop_name, prop_value)
+        for prop_changed in self._props_changed:
+            prop_changed.gst_element = self.track_object.getElement()
+        self._props_changed = []
 
         self._undone()
 
@@ -323,6 +344,7 @@ class TimelineLogObserver(object):
         self.timeline_object_property_trackers = {}
         self.interpolator_keyframe_trackers = {}
         self.effect_properties_tracker = EffectGstElementPropertyChangeTracker(log)
+        self._pipeline = None
 
     def startObserving(self, timeline):
         self._connectToTimeline(timeline)
@@ -405,7 +427,8 @@ class TimelineLogObserver(object):
 
     def _timelineObjectTrackObjectAddedCb(self, timeline_object, track_object):
         if isinstance(track_object, TrackEffect):
-            action = self.trackEffectAddAction(timeline_object, track_object)
+            action = self.trackEffectAddAction(timeline_object, track_object,
+                                               self.effect_properties_tracker)
             #We use the action instead of the track object 
             #because the track_object changes when redoing
             track_object.connect("active-changed",
@@ -421,7 +444,9 @@ class TimelineLogObserver(object):
     def _timelineObjectTrackObjectRemovedCb(self, timeline_object,
                                             track_object):
         if isinstance(track_object, TrackEffect):
-            action = self.trackEffectRemovedAction(timeline_object, track_object)
+            action = self.trackEffectRemovedAction(timeline_object,
+                                                track_object,
+                                                self.effect_properties_tracker)
             self.log.push(action)
         else:
             self._disconnectFromTrackObject(track_object)
diff --git a/pitivi/timeline/track.py b/pitivi/timeline/track.py
index 3654335..d5ed061 100644
--- a/pitivi/timeline/track.py
+++ b/pitivi/timeline/track.py
@@ -744,9 +744,9 @@ class TrackEffect(TrackObject):
     def __init__(self, factory, stream, start=0,
             duration=0, in_point=0,
             media_duration=0, priority=0):
-        TrackObject.__init__(self, factory, stream, start=0,
-                             duration=0, in_point=0,
-                             media_duration=0, priority=0)
+        TrackObject.__init__(self, factory, stream, start=start,
+                             duration=duration, in_point=in_point,
+                             media_duration=media_duration, priority=priority)
         self._element = None
 
     def _makeGnlObject(self):
diff --git a/pitivi/ui/clipproperties.py b/pitivi/ui/clipproperties.py
index 4d3d999..f669e6e 100644
--- a/pitivi/ui/clipproperties.py
+++ b/pitivi/ui/clipproperties.py
@@ -79,7 +79,6 @@ class ClipProperties(gtk.VBox, Loggable):
         self._project = project
         if project:
             self.effect_expander._connectTimelineSelection(self._project.timeline)
-            self.effect_properties_handling.pipeline = self._project.pipeline
 
     def _getProject(self):
         return self._project
diff --git a/pitivi/ui/effectsconfiguration.py b/pitivi/ui/effectsconfiguration.py
index 855d1d4..052ef50 100644
--- a/pitivi/ui/effectsconfiguration.py
+++ b/pitivi/ui/effectsconfiguration.py
@@ -33,7 +33,6 @@ PROPS_TO_IGNORE = ['name', 'qos', 'silent', 'message']
 class EffectsPropertiesHandling:
     def __init__(self, action_log):
         self.cache_dict = {}
-        self.pipeline = None
         self._current_effect_setting_ui = None
         self._current_element_values = {}
         self.action_log = action_log
@@ -72,7 +71,7 @@ class EffectsPropertiesHandling:
 
     def cleanCache(self, effect):
         if self.cache_dict.has_key(effect):
-            conf_ui = self.effect_props_handling.cache_dict.get(effect)
+            conf_ui = self.cache_dict.get(effect)
             self.cache_dict.pop(effect)
             return conf_ui
 
@@ -85,14 +84,6 @@ class EffectsPropertiesHandling:
 
         return effect_set_ui
 
-    def _flushSeekVideo(self):
-        self.pipeline.pause()
-        if self.pipeline is not None:
-            try:
-                self.pipeline.seekRelative(0)
-            except PipelineError:
-                pass
-
     def _connectAllWidgetCbs(self, effect_configuration_ui, effect):
         for prop, widget in effect_configuration_ui.properties.iteritems():
             widget.connectValueChanged(self._onValueChangedCb, widget, prop)
@@ -107,14 +98,3 @@ class EffectsPropertiesHandling:
             self._current_effect_setting_ui.element.set_property(prop.name, value)
             self.action_log.commit()
             self._current_element_values[prop.name] = value
-
-        self._flushSeekVideo()
-
-
-class AspectRatioUi(GstElementSettingsWidget):
-    """
-        UI to configure AspectRatio effects
-    """
-    def __init__(self):
-        GstElementSettingsWidget.__init__(self)
-
diff --git a/pitivi/ui/mainwindow.py b/pitivi/ui/mainwindow.py
index 307c58a..18602b8 100644
--- a/pitivi/ui/mainwindow.py
+++ b/pitivi/ui/mainwindow.py
@@ -1017,6 +1017,8 @@ class PitiviMainWindow(gtk.Window, Loggable):
             if self.timeline:
                 self.timeline.project = self.project
                 self.clipconfig.project = self.project
+                self.app.timelineLogObserver.effect_properties_tracker.pipeline\
+                                                        = self.project.pipeline
 
     project = receiver(_setProject)
 
diff --git a/tests/test_etree_formatter.py b/tests/test_etree_formatter.py
index a146432..1f9df38 100644
--- a/tests/test_etree_formatter.py
+++ b/tests/test_etree_formatter.py
@@ -414,7 +414,7 @@ class TestFormatterLoad(TestCase):
                 media_duration=ts(15 * gst.SECOND), priority=ts(5), id="1")
         effect_elem = SubElement(element, "effect")
         factory_elem = SubElement(effect_elem, "factory", name="identity")
-        properties_elem = SubElement(effect_elem, "gst-element-properties", async_handling="(bool)True")
+        properties_elem = SubElement(effect_elem, "gst-element-properties", sync="(bool)True")
 
         # insert our fake factory into the context
         stream = AudioStream(gst.Caps("audio/x-raw-int"))
diff --git a/tests/test_timeline_undo.py b/tests/test_timeline_undo.py
index bf649d5..3370bf6 100644
--- a/tests/test_timeline_undo.py
+++ b/tests/test_timeline_undo.py
@@ -25,6 +25,7 @@ import gobject
 gobject.threads_init()
 import gst
 
+from pitivi.pipeline import Pipeline
 from pitivi.timeline.timeline import Timeline, TimelineObject, SELECT_ADD
 from pitivi.timeline.track import Track, SourceTrackObject, TrackEffect
 from pitivi.factories.test import VideoTestSourceFactory, TestEffectFactory
@@ -167,9 +168,11 @@ class  TestTimelineUndo(TestCase):
 
     def testAddEffectToTimelineObject(self):
         stacks = []
+        pipeline = Pipeline()
         def commitCb(action_log, stack, nested):
             stacks.append(stack)
         self.action_log.connect("commit", commitCb)
+        self.observer.pipeline = pipeline
 
         #FIXME Should I commit it and check there are 2 elements
         #in the stacks
@@ -211,47 +214,6 @@ class  TestTimelineUndo(TestCase):
 
         self.timeline.removeTimelineObject(self.timeline_object1, deep=True)
 
-    def testRemoveEffectToTimelineObject(self):
-        stacks = []
-        def commitCb(action_log, stack, nested):
-            stacks.append(stack)
-        self.action_log.connect("commit", commitCb)
-
-        self.timeline.addTimelineObject(self.timeline_object1)
-        self.timeline_object1.addTrackObject(self.track_effect1)
-        self.track1.addTrackObject(self.track_effect1)
-
-        self.action_log.begin("remove effect")
-        self.timeline_object1.removeTrackObject(self.track_effect1)
-        self.track1.removeTrackObject(self.track_effect1)
-        self.action_log.commit()
-
-        self.failUnlessEqual(len(stacks), 1)
-        stack = stacks[0]
-        self.failUnlessEqual(len(stack.done_actions), 1)
-        action = stack.done_actions[0]
-        self.failUnless(isinstance(action, TrackEffectRemoved))
-
-        self.failIf(self.track_effect1 \
-                in self.timeline_object1.track_objects)
-        self.failIf(self.track_effect1 \
-                in self.track1.track_objects)
-
-        self.action_log.undo()
-        self.failUnless(len([effect for effect in
-                                self.timeline_object1.track_objects
-                                if isinstance(effect, TrackEffect)]) == 1)
-        self.failUnless(len([effect for effect in self.track1.track_objects
-                             if isinstance(effect, TrackEffect)]) == 1)
-
-        self.action_log.redo()
-        self.failIf(self.track_effect1 \
-                in self.timeline_object1.track_objects)
-        self.failIf(self.track_effect1 \
-                in self.track1.track_objects)
-
-        self.timeline.removeTimelineObject(self.timeline_object1, deep=True)
-
     def testTimelineObjectPropertyChange(self):
         stacks = []
         def commitCb(action_log, stack, nested):