Skip to content

#841 Track range_of_all_children applies TimeEffect effect to duratio… #842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
splidje wants to merge 7 commits into
base: main
Choose a base branch
from
Open

#841 Track range_of_all_children applies TimeEffect effect to duratio… #842

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a2bfebf
#841 Track range_of_all_children applies TimeEffect effect to duratio…
splidje Dec 10, 2020
67046c5
#841 addressed code review comments.
splidje Dec 11, 2020
c3bea68
#841 added test for LinearTimeWarp output_range
splidje Dec 11, 2020
7567875
#841 made FreezeFrame a direct subclass of TimeEffect to differentiat…
splidje Dec 11, 2020
489c071
addressed lint issues. still 1 test failing I can't get my head aroun…
splidje Dec 11, 2020
553e112
Further down the rabbit hole of TimeEffect giving the answer to effec…
splidje Dec 15, 2020
1a8b2aa
Back to the previous model of an item's source_range indicating its d…
splidje Dec 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/opentime/rationalTime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ class RationalTime {
return RationalTime {-lhs._value, lhs._rate};
}

friend RationalTime operator* (RationalTime lhs, double rhs) {
return RationalTime {lhs._value * rhs, lhs._rate};
}

friend RationalTime operator/ (RationalTime lhs, double rhs) {
return RationalTime {lhs._value / rhs, lhs._rate};
}

@reinecke reinecke Dec 10, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We originally omitted overloading / and * operators for RationalTIme because we felt that the meaning of division and multiplication in the context of RationalTime could potentially be ambiguous. Instead we provided TimeTransform to make sure it was explicit how these operations work.

friend bool operator> (RationalTime lhs, RationalTime rhs) {
return (lhs._value / lhs._rate) > (rhs._value / rhs._rate);
}
Expand Down
5 changes: 5 additions & 0 deletions src/opentimelineio/linearTimeWarp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "opentimelineio/version.h"
#include "opentimelineio/timeEffect.h"
#include "opentime/timeRange.h"

namespace opentimelineio { namespace OPENTIMELINEIO_VERSION {

Expand All @@ -27,6 +28,10 @@ class LinearTimeWarp : public TimeEffect {
_time_scalar = time_scalar;
}

virtual TimeRange output_range(TimeRange input_range, ErrorStatus* error_status) const {
return TimeRange(input_range.start_time(), input_range.duration() / _time_scalar);

@reinecke reinecke Dec 10, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you could construct aTimeTransform and then use .applied_to(input_range.duration() to compute the scaled duration.

Also, another thing to consider is what happens when _time_scalar is zero (like for FreezeFrame). Perhaps, if it doesn't exist, a unittest for the FreezeFame case should be added.

}

protected:
virtual ~LinearTimeWarp();

Expand Down
5 changes: 5 additions & 0 deletions src/opentimelineio/timeEffect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,9 @@ TimeEffect::TimeEffect(std::string const& name,
TimeEffect::~TimeEffect() {
}

TimeRange TimeEffect::output_range(TimeRange input_range, ErrorStatus* error_status) const {
*error_status = ErrorStatus::NOT_IMPLEMENTED;
return TimeRange();
}

} }
4 changes: 4 additions & 0 deletions src/opentimelineio/timeEffect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "opentimelineio/version.h"
#include "opentimelineio/effect.h"
#include "opentime/timeRange.h"

namespace opentimelineio { namespace OPENTIMELINEIO_VERSION {

Expand All @@ -17,6 +18,9 @@ class TimeEffect : public Effect {
TimeEffect(std::string const& name = std::string(),
std::string const& effect_name = std::string(),
AnyDictionary const& metadata = AnyDictionary());

virtual TimeRange output_range(TimeRange input_range, ErrorStatus* error_status) const;

protected:
virtual ~TimeEffect();

Expand Down
9 changes: 8 additions & 1 deletion src/opentimelineio/track.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "opentimelineio/transition.h"
#include "opentimelineio/gap.h"
#include "opentimelineio/vectorIndexing.h"
#include "opentimelineio/timeEffect.h"

namespace opentimelineio { namespace OPENTIMELINEIO_VERSION {

Expand Down Expand Up @@ -196,7 +197,13 @@ std::map<Composable*, TimeRange> Track::range_of_all_children(ErrorStatus* error
transition->out_offset() + transition->in_offset());
}
else if (auto item = dynamic_cast<Item*>(child.value)) {
auto last_range = TimeRange(last_end_time, item->trimmed_range(error_status).duration());
auto output_range = TimeRange(RationalTime(0), item->trimmed_range(error_status).duration());

@reinecke reinecke Dec 10, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since considerable work is potentially being done in the following for loop, should error_status be right after instead of later below?

for (auto effect: item->effects()) {
if (auto time_effect = dynamic_cast<TimeEffect*>(effect.value)) {
output_range = time_effect->output_range(output_range, error_status);
}
}
auto last_range = TimeRange(last_end_time, output_range.duration());
result[child] = last_range;
last_end_time = last_range.end_time_exclusive();
}
Expand Down
2 changes: 2 additions & 0 deletions src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ void opentime_rationalTime_bindings(py::module m) {
})
.def(py::self - py::self)
.def(py::self + py::self)
.def(py::self * double())
.def(py::self / double())
// The simple "py::self += py::self" returns the original,
// which is not what we want here: we need this to return a new copy
// to avoid mutating any additional references, since this class has complete value semantics.
Expand Down
5 changes: 4 additions & 1 deletion src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,10 @@ static void define_effects(py::module m) {
return new TimeEffect(name, effect_name, py_to_any_dictionary(metadata)); }),
name_arg,
"effect_name"_a = std::string(),
metadata_arg);
metadata_arg)
.def("output_range", [](TimeEffect* effect, TimeRange input_range) {
return effect->output_range(input_range, ErrorStatusHandler());
});

py::class_<LinearTimeWarp, TimeEffect, managing_ptr<LinearTimeWarp>>(m, "LinearTimeWarp", py::dynamic_attr())
.def(py::init([](std::string name,
Expand Down