Skip to content

Commit

Permalink
Enforce stricter interval algebra rules for overlaps and contains (#760)
Browse files Browse the repository at this point in the history 
* Create python-package.yml

* Update flake8 config

* Enforce stricter interval algebra rules for overlaps operator
and update trackAlgorithm accordingly.

* Enforce stricter interval algebra rules for contains operator
and update trackAlgorithm accordingly

* Minor fixes

* Add intersects predicate, constexpr functions to compare time and reformatting

* Make greater_than() & less_than() just inline

* Delete python-package.yml

Co-authored-by: Nick Porcino <[email protected]>
  • Loading branch information
@KarthikRIyer @meshula
KarthikRIyer and meshula authored Aug 21, 2020
1 parent 887575a commit 65ffe2a
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 52 deletions.
101 changes: 66 additions & 35 deletions src/opentime/timeRange.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace opentime {
*/

/**
* This default epsilon value is used in comparison between floating numbers.
* This default epsilon_s value is used in comparison between floating numbers.
* It is computed to be twice 192khz, the fastest commonly used audio rate.
* It can be changed in the future if necessary due to higher sampling rates
* or some other kind of numeric tolerance detected in the library.
Expand Down Expand Up @@ -92,8 +92,9 @@ class TimeRange {
*/

/**
* In the relations that follow, epsilon indicates the tolerance,in the sense that if abs(a-b) < epsilon,
* we consider a and b to be equal
* In the relations that follow, epsilon_s indicates the tolerance,in the sense that if abs(a-b) < epsilon_s,
* we consider a and b to be equal.
* The time comparison is done in double seconds.
*/

/**
Expand All @@ -117,8 +118,12 @@ class TimeRange {
* The converse would be <em>other.contains(this)</em>
* @param other
*/
bool contains(TimeRange other) const {
return _start_time <= other._start_time && end_time_exclusive() >= other.end_time_exclusive();
bool contains(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisStart = _start_time.to_seconds();
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
double otherEnd = other.end_time_exclusive().to_seconds();
return greater_than(otherStart, thisStart, epsilon_s) && lesser_than(otherEnd, thisEnd, epsilon_s);
}

/**
Expand All @@ -134,48 +139,49 @@ class TimeRange {
}

/**
* The start of <b>this</b> strictly precedes end of <b>other</b> by a value >= <b>epsilon</b>.
* The end of <b>this</b> strictly antecedes start of <b>other</b> by a value >= <b>epsilon</b>.
* The start of <b>this</b> strictly precedes end of <b>other</b> by a value >= <b>epsilon_s</b>.
* The end of <b>this</b> strictly antecedes start of <b>other</b> by a value >= <b>epsilon_s</b>.
* [ this ]
* [ other ]
* The converse would be <em>other.overlaps(this)</em>
* @param other
* @param epsilon
* @param epsilon_s
*/
bool overlaps(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
bool overlaps(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisStart = _start_time.to_seconds();
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
double otherEnd = other.end_time_exclusive().to_seconds();
return (otherEnd - thisStart >= epsilon) &&
(thisEnd - otherStart >= epsilon);
return lesser_than(thisStart, otherStart, epsilon_s) &&
greater_than(thisEnd, otherStart, epsilon_s) &&
greater_than(otherEnd, thisEnd, epsilon_s);
}

/**
* The end of <b>this</b> strictly precedes the start of <b>other</b> by a value >= <b>epsilon</b>.
* The end of <b>this</b> strictly precedes the start of <b>other</b> by a value >= <b>epsilon_s</b>.
* [ this ] [ other ]
* The converse would be <em>other.before(this)</em>
* @param other
* @param epsilon
* @param epsilon_s
*/
bool before(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
bool before(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
return otherStart - thisEnd >= epsilon;
return greater_than(otherStart, thisEnd, epsilon_s);
}

/**
* The end of <b>this</b> strictly precedes <b>other</b> by a value >= <b>epsilon</b>.
* The end of <b>this</b> strictly precedes <b>other</b> by a value >= <b>epsilon_s</b>.
* other
* ↓
* [ this ] *
* @param other
* @param epsilon
* @param epsilon_s
*/
bool before(RationalTime other, double epsilon = DEFAULT_EPSILON_s) const {
bool before(RationalTime other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisEnd = end_time_exclusive().to_seconds();
double otherTime = other.to_seconds();
return otherTime - thisEnd >= epsilon;
return lesser_than(thisEnd, otherTime, epsilon_s);
}

/**
Expand All @@ -184,29 +190,29 @@ class TimeRange {
* [this][other]
* The converse would be <em>other.meets(this)</em>
* @param other
* @param epsilon
* @param epsilon_s
*/
bool meets(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
bool meets(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
return otherStart - thisEnd <= epsilon && otherStart - thisEnd >= 0;
return otherStart - thisEnd <= epsilon_s && otherStart - thisEnd >= 0;
}

/**
* The start of <b>this</b> strictly equals the start of <b>other</b>.
* The end of <b>this</b> strictly precedes the end of <b>other</b> by a value >= <b>epsilon</b>.
* The end of <b>this</b> strictly precedes the end of <b>other</b> by a value >= <b>epsilon_s</b>.
* [ this ]
* [ other ]
* The converse would be <em>other.begins(this)</em>
* @param other
* @param epsilon
* @param epsilon_s
*/
bool begins(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
bool begins(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisStart = _start_time.to_seconds();
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
double otherEnd = other.end_time_exclusive().to_seconds();
return fabs(otherStart - thisStart) <= epsilon && otherEnd - thisEnd >= epsilon;
return fabs(otherStart - thisStart) <= epsilon_s && lesser_than(thisEnd, otherEnd, epsilon_s);
}

/**
Expand All @@ -217,27 +223,27 @@ class TimeRange {
* [ this ]
* @param other
*/
bool begins(RationalTime other, double epsilon = DEFAULT_EPSILON_s) const {
bool begins(RationalTime other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisStart = _start_time.to_seconds();
double otherStart = other.to_seconds();
return fabs(otherStart - thisStart) <= epsilon;
return fabs(otherStart - thisStart) <= epsilon_s;
}

/**
* The start of <b>this</b> strictly antecedes the start of <b>other</b> by a value >= <b>epsilon</b>.
* The start of <b>this</b> strictly antecedes the start of <b>other</b> by a value >= <b>epsilon_s</b>.
* The end of <b>this</b> strictly equals the end of <b>other</b>.
* [ this ]
* [ other ]
* The converse would be <em>other.finishes(this)</em>
* @param other
* @param epsilon
* @param epsilon_s
*/
bool finishes(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
bool finishes(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisStart = _start_time.to_seconds();
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
double otherEnd = other.end_time_exclusive().to_seconds();
return fabs(thisEnd - otherEnd) <= epsilon && thisStart - otherStart >= epsilon;
return fabs(thisEnd - otherEnd) <= epsilon_s && greater_than(thisStart, otherStart, epsilon_s);
}

/**
Expand All @@ -247,12 +253,29 @@ class TimeRange {
* *
* [ this ]
* @param other
* @param epsilon
* @param epsilon_s
*/
bool finishes(RationalTime other, double epsilon = DEFAULT_EPSILON_s) const {
bool finishes(RationalTime other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisEnd = end_time_exclusive().to_seconds();
double otherEnd = other.to_seconds();
return fabs(thisEnd - otherEnd) <= epsilon;
return fabs(thisEnd - otherEnd) <= epsilon_s;
}

/**
* The start of <b>this</b> precedes or equals the end of <b>other</b> by a value >= <b>epsilon_s</b>.
* The end of <b>this</b> antecedes or equals the start of <b>other</b> by a value >= <b>epsilon_s</b>.
* [ this ] OR [ other ]
* [ other ] [ this ]
* The converse would be <em>other.finishes(this)</em>
* @param other
* @param epsilon_s
*/
bool intersects(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
double thisStart = _start_time.to_seconds();
double thisEnd = end_time_exclusive().to_seconds();
double otherStart = other._start_time.to_seconds();
double otherEnd = other.end_time_exclusive().to_seconds();
return lesser_than(thisStart, otherEnd, epsilon_s) && greater_than(thisEnd, otherStart, epsilon_s);
}


Expand Down Expand Up @@ -288,6 +311,14 @@ class TimeRange {
private:
RationalTime _start_time, _duration;
friend class TimeTransform;

inline bool greater_than(double lhs, double rhs, double epsilon) const{
return lhs - rhs >= epsilon;
}

inline bool lesser_than(double lhs, double rhs, double epsilon) const{
return rhs - lhs >= epsilon;
}
};

} }
Expand Down
2 changes: 1 addition & 1 deletion src/opentimelineio/trackAlgorithm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Track* track_trimmed_to_range(Track* in_track, TimeRange trim_range, ErrorStatus
}

auto child_range = child_range_it->second;
if (!trim_range.overlaps(child_range)) {
if (!trim_range.intersects(child_range)) {
new_track->remove_child(static_cast<int>(i), error_status);
if (*error_status) {
return nullptr;
Expand Down
19 changes: 10 additions & 9 deletions src/py-opentimelineio/opentime-bindings/opentime_timeRange.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,17 @@ void opentime_timeRange_bindings(py::module m) {
.def("clamped", (RationalTime (TimeRange::*)(RationalTime) const) &TimeRange::clamped, "other"_a)
.def("clamped", (TimeRange (TimeRange::*)(TimeRange) const) &TimeRange::clamped, "other"_a)
.def("contains", (bool (TimeRange::*)(RationalTime) const) &TimeRange::contains, "other"_a)
.def("contains", (bool (TimeRange::*)(TimeRange) const) &TimeRange::contains, "other"_a)
.def("contains", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::contains, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("overlaps", (bool (TimeRange::*)(RationalTime) const) &TimeRange::overlaps, "other"_a)
.def("overlaps", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::overlaps, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("before", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::before, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("before", (bool (TimeRange::*)(RationalTime, double ) const) &TimeRange::before, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("meets", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::meets, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("begins", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::begins, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("begins", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::begins, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("finishes", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::finishes, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("finishes", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::finishes, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
.def("overlaps", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::overlaps, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("before", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::before, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("before", (bool (TimeRange::*)(RationalTime, double ) const) &TimeRange::before, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("meets", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::meets, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("begins", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::begins, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("begins", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::begins, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("finishes", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::finishes, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("finishes", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::finishes, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("intersects", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::intersects, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
.def("__copy__", [](TimeRange tr) {
return tr;
})
Expand Down
2 changes: 1 addition & 1 deletion src/py-opentimelineio/opentimelineio/algorithms/track_algo.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def track_trimmed_to_range(in_track, trim_range):
# iterate backwards so we can delete items
for c, child in reversed(list(enumerate(new_track))):
child_range = track_map[child]
if not trim_range.overlaps(child_range):
if not trim_range.intersects(child_range):
# completely outside the trim range, so we discard it
del new_track[c]
elif trim_range.contains(child_range):
Expand Down
83 changes: 77 additions & 6 deletions tests/test_opentime.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ def test_contains(self):
self.assertFalse(tr.contains(tstart + tdur))
self.assertFalse(tr.contains(tstart - tdur))

self.assertTrue(tr.contains(tr))
self.assertFalse(tr.contains(tr))

tr_2 = otio.opentime.TimeRange(tstart - tdur, tdur)
self.assertFalse(tr.contains(tr_2))
Expand Down Expand Up @@ -915,25 +915,25 @@ def test_overlaps_timerange(self):
tdur = otio.opentime.RationalTime(3, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.overlaps(tr_t))
self.assertFalse(tr.overlaps(tr_t))

tstart = otio.opentime.RationalTime(13, 25)
tdur = otio.opentime.RationalTime(1, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.overlaps(tr_t))
self.assertFalse(tr.overlaps(tr_t))

tstart = otio.opentime.RationalTime(2, 25)
tdur = otio.opentime.RationalTime(30, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.overlaps(tr_t))
self.assertFalse(tr.overlaps(tr_t))

tstart = otio.opentime.RationalTime(2, 50)
tdur = otio.opentime.RationalTime(60, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.overlaps(tr_t))
self.assertFalse(tr.overlaps(tr_t))

tstart = otio.opentime.RationalTime(2, 50)
tdur = otio.opentime.RationalTime(14, 50)
Expand All @@ -945,14 +945,85 @@ def test_overlaps_timerange(self):
tdur = otio.opentime.RationalTime(400, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.overlaps(tr_t))
self.assertFalse(tr.overlaps(tr_t))

tstart = otio.opentime.RationalTime(100, 50)
tdur = otio.opentime.RationalTime(400, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertFalse(tr.overlaps(tr_t))

def test_intersects_timerange(self):
tstart = otio.opentime.RationalTime(12, 25)
tdur = otio.opentime.RationalTime(3, 25)
tr = otio.opentime.TimeRange(tstart, tdur)

tstart = otio.opentime.RationalTime(0, 25)
tdur = otio.opentime.RationalTime(3, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertFalse(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(10, 25)
tdur = otio.opentime.RationalTime(3, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(10, 25)
tdur = otio.opentime.RationalTime(2, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertFalse(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(14, 25)
tdur = otio.opentime.RationalTime(2, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(15, 25)
tdur = otio.opentime.RationalTime(2, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertFalse(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(13, 25)
tdur = otio.opentime.RationalTime(1, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(2, 25)
tdur = otio.opentime.RationalTime(30, 25)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(2, 50)
tdur = otio.opentime.RationalTime(60, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(2, 50)
tdur = otio.opentime.RationalTime(14, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertFalse(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(-100, 50)
tdur = otio.opentime.RationalTime(400, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertTrue(tr.intersects(tr_t))

tstart = otio.opentime.RationalTime(100, 50)
tdur = otio.opentime.RationalTime(400, 50)
tr_t = otio.opentime.TimeRange(tstart, tdur)

self.assertFalse(tr.intersects(tr_t))

def test_before_timerange(self):
tstart = otio.opentime.RationalTime(12, 25)
tdur = otio.opentime.RationalTime(3, 25)
Expand Down

0 comments on commit 65ffe2a

Please sign in to comment.