Skip to content

Commit 8a7bd25

Browse files
authored
Address big integers bug (#892)
* coerce all integer types to int64_t - to remove integer ambiguity around 32/64bit integers, we coerce all ints to int64_t. * handle int64_t constraint/ ImageSequenceReference * plumb uint64_t support all the way through the SDK - When a uint64_t is serialized, it is bitwise & with INT64_MAX first to avoid overflowing the integer. In order to do that comparison at serialization time, the uint64_t needs to be stored as a uint64_t all the way through the python bindings into the any mapping. - the next step is to move the & operation up into the python bindings and prevent people from storing invalid numbers in OTIO files. * add negative numbers to test and compare values Please enter the commit message for your changes. Lines starting * add in C++ sdk tests * add unicode unit test as well * clean up big integers test * add serialization support for nan, inf, etc - this is to allow parity with the values in python files * [de]serialize a variety of numeric types correctly - add support in the serializer/deserializer for NaN, Inf, -Infinity etc from rapidjson - block pybind11 from automatically casting numbers that don't fit into double or int64_t into bool - add a unit test and test data to test the serialization and behavior * Clean up invalid type/value exceptions in Any - Adds specific messages that test if something is an out of range integer, or an invalid type - includes a list of supported types * cleanup * Add note to docs about supported numeric types * Lint fixes * Fix ImageSequenceReference cloning for int64_t - The 'Writer' part of SerializableObject needed the same tightening as the RapidJSON dispatcher - this is used for things that serialize and compare JSON (like cloning and the `is_equivalent_to` method. - Because the ImageSequenceReference schema has `int` in it (which should at some point be changed to int64_t), it was tripping this check on certain platforms * Some more minor tightening. ready to ship! * use the big_int_type for python2 support Co-authored-by: ssteinbach <[email protected]>
1 parent e8b6d6c commit 8a7bd25

File tree

15 files changed

+580
-29
lines changed

15 files changed

+580
-29
lines changed

docs/tutorials/otio-file-format-specification.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,20 @@ It is strongly recommended that everyone use the OpenTimelineIO library to read
1111
OpenTimelineIO files should have a `.otio` path extension. Please do not use `.json` to name OTIO files.
1212

1313
## Contents
14-
OpenTimelineIO files are serialized as JSON (http://www.json.org)
14+
OpenTimelineIO files are serialized as JSON (http://www.json.org).
15+
16+
### Number Types
17+
18+
Supported number types:
19+
20+
- integers: `int64_t` (signed 64 bit integer)
21+
- floating point numbers: `double` (IEEE754 64 bit signed floating point number)
22+
23+
In addition to the basic JSON spec, OTIO allows the following values for doubles:
24+
25+
- `NaN` (not a number)
26+
- `Inf`, `Infinity` (positive infinity)
27+
- `-Inf, -Infinity (negative infinity)
1528

1629
## Structure
1730
An OTIO file is a tree structure of nested OTIO objects. Each OTIO object is stored as a JSON dictionary with member fields,

src/opentimelineio/deserialization.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,19 @@ class JSONDecoder : public OTIO_rapidjson::BaseReaderHandler<OTIO_rapidjson::UTF
3535
}
3636
}
3737

38-
bool Null() { return store(any()); }
38+
bool Null() { return store(any()); }
3939
bool Bool(bool b) { return store(any(b)); }
40-
bool Int(int i) { return store(any(i)); }
41-
bool Uint(unsigned u) { return store(any(int(u))); }
42-
bool Int64(int64_t i) { return store(any(i)); }
43-
bool Uint64(uint64_t u) { return store(any(int64_t(u))); }
40+
41+
// coerce all integer types to int64_t...
42+
bool Int(int i) { return store(any(static_cast<int64_t>(i))); }
43+
bool Int64(int64_t i) { return store(any(static_cast<int64_t>(i))); }
44+
bool Uint(unsigned u) { return store(any(static_cast<int64_t>(u))); }
45+
bool Uint64(uint64_t u) {
46+
/// prevent an overflow
47+
return store(any(static_cast<int64_t>(u & 0x7FFFFFFFFFFFFFFF)));
48+
}
49+
50+
// ...and all floating point types to double
4451
bool Double(double d) { return store(any(d)); }
4552

4653
bool String(const char* str, OTIO_rapidjson::SizeType length, bool /* copy */) {
@@ -571,7 +578,7 @@ bool deserialize_json_from_string(std::string const& input, any* destination, Er
571578
OTIO_rapidjson::CursorStreamWrapper<decltype(ss)> csw(ss);
572579
JSONDecoder handler(std::bind(&decltype(csw)::GetLine, &csw));
573580

574-
bool status = reader.Parse(csw, handler);
581+
bool status = reader.Parse<OTIO_rapidjson::kParseNanAndInfFlag>(csw, handler);
575582
handler.finalize();
576583

577584
if (handler.has_errored(error_status)) {
@@ -613,7 +620,7 @@ bool deserialize_json_from_file(std::string const& file_name, any* destination,
613620
OTIO_rapidjson::CursorStreamWrapper<decltype(fs)> csw(fs);
614621
JSONDecoder handler(std::bind(&decltype(csw)::GetLine, &csw));
615622

616-
bool status = reader.Parse(csw, handler);
623+
bool status = reader.Parse<OTIO_rapidjson::kParseNanAndInfFlag>(csw, handler);
617624
fclose(fp);
618625

619626
handler.finalize();

src/opentimelineio/imageSequenceReference.cpp

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,22 @@ ImageSequenceReference::ImageSequenceReference(std::string const& target_url_bas
121121
}
122122

123123
bool ImageSequenceReference::read_from(Reader& reader) {
124+
125+
int64_t start_frame_value = 0;
126+
int64_t frame_step_value = 0;
127+
int64_t frame_zero_padding_value = 0;
128+
124129
auto result = reader.read("target_url_base", &_target_url_base) &&
125130
reader.read("name_prefix", &_name_prefix) &&
126131
reader.read("name_suffix", &_name_suffix) &&
127-
reader.read("start_frame", &_start_frame) &&
128-
reader.read("frame_step", &_frame_step) &&
132+
reader.read("start_frame", &start_frame_value) &&
133+
reader.read("frame_step", &frame_step_value) &&
129134
reader.read("rate", &_rate) &&
130-
reader.read("frame_zero_padding", &_frame_zero_padding);
135+
reader.read("frame_zero_padding", &frame_zero_padding_value);
136+
137+
_start_frame = static_cast<int>(start_frame_value);
138+
_frame_step = static_cast<int>(frame_step_value);
139+
_frame_zero_padding = static_cast<int>(frame_zero_padding_value);
131140

132141
std::string missing_frame_policy_value;
133142
result && reader.read("missing_frame_policy", &missing_frame_policy_value);
@@ -156,14 +165,18 @@ ImageSequenceReference::ImageSequenceReference(std::string const& target_url_bas
156165
}
157166

158167
void ImageSequenceReference::write_to(Writer& writer) const {
168+
int64_t start_frame_value = static_cast<int64_t>(_start_frame);
169+
int64_t frame_step_value = static_cast<int64_t>(_frame_step);
170+
int64_t frame_zero_padding_value = static_cast<int64_t>(_frame_zero_padding);
171+
159172
Parent::write_to(writer);
160173
writer.write("target_url_base", _target_url_base);
161174
writer.write("name_prefix", _name_prefix);
162175
writer.write("name_suffix", _name_suffix);
163-
writer.write("start_frame", _start_frame);
164-
writer.write("frame_step", _frame_step);
176+
writer.write("start_frame", start_frame_value);
177+
writer.write("frame_step", frame_step_value);
165178
writer.write("rate", _rate);
166-
writer.write("frame_zero_padding", _frame_zero_padding);
179+
writer.write("frame_zero_padding", frame_zero_padding_value);
167180

168181
std::string missing_frame_policy_value;
169182
switch (_missing_frame_policy)

src/opentimelineio/safely_typed_any.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ any create_safely_typed_any(int64_t&& value) {
1414
return any(value);
1515
}
1616

17+
any create_safely_typed_any(uint64_t&& value) {
18+
return any(value);
19+
}
20+
1721
any create_safely_typed_any(double&& value) {
1822
return any(value);
1923
}
@@ -59,6 +63,10 @@ int64_t safely_cast_int64_any(any const& a) {
5963
return any_cast<int64_t>(a);
6064
}
6165

66+
uint64_t safely_cast_uint64_any(any const& a) {
67+
return any_cast<uint64_t>(a);
68+
}
69+
6270
double safely_cast_double_any(any const& a) {
6371
return any_cast<double>(a);
6472
}

src/opentimelineio/safely_typed_any.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ namespace opentimelineio { namespace OPENTIMELINEIO_VERSION {
2727
any create_safely_typed_any(bool&&);
2828
any create_safely_typed_any(int&&);
2929
any create_safely_typed_any(int64_t&&);
30+
any create_safely_typed_any(uint64_t&&);
3031
any create_safely_typed_any(double&&);
3132
any create_safely_typed_any(std::string&&);
3233
any create_safely_typed_any(RationalTime&&);
@@ -39,6 +40,7 @@ any create_safely_typed_any(SerializableObject*);
3940
bool safely_cast_bool_any(any const& a);
4041
int safely_cast_int_any(any const& a);
4142
int64_t safely_cast_int64_any(any const& a);
43+
uint64_t safely_cast_uint64_any(any const& a);
4244
double safely_cast_double_any(any const& a);
4345
std::string safely_cast_string_any(any const& a);
4446
RationalTime safely_cast_rational_time_any(any const& a);

src/opentimelineio/serializableObject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ class SerializableObject {
319319
static bool write_root(any const& value, class Encoder& encoder, ErrorStatus* error_status);
320320

321321
void write(std::string const& key, bool value);
322-
void write(std::string const& key, int value);
322+
void write(std::string const& key, int64_t value);
323323
void write(std::string const& key, double value);
324324
void write(std::string const& key, std::string const& value);
325325
void write(std::string const& key, RationalTime value);

src/opentimelineio/serialization.cpp

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class Encoder {
4747
virtual void write_value(bool value) = 0;
4848
virtual void write_value(int value) = 0;
4949
virtual void write_value(int64_t value) = 0;
50+
virtual void write_value(uint64_t value) = 0;
5051
virtual void write_value(double value) = 0;
5152
virtual void write_value(std::string const& value) = 0;
5253
virtual void write_value(class RationalTime const& value) = 0;
@@ -128,6 +129,10 @@ class CloningEncoder : public Encoder {
128129
_store(any(value));
129130
}
130131

132+
void write_value(uint64_t value) {
133+
_store(any(value));
134+
}
135+
131136
void write_value(std::string const& value) {
132137
_store(any(value));
133138
}
@@ -282,6 +287,10 @@ class JSONEncoder : public Encoder {
282287
_writer.Int64(value);
283288
}
284289

290+
void write_value(uint64_t value) {
291+
_writer.Uint64(value);
292+
}
293+
285294
void write_value(std::string const& value) {
286295
_writer.String(value.c_str());
287296
}
@@ -392,7 +401,6 @@ void SerializableObject::Writer::_build_dispatch_tables() {
392401
auto& wt = _write_dispatch_table;
393402
wt[&typeid(void)] = [this](any const&) { _encoder.write_null_value(); };
394403
wt[&typeid(bool)] = [this](any const& value) { _encoder.write_value(any_cast<bool>(value)); };
395-
wt[&typeid(int)] = [this](any const& value) { _encoder.write_value(any_cast<int>(value)); };
396404
wt[&typeid(int64_t)] = [this](any const& value) { _encoder.write_value(any_cast<int64_t>(value)); };
397405
wt[&typeid(double)] = [this](any const& value) { _encoder.write_value(any_cast<double>(value)); };
398406
wt[&typeid(std::string)] = [this](any const& value) { _encoder.write_value(any_cast<std::string const&>(value)); };
@@ -425,7 +433,6 @@ void SerializableObject::Writer::_build_dispatch_tables() {
425433
auto& et = _equality_dispatch_table;
426434
et[&typeid(void)] = &_simple_any_comparison<void>;
427435
et[&typeid(bool)] = &_simple_any_comparison<bool>;
428-
et[&typeid(int)] = &_simple_any_comparison<int>;
429436
et[&typeid(int64_t)] = &_simple_any_comparison<int64_t>;
430437
et[&typeid(double)] = &_simple_any_comparison<double>;
431438
et[&typeid(std::string)] = &_simple_any_comparison<std::string>;
@@ -509,7 +516,7 @@ void SerializableObject::Writer::write(std::string const& key, bool value) {
509516
_encoder.write_value(value);
510517
}
511518

512-
void SerializableObject::Writer::write(std::string const& key, int value) {
519+
void SerializableObject::Writer::write(std::string const& key, int64_t value) {
513520
_encoder_write_key(key);
514521
_encoder.write_value(value);
515522
}
@@ -724,15 +731,28 @@ std::string serialize_json_to_string(any const& value, ErrorStatus* error_status
724731
OTIO_rapidjson::StringBuffer s;
725732

726733
if (indent < 0) {
727-
OTIO_rapidjson::Writer<decltype(s)> json_writer(s);
734+
OTIO_rapidjson::Writer<
735+
decltype(s),
736+
OTIO_rapidjson::UTF8<>,
737+
OTIO_rapidjson::UTF8<>,
738+
OTIO_rapidjson::CrtAllocator,
739+
OTIO_rapidjson::kWriteNanAndInfFlag
740+
> json_writer(s);
728741
JSONEncoder<decltype(json_writer)> json_encoder(json_writer);
729742

730743
if (!SerializableObject::Writer::write_root(value, json_encoder, error_status)) {
731744
return std::string();
732745
}
733746
}
734747
else {
735-
OTIO_rapidjson::PrettyWriter<decltype(s)> json_writer(s);
748+
OTIO_rapidjson::PrettyWriter<
749+
decltype(s),
750+
OTIO_rapidjson::UTF8<>,
751+
OTIO_rapidjson::UTF8<>,
752+
OTIO_rapidjson::CrtAllocator,
753+
OTIO_rapidjson::kWriteNanAndInfFlag
754+
> json_writer(s);
755+
736756
JSONEncoder<decltype(json_writer)> json_encoder(json_writer);
737757

738758
json_writer.SetIndent(' ', indent);
@@ -756,12 +776,24 @@ bool serialize_json_to_file(any const& value, std::string const& file_name,
756776
bool status;
757777

758778
if (indent < 0) {
759-
OTIO_rapidjson::Writer<decltype(osw)> json_writer(osw);
779+
OTIO_rapidjson::Writer<
780+
decltype(osw),
781+
OTIO_rapidjson::UTF8<>,
782+
OTIO_rapidjson::UTF8<>,
783+
OTIO_rapidjson::CrtAllocator,
784+
OTIO_rapidjson::kWriteNanAndInfFlag
785+
> json_writer(osw);
760786
JSONEncoder<decltype(json_writer)> json_encoder(json_writer);
761787
status = SerializableObject::Writer::write_root(value, json_encoder, error_status);
762788
}
763789
else {
764-
OTIO_rapidjson::PrettyWriter<decltype(osw)> json_writer(osw);
790+
OTIO_rapidjson::PrettyWriter<
791+
decltype(osw),
792+
OTIO_rapidjson::UTF8<>,
793+
OTIO_rapidjson::UTF8<>,
794+
OTIO_rapidjson::CrtAllocator,
795+
OTIO_rapidjson::kWriteNanAndInfFlag
796+
> json_writer(osw);
765797
JSONEncoder<decltype(json_writer)> json_encoder(json_writer);
766798

767799
json_writer.SetIndent(' ', indent);

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,18 +94,29 @@ PYBIND11_MODULE(_otio, m) {
9494
}, "filename"_a);
9595

9696
py::class_<PyAny>(m, "PyAny")
97-
.def(py::init([](bool b) { return new PyAny(b); }))
98-
.def(py::init([](int i) { return new PyAny(i); }))
99-
.def(py::init([](int64_t i) { return new PyAny(i); }))
100-
.def(py::init([](double d) { return new PyAny(d); }))
97+
// explicitly map python bool, int and double classes so that they
98+
// do NOT accidentally cast in valid values
99+
.def(py::init([](py::bool_ b) {
100+
bool result = b.cast<bool>();
101+
return new PyAny(result);
102+
}))
103+
.def(py::init([](py::int_ i) {
104+
int64_t result = i.cast<int64_t>();
105+
return new PyAny(result);
106+
}))
107+
.def(py::init([](py::float_ d) {
108+
double result = d.cast<double>();
109+
return new PyAny(result);
110+
}))
101111
.def(py::init([](std::string s) { return new PyAny(s); }))
102112
.def(py::init([](py::none) { return new PyAny(); }))
103113
.def(py::init([](SerializableObject* s) { return new PyAny(s); }))
104114
.def(py::init([](RationalTime rt) { return new PyAny(rt); }))
105115
.def(py::init([](TimeRange tr) { return new PyAny(tr); }))
106116
.def(py::init([](TimeTransform tt) { return new PyAny(tt); }))
107117
.def(py::init([](AnyVectorProxy* p) { return new PyAny(p->fetch_any_vector()); }))
108-
.def(py::init([](AnyDictionaryProxy* p) { return new PyAny(p->fetch_any_dictionary()); }));
118+
.def(py::init([](AnyDictionaryProxy* p) { return new PyAny(p->fetch_any_dictionary()); }))
119+
;
109120

110121
m.def("register_serializable_object_type", &register_python_type,
111122
"class_object"_a, "schema_name"_a, "schema_version"_a);

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "opentimelineio/serializableObject.h"
55
#include "opentimelineio/serializableObjectWithMetadata.h"
66
#include "opentimelineio/serializableCollection.h"
7+
#include "opentimelineio/timeline.h"
78
#include "otio_utils.h"
89

910
namespace py = pybind11;
@@ -130,6 +131,26 @@ void otio_xyzzy(std::string msg) {
130131
/* used as a debugger breakpoint */
131132
}
132133

134+
/// test the behavior of big integers in OTIO
135+
bool test_big_uint() {
136+
int64_t some_int = 4;
137+
uint64_t number_base = INT64_MAX;
138+
uint64_t giant_number = number_base + some_int;
139+
140+
SerializableObjectWithMetadata* so = new SerializableObjectWithMetadata();
141+
142+
so->metadata()["giant_number"] = giant_number;
143+
144+
bool result = true;
145+
146+
if (any_cast<uint64_t>(so->metadata()["giant_number"]) != giant_number) {
147+
return false;
148+
}
149+
150+
so->possibly_delete();
151+
return true;
152+
}
153+
133154
void otio_tests_bindings(py::module m) {
134155
TypeRegistry& r = TypeRegistry::instance();
135156
r.register_type<TestObject>();
@@ -146,4 +167,5 @@ void otio_tests_bindings(py::module m) {
146167
test.def("bash_retainers2", &test_bash_retainers2);
147168
test.def("gil_scoping", &test_gil_scoping);
148169
test.def("xyzzy", &otio_xyzzy);
170+
test.def("test_big_uint", &test_big_uint);
149171
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,19 @@ py::object plain_int(int64_t i) {
3939
return py::reinterpret_steal<py::object>(p);
4040
}
4141

42+
py::object plain_uint(uint64_t i) {
43+
PyObject *p = PyLong_FromUnsignedLongLong(i);
44+
return py::reinterpret_steal<py::object>(p);
45+
}
46+
4247
void _build_any_to_py_dispatch_table() {
4348
auto& t = _py_cast_dispatch_table;
4449

4550
t[&typeid(void)] = [](any const& /* a */, bool) { return py::none(); };
4651
t[&typeid(bool)] = [](any const& a, bool) { return py::cast(safely_cast_bool_any(a)); };
4752
t[&typeid(int)] = [](any const& a, bool) { return plain_int(safely_cast_int_any(a)); };
4853
t[&typeid(int64_t)] = [](any const& a, bool) { return plain_int(safely_cast_int64_any(a)); };
54+
t[&typeid(uint64_t)] = [](any const& a, bool) { return plain_uint(safely_cast_uint64_any(a)); };
4955
t[&typeid(double)] = [](any const& a, bool) { return py::cast(safely_cast_double_any(a)); };
5056
t[&typeid(std::string)] = [](any const& a, bool) { return py::cast(safely_cast_string_any(a)); };
5157
t[&typeid(RationalTime)] = [](any const& a, bool) { return py::cast(safely_cast_rational_time_any(a)); };

0 commit comments

Comments
 (0)