Skip to content

Commit f984467

Browse files
committed
Fix issue with Timestamp overflow
Fix #140
1 parent 68a21be commit f984467

File tree

3 files changed

+63
-7
lines changed

3 files changed

+63
-7
lines changed

src/timestamp.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,12 @@ Timestamp::operator bool() const noexcept
8989

9090
Timestamp operator+(const Timestamp &left, const Timestamp &right) noexcept
9191
{
92-
// Use more good precision
93-
if (left.timebase() < right.timebase()) {
94-
auto ts = av_add_stable(left.timebase(), left.timestamp(),
95-
right.timebase(), right.timestamp());
92+
// av_add_stable() required that rhs value less than lhs
93+
if (left >= right) {
94+
auto const ts = av_add_stable(left.timebase(), left.timestamp(), right.timebase(), right.timestamp());
9695
return {ts, left.timebase()};
9796
} else {
98-
auto ts = av_add_stable(right.timebase(), right.timestamp(),
99-
left.timebase(), left.timestamp());
97+
auto ts = av_add_stable(right.timebase(), right.timestamp(), left.timebase(), left.timestamp());
10098
return {ts, right.timebase()};
10199
}
102100
}

tests/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ add_executable(test_executor
1414
AvDeleter.cpp
1515
Packet.cpp
1616
Format.cpp
17-
Rational.cpp)
17+
Rational.cpp
18+
Timestamp.cpp)
1819
target_link_libraries(test_executor PUBLIC Catch2::Catch2 test_main avcpp::avcpp)
1920

2021
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/../catch2/contrib")

tests/Timestamp.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#include <catch2/catch.hpp>
2+
3+
#include <vector>
4+
5+
#include "rational.h"
6+
#include "timestamp.h"
7+
8+
#ifdef _MSC_VER
9+
# pragma warning(disable : 4702) // Disable warning: unreachable code
10+
#endif
11+
12+
13+
using namespace std;
14+
15+
TEST_CASE("Core", "[Overflow]")
16+
{
17+
SECTION("Overflow operator+(a,b)")
18+
{
19+
{
20+
auto const tsLimit = std::numeric_limits<int64_t>::max() / 48000;
21+
22+
av::Timestamp t {
23+
tsLimit, av::Rational {1, 48000}
24+
};
25+
av::Timestamp inc {48000, t.timebase()}; // 1s
26+
27+
auto v1 = t;
28+
v1 += inc;
29+
30+
auto v2 = t + inc;
31+
auto v3 = inc + t;
32+
33+
CHECK(v1 == v2);
34+
CHECK(v1 == v3);
35+
}
36+
37+
#if 0
38+
av::Timestamp t1(48000, av::Rational {1, 48000}); // 1s
39+
av::Timestamp t2 = t1;
40+
av::Timestamp t3 = t1;
41+
for (int64_t i = 0; i < 4194258; ++i) {
42+
t1 = t1 + av::Timestamp {1024, t1.timebase()}; // fail case
43+
t2 += av::Timestamp {1024, t2.timebase()}; // good
44+
t3 = av::Timestamp {1024, t1.timebase()} + t3;
45+
46+
CHECK(t3 == t2);
47+
CHECK(t1 == t2);
48+
CHECK(t1.seconds() >= 1.0);
49+
50+
if (t1 != t2 || t2 != t3 || t1.seconds() < 1.0) {
51+
CHECK(i < 0); // always fail, just for report
52+
break;
53+
}
54+
}
55+
#endif
56+
}
57+
}

0 commit comments

Comments
 (0)