#841 Track range_of_all_children applies TimeEffect effect to duratio…

contrib/opentimelineio_contrib/adapters/tests/test_aaf_adapter.py

@@ -743,7 +743,6 @@ def test_read_misc_speed_effects(self):
         self.assertEqual(1, len(clip.effects))
         effect = clip.effects[0]
         self.assertEqual(otio.schema.FreezeFrame, type(effect))
-        self.assertEqual(0, effect.time_scalar)
         self.assertEqual(8, clip.duration().value)
 
         clip = track[2]

docs/tutorials/otio-serialized-schema.md

@@ -318,7 +318,6 @@ parameters:
 - *effect_name*: 
 - *metadata*: 
 - *name*: 
-- *time_scalar*: 
 
 ### Gap.1
 

src/opentimelineio/freezeFrame.cpp

@@ -4,7 +4,7 @@ namespace opentimelineio { namespace OPENTIMELINEIO_VERSION  {
 
 FreezeFrame::FreezeFrame(std::string const& name,
                          AnyDictionary const& metadata)
-    : Parent(name, "FreezeFrame", 0.0, metadata) {
+    : Parent(name, "FreezeFrame", metadata) {
 }
 
 FreezeFrame::~FreezeFrame() {

src/opentimelineio/freezeFrame.h

@@ -1,18 +1,18 @@
 #pragma once
 
 #include "opentimelineio/version.h"
-#include "opentimelineio/linearTimeWarp.h"
+#include "opentimelineio/timeEffect.h"
 
 namespace opentimelineio { namespace OPENTIMELINEIO_VERSION  {
 
-class FreezeFrame : public LinearTimeWarp {
+class FreezeFrame : public TimeEffect {
 public:
     struct Schema {
         static auto constexpr name = "FreezeFrame";
         static int constexpr version = 1;
     };
 
-    using Parent = LinearTimeWarp;
+    using Parent = TimeEffect;
 
     FreezeFrame(std::string const& name = std::string(),
                 AnyDictionary const& metadata = AnyDictionary());
@@ -21,7 +21,6 @@ class FreezeFrame : public LinearTimeWarp {
     virtual ~FreezeFrame();
 
 private:
-
 };
 
 } }

src/opentimelineio/linearTimeWarp.cpp

@@ -3,10 +3,9 @@
 namespace opentimelineio { namespace OPENTIMELINEIO_VERSION  {
 
 LinearTimeWarp::LinearTimeWarp(std::string const& name,
-                               std::string const& effect_name,
                                double time_scalar,
                                AnyDictionary const& metadata)
-    : Parent(name, effect_name, metadata),
+    : Parent(name, "LinearTimeWarp", metadata),
       _time_scalar(time_scalar) {
 }
 

src/opentimelineio/linearTimeWarp.h

@@ -15,7 +15,6 @@ class LinearTimeWarp : public TimeEffect {
     using Parent = TimeEffect;
 
     LinearTimeWarp(std::string const& name = std::string(),
-                   std::string const& effect_name = std::string(),
                    double time_scalar = 1,
                    AnyDictionary const& metadata = AnyDictionary());
 
@@ -24,6 +23,8 @@ class LinearTimeWarp : public TimeEffect {
     }
 
     void set_time_scalar(double time_scalar) {
+        // TODO: Is there a concept of validation within set methods?
+        // I think the only invalid value is 0 (use a freeze frame)
         _time_scalar = time_scalar;
     }
 

src/opentimelineio/timeEffect.h

@@ -17,6 +17,7 @@ class TimeEffect : public Effect {
     TimeEffect(std::string const& name = std::string(),
                std::string const& effect_name = std::string(),
                AnyDictionary const& metadata = AnyDictionary());
+
 protected:
     virtual ~TimeEffect();
 

src/opentimelineio/track.cpp

@@ -197,13 +197,12 @@ std::map<Composable*, TimeRange> Track::range_of_all_children(ErrorStatus* error
         }
         else if (auto item = dynamic_cast<Item*>(child.value)) {
             auto last_range = TimeRange(last_end_time, item->trimmed_range(error_status).duration());
+            if (*error_status) {
+                return result;
+            }
             result[child] = last_range;
             last_end_time = last_range.end_time_exclusive();
         }
-
-        if (*error_status) {
-            return result;
-        }
     }
 
     return result;

src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp

@@ -551,14 +551,14 @@ static void define_effects(py::module m) {
         .def(py::init([](std::string name,
                          double time_scalar,
                          py::object metadata) {
-                          return new LinearTimeWarp(name, "LinearTimeWarp", time_scalar,
+                          return new LinearTimeWarp(name, time_scalar,
                                                     py_to_any_dictionary(metadata)); }),
              name_arg,
              "time_scalar"_a = 1.0,
              metadata_arg)
         .def_property("time_scalar", &LinearTimeWarp::time_scalar, &LinearTimeWarp::set_time_scalar);
 
-    py::class_<FreezeFrame, LinearTimeWarp, managing_ptr<FreezeFrame>>(m, "FreezeFrame", py::dynamic_attr())
+    py::class_<FreezeFrame, TimeEffect, managing_ptr<FreezeFrame>>(m, "FreezeFrame", py::dynamic_attr())
         .def(py::init([](std::string name, py::object metadata) {
                     return new FreezeFrame(name, py_to_any_dictionary(metadata)); }),
             name_arg,

src/py-opentimelineio/opentimelineio/adapters/cmx_3600.py

@@ -148,33 +148,35 @@ def add_clip(self, line, comments, rate=24):
             edl_rate
         )
 
-        src_duration = clip.duration()
         rec_duration = record_out - record_in
-        if rec_duration != src_duration:
-            motion = comment_handler.handled.get('motion_effect')
-            freeze = comment_handler.handled.get('freeze_frame')
-            if motion is not None or freeze is not None:
-                # Adjust the clip to match the record duration
-                clip.source_range = opentime.TimeRange(
-                    start_time=clip.source_range.start_time,
-                    duration=rec_duration
+
+        motion = comment_handler.handled.get('motion_effect')
+        if motion is not None:
+            fps = float(
+                SPEED_EFFECT_RE.match(motion).group("speed")
+            )
+            # linear time warp
+            if fps:
+                time_scalar = fps / rate
+                clip.effects.append(
+                    schema.LinearTimeWarp(time_scalar=time_scalar)
                 )
+            # freeze frame
+            else:
+                clip.effects.append(schema.FreezeFrame())
+                # XXX remove 'FF' suffix (writing edl will add it back)
+                if clip.name.endswith(' FF'):
+                    clip.name = clip.name[:-3]
+
+            clip.source_range = opentime.TimeRange(
+                start_time=clip.source_range.start_time,
+                duration=rec_duration,
+            )
 
-                if freeze is not None:
-                    clip.effects.append(schema.FreezeFrame())
-                    # XXX remove 'FF' suffix (writing edl will add it back)
-                    if clip.name.endswith(' FF'):
-                        clip.name = clip.name[:-3]
-                elif motion is not None:
-                    fps = float(
-                        SPEED_EFFECT_RE.match(motion).group("speed")
-                    )
-                    time_scalar = fps / rate
-                    clip.effects.append(
-                        schema.LinearTimeWarp(time_scalar=time_scalar)
-                    )
+        src_duration = clip.duration()
 
-            elif self.ignore_timecode_mismatch:
+        if rec_duration != src_duration:
+            if self.ignore_timecode_mismatch:
                 # Pretend there was no problem by adjusting the record_out.
                 # Note that we don't actually use record_out after this
                 # point in the code, since all of the subsequent math uses
@@ -1005,7 +1007,7 @@ def get_content_for_track_at_index(self, idx, title):
 def _supported_timing_effects(clip):
     return [
         fx for fx in clip.effects
-        if isinstance(fx, schema.LinearTimeWarp)
+        if isinstance(fx, (schema.FreezeFrame, schema.LinearTimeWarp))
     ]
 
 
@@ -1061,7 +1063,7 @@ def __init__(
 
         range_in_timeline = clip.transformed_time_range(
             clip.trimmed_range(),
-            tracks
+            tracks,
         )
         line.record_in = range_in_timeline.start_time
         line.record_out = range_in_timeline.end_time_exclusive()
@@ -1289,14 +1291,21 @@ def _generate_comment_lines(
             )
         )
 
-    if timing_effect and isinstance(timing_effect, schema.LinearTimeWarp):
+    if (
+        timing_effect
+        and isinstance(timing_effect, (schema.FreezeFrame, schema.LinearTimeWarp))
+    ):
+        if isinstance(timing_effect, schema.FreezeFrame):
+            new_rate = 0
+        elif isinstance(timing_effect, schema.LinearTimeWarp):
+            new_rate = timing_effect.time_scalar * edl_rate
         lines.append(
             'M2   {}\t\t{}\t\t\t{}'.format(
                 clip.name,
-                timing_effect.time_scalar * edl_rate,
+                new_rate,
                 opentime.to_timecode(
                     clip.trimmed_range().start_time,
-                    edl_rate
+                    edl_rate,
                 )
             )
         )

tests/sample_data/speed_effects.edl

@@ -566,7 +566,7 @@ M2   Z682_157       000.0                01:00:10:20
 000281  Z686_5A. V     C        01:00:04:04 01:00:06:00 01:11:29:20 01:11:31:16 
 * FROM CLIP NAME:  Z686_5A (LAY2)
 000282  Z686_5A. V     C        01:00:06:00 01:00:08:22 01:11:31:16 01:11:33:04 
-M2   Z686_5A.       047.6                01:00:06:00 
+M2   Z686_5A.       046.7                01:00:06:00
 * FROM CLIP NAME:  Z686_5A (LAY2) (47.56 FPS)
 000283  Z686_7.R V     C        01:00:09:15 01:00:10:07 01:11:33:04 01:11:33:20 
 * FROM CLIP NAME:  Z686_7 (RENDER-ANIM20)
@@ -951,8 +951,8 @@ M2   Z686_5A.       047.6                01:00:06:00
 000473  Z694_51B V     C        01:00:14:21 01:00:15:13 01:17:19:12 01:17:20:04 
 * FROM CLIP NAME:  Z694_51B (LAY2)
 000474  Z694_51B V     C        01:00:15:13 01:00:16:04 01:17:20:04 01:17:20:10 
-M2   Z694_51B       068.0                01:00:15:13 
-* FROM CLIP NAME:  Z694_51B (LAY2) (68.00 FPS)
+M2   Z694_51B       060.0                01:00:15:13
+* FROM CLIP NAME:  Z694_51B (LAY2) (60.00 FPS)
 000475  Z694_51B V     C        01:00:16:05 01:00:17:18 01:17:20:10 01:17:21:05 
 M2   Z694_51B       047.8                01:00:16:05 
 * FROM CLIP NAME:  Z694_51B (LAY2) (47.75 FPS)
@@ -968,15 +968,15 @@ M2   Z694_MLE       029.9                01:00:17:16
 M2   Z694_MLE       035.1                01:00:18:15 
 * FROM CLIP NAME:  Z694_56C (LAY2) (35.08 FPS)
 000480  Z694_MLE V     C        01:00:18:19 01:00:21:11 01:17:23:16 01:17:25:11 
-M2   Z694_MLE       036.2                01:00:18:19 
-* FROM CLIP NAME:  Z694_56B (LAY5) (36.21 FPS)
+M2   Z694_MLE       035.7                01:00:18:19
+* FROM CLIP NAME:  Z694_56B (LAY5) (35.721 FPS)
 000481  Z694_56D V     C        01:00:21:09 01:00:23:00 01:17:25:11 01:17:27:02 
 * FROM CLIP NAME:  Z694_56D (LAY4)
 000482  Z694_MLE V     C        01:00:17:07 01:00:18:09 01:17:27:02 01:17:28:04 
 * FROM CLIP NAME:  Z694_151 (LAY3)
 000483  Z694_MLE V     C        01:00:18:09 01:00:19:01 01:17:28:04 01:17:28:13 
-M2   Z694_MLE       045.3                01:00:18:09 
-* FROM CLIP NAME:  Z694_151 (LAY3) (45.33 FPS)
+M2   Z694_MLE       042.7                01:00:18:09
+* FROM CLIP NAME:  Z694_151 (LAY3) (42.667 FPS)
 000484  Z694_MLE V     C        01:00:19:01 01:00:20:01 01:17:28:13 01:17:29:13 
 * FROM CLIP NAME:  Z694_151 (LAY3)
 000485  Z694_154 V     C        01:00:21:12 01:00:23:21 01:17:29:13 01:17:31:22 
@@ -986,8 +986,8 @@ M2   Z694_MLE       045.3                01:00:18:09
 000487  Z694_MLE V     C        01:00:22:22 01:00:25:09 01:17:34:10 01:17:36:21 
 * FROM CLIP NAME:  Z694_153 (LAY2)
 000488  Z694_SHI V     C        01:00:25:01 01:00:33:09 01:17:36:21 01:17:41:02 
-M2   Z694_SHI       047.9                01:00:25:01 
-* FROM CLIP NAME:  Z694_ZZZ_1T (FX7) (47.88 FPS)
+M2   Z694_SHI       047.5                01:00:25:01
+* FROM CLIP NAME:  Z694_ZZZ_1T (FX7) (47.525 FPS)
 000489  DEVFX_HY V     C        01:00:10:04 01:00:12:13 01:17:41:02 01:17:43:11
 * FROM CLIP NAME:  DEVFX_ZZZ_3 (FX2)
 000490  Z694_201 V     C        01:00:04:11 01:00:06:10 01:17:43:11 01:17:45:10 
@@ -1073,7 +1073,7 @@ M2   Z694_307       000.0                01:00:38:21
 000525  Z700_303 V     C        01:00:06:04 01:00:07:18 01:19:25:12 01:19:27:02 
 * FROM CLIP NAME:  Z700_303B (LAY1)
 000526  Z700_303 V     C        01:00:07:18 01:00:10:03 01:19:27:02 01:19:28:05 
-M2   Z700_303       052.6                01:00:07:18 
+M2   Z700_303       050.7                01:00:07:18
 * FROM CLIP NAME:  Z700_303B (LAY1)
 000527  Z700_304 V     C        01:00:04:18 01:00:06:23 01:19:28:05 01:19:30:10 
 * FROM CLIP NAME:  Z700_304 (LAY6)

tests/sample_data/speed_effects_small.edl

@@ -13,5 +13,5 @@ M2   Z682_157       000.0                01:00:10:20
 004  Z682_157 V     C        01:00:10:20 01:00:11:14 01:08:30:18 01:08:31:12 
 * FROM CLIP NAME:  Z682_157 (LAY2)
 005  Z686_5A. V     C        01:00:06:00 01:00:08:22 01:11:31:16 01:11:33:04 
-M2   Z686_5A.       047.6                01:00:06:00 
-* FROM CLIP NAME:  Z686_5A (LAY2) (47.56 FPS)
+M2   Z686_5A.       046.7                01:00:06:00
+* FROM CLIP NAME:  Z686_5A (LAY2) (46.66 FPS)

tests/test_cmx_3600_adapter.py

@@ -997,7 +997,7 @@ def test_speed_effects(self):
         self.assertTrue(
             clip.effects and clip.effects[0].effect_name == "LinearTimeWarp"
         )
-        self.assertAlmostEqual(clip.effects[0].time_scalar, 1.98333333)
+        self.assertAlmostEqual(clip.effects[0].time_scalar, 1.94583333)
 
         self.assertIsNone(
             clip.metadata.get("cmx_3600", {}).get("motion")

tests/test_effect.py

@@ -98,7 +98,6 @@ def test_cons(self):
         ef = otio.schema.FreezeFrame("Foo", {'foo': 'bar'})
         self.assertEqual(ef.effect_name, "FreezeFrame")
         self.assertEqual(ef.name, "Foo")
-        self.assertEqual(ef.time_scalar, 0)
         self.assertEqual(ef.metadata, {"foo": "bar"})