Skip to content

Commit e8594b8

Browse files
authored
Fix the integer overflow when converting PTS to rational type (#1185)
`AVRational` stores the data as a tuple of int (`int32_t`). `AVPacket::pts` stores PTS as `int64_t`. There are cases where the `AVPacket::pts` value is larger than`INT32_MAX`. In such case, we cannot simply cast the variable type. We need to perform reduction, otherwise it causes interger overflow. The test will be added in a subsequent PR.
1 parent f9c75ec commit e8594b8

2 files changed

Lines changed: 48 additions & 4 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
#pragma once
10+
11+
#include "libspdl/core/detail/logging.h"
12+
13+
extern "C" {
14+
#include <libavutil/rational.h>
15+
}
16+
17+
namespace spdl::core::detail {
18+
// Check if a given AVRational value falls within a specified window [start,
19+
// end). Returns true if start <= val < end (half-open interval).
20+
inline bool is_within_window(
21+
const AVRational& val,
22+
const AVRational& start,
23+
const AVRational& end) {
24+
return (av_cmp_q(start, val) <= 0) && (av_cmp_q(val, end) < 0);
25+
}
26+
27+
inline AVRational to_rational(int64_t val, const AVRational time_base) {
28+
AVRational ret;
29+
if (av_reduce(
30+
&ret.num, &ret.den, val * time_base.num, time_base.den, INT32_MAX)) {
31+
// Warn once that reduced PTS may be inexact due to rational reduction
32+
// constraints.
33+
static bool warned_inexact_pts = false;
34+
if (!warned_inexact_pts) {
35+
LOG(WARNING) << "PTS estimation was not exact during rational reduction; "
36+
"timestamps might be slightly inaccurate.";
37+
warned_inexact_pts = true;
38+
}
39+
}
40+
return ret;
41+
}
42+
} // namespace spdl::core::detail

src/libspdl/core/packets.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <libspdl/core/packets.h>
1010

1111
#include "libspdl/core/detail/ffmpeg/logging.h"
12+
#include "libspdl/core/detail/ffmpeg/rational_utils.h"
1213
#include "libspdl/core/detail/tracing.h"
1314

1415
#include <fmt/core.h>
@@ -313,11 +314,12 @@ std::vector<double> get_timestamps(const Packets<media>& packets, bool raw) {
313314

314315
auto [start, end] = packets.timestamp.value_or(NO_WINDOW);
315316

317+
auto s = av_d2q(start, AV_TIME_BASE);
318+
auto e = av_d2q(end, AV_TIME_BASE);
316319
for (const auto& pkt : pkts) {
317-
auto pts = AVRational{static_cast<int>(pkt->pts), 1};
318-
auto ts = av_q2d(av_mul_q(pts, packets.time_base));
319-
if (raw || (start <= ts && ts < end)) {
320-
ret.emplace_back(ts);
320+
AVRational pts = detail::to_rational(pkt->pts, packets.time_base);
321+
if (raw || detail::is_within_window(pts, s, e)) {
322+
ret.emplace_back(av_q2d(pts));
321323
}
322324
}
323325
if (!raw) {

0 commit comments

Comments
 (0)