Skip to content

Commit 0e24ef3

Browse files
kongchen1992facebook-github-bot
kongchen1992
authored and
facebook-github-bot
committed
{BugFix} Core - Restore Gen2 time sync and load TimeSync records on demand
Summary: Explanation: Every Aria Gen 2 recording currently loses its cross-device time sync. On an affected file `supports_time_domain()` reports false for SubGhz, Utc, TimeCode and TicSync alike, `get_first_time_ns()` and `get_index_by_time_ns()` throw for those domains, and every `SensorData` comes back with an empty sync-timestamp map. Reproduced on the published Gen 2 sample pair: the SubGHz receiver recording fails with `Time domain SubGhz not supported for the stream RGB Camera Class #1`. Two defects combined: 1. `determineTimeSyncModeFromJson` decided the Gen 2 mode from `metadata.recording.subghz_mode` alone. Recordings predating the OS change that introduced that field carry no such hint, so a genuine SubGHz receiver resolved to `NotEnabled`. The Gen 2 branch also never looked for UTC, so a recording with SubGHz simply switched off -- the common case -- resolved to `NotEnabled` too, despite carrying a perfectly usable UTC stream. 2. `TimeSyncMapper` treated `NotEnabled` as "these streams are unusable" and skipped its preload, returning before `timesyncPlayers_` was even assigned. That left `supportsMode()` false for every mode at once, so a single SubGHz-only signal silently disabled all four time domains. The Gen 2 branch now falls back to the streams the file actually contains, the way Gen 1 already did, and keeps the metadata field for the one case the streams cannot express: a broadcaster is the clock reference, so it logs no mapping stream of its own. Removing the skip alone would restore correctness at an unacceptable price: opening the multi-hour Manifold recording from T272680899 goes from 27s to 424s, because `TimeSyncMapper` walked every record of every TimeSync stream up front. That is the worst possible access order for a remote file. TimeSync records are interleaved with sensor data across the whole recording, so reading them cold pulls -- and then evicts -- essentially every cache block before any sensor read can reuse one. `TimeSyncMapper` now reads no record payloads at construction. It keeps each stream's record index, which is already in memory, and fetches samples the first time a conversion needs them, locating the bracketing pair through the index rather than by scanning. A sequential sweep then picks up each TimeSync record from the block it is already reading, and a cursor keeps the common case at O(1) instead of a binary search. Also in this change, all consequences of the above: - `MetadataTimeSyncMode::SubGhz` and `::Utc` were never registered with pybind, so Python printed `MetadataTimeSyncMode.???` and `== MetadataTimeSyncMode.SubGhz` raised `AttributeError`. Both values, and the two missing `timeSyncModeStr()` cases, are added. - `convertFrom*` took `auto` rather than `const auto&` and so copied the entire sample vector on every call. Harmless while the vector was always empty, ruinous the moment it is not. - `convertFromSyncTimeToDeviceTimeNs` called `front()` without the empty-vector guard its counterpart had. - `recordInfoTimeNs_` was written and never read; it becomes the index timeline the on-demand search runs on. Reproducibility: Open any Gen 2 recording and call `supports_time_domain(stream_id, TimeDomain.UTC)`: false before this change, true after, on files that carry a UTC stream. The log line `Skipping TimeSyncMapper preload: file metadata reports time-sync NotEnabled (N TimeSync stream(s) present but unused)` marks each affected open. Reviewed By: ryanfrawley Differential Revision: D115652745
1 parent ff55f96 commit 0e24ef3

7 files changed

Lines changed: 472 additions & 116 deletions

File tree

‎core/data_provider/RecordReaderInterface.cpp‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,30 @@ MetadataTimeSyncMode determineTimeSyncModeFromJson(
6363
return MetadataTimeSyncMode::NotEnabled;
6464
}
6565
if (deviceVersion == calibration::DeviceVersion::Gen2) {
66-
// The OS writes `recording.subghz_mode` only when SubGHz is configured
67-
// (broadcaster or receiver); the field is absent otherwise.
68-
if (metadataJson.contains("recording") && metadataJson["recording"].is_object() &&
69-
metadataJson["recording"].contains("subghz_mode")) {
66+
// Recordings made after the OS started writing this field carry it whenever
67+
// SubGHz was configured. It is the only way to recognise a broadcaster: the
68+
// broadcaster is the clock reference, so it logs no mapping stream of its own.
69+
// Only the two role values count -- a value meaning "off", or a spelling this
70+
// code does not know, falls through to the streams below, which stay
71+
// authoritative for receivers either way.
72+
if (metadataJson.contains("recording") && metadataJson["recording"].is_object()) {
73+
const auto& recording = metadataJson["recording"];
74+
const auto subghzModeIt = recording.find("subghz_mode");
75+
if (subghzModeIt != recording.end() && subghzModeIt->is_string()) {
76+
const auto subghzMode = subghzModeIt->get<std::string>();
77+
if (subghzMode == "receiver" || subghzMode == "broadcaster") {
78+
return MetadataTimeSyncMode::SubGhz;
79+
}
80+
}
81+
}
82+
// Older recordings predate the field, and SubGHz is not the only sync a Gen 2
83+
// file can carry, so fall back to the streams present -- as Gen 1 already does.
84+
if (timesyncPlayers.count(TimeSyncMode::SUBGHZ) != 0) {
7085
return MetadataTimeSyncMode::SubGhz;
7186
}
87+
if (timesyncPlayers.count(TimeSyncMode::UTC) != 0) {
88+
return MetadataTimeSyncMode::Utc;
89+
}
7290
return MetadataTimeSyncMode::NotEnabled;
7391
}
7492
return MetadataTimeSyncMode::NotEnabled;

‎core/data_provider/TimeSyncMapper.cpp‎

Lines changed: 204 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -24,87 +24,200 @@
2424

2525
namespace projectaria::tools::data_provider {
2626

27+
namespace {
28+
/// Linear interpolation between two clock samples, expressed as a ratio so both
29+
/// conversion directions share one rounding behaviour.
30+
int64_t
31+
interpolate(int64_t fromLeft, int64_t fromRight, int64_t toLeft, int64_t toRight, int64_t from) {
32+
if (fromRight == fromLeft) {
33+
return toLeft;
34+
}
35+
const double ratioRight =
36+
static_cast<double>(from - fromLeft) / static_cast<double>(fromRight - fromLeft);
37+
const double ratioLeft = 1 - ratioRight;
38+
return static_cast<int64_t>(
39+
ratioLeft * static_cast<double>(toLeft) + ratioRight * static_cast<double>(toRight));
40+
}
41+
} // namespace
42+
2743
TimeSyncMapper::TimeSyncMapper(
2844
const std::shared_ptr<vrs::MultiRecordFileReader>& reader,
29-
const std::map<TimeSyncMode, std::shared_ptr<TimeSyncPlayer>>& timesyncPlayers,
30-
const MetadataTimeSyncMode metadataTimeSyncMode) {
31-
if (timesyncPlayers.empty()) {
32-
return;
33-
}
34-
if (metadataTimeSyncMode == MetadataTimeSyncMode::NotEnabled) {
35-
// The file's metadata says no cross-device time sync was enabled during
36-
// recording. Even if TimeSync streams are physically present, their records
37-
// are unusable — preloading them costs one HTTP RTT per record on remote
38-
// file systems and the data is discarded by the conversion APIs anyway.
39-
XR_LOGI(
40-
"Skipping TimeSyncMapper preload: file metadata reports time-sync NotEnabled "
41-
"({} TimeSync stream(s) present but unused).",
42-
timesyncPlayers.size());
43-
return;
44-
}
45-
timesyncPlayers_ = timesyncPlayers;
45+
const std::map<TimeSyncMode, std::shared_ptr<TimeSyncPlayer>>& timesyncPlayers)
46+
: reader_(reader) {
47+
const std::scoped_lock lock(mutex_);
4648
for (const auto& [mode, player] : timesyncPlayers) {
47-
vrs::StreamId streamId = player->getStreamId();
48-
int numTimeCode = reader->getRecordCount(streamId, vrs::Record::Type::DATA);
49-
recordInfoTimeNs_[mode].reserve(numTimeCode);
50-
timeSyncData_[mode].reserve(numTimeCode);
51-
timeSyncModes_.push_back(mode);
52-
53-
for (int index = 0; index < numTimeCode; ++index) {
49+
const vrs::StreamId streamId = player->getStreamId();
50+
// A stream can legitimately carry zero data records -- the UTC stream ticks
51+
// once a minute, so a short recording has none. Such a mode is still
52+
// registered, and reported as supported, to keep the long-standing contract;
53+
// its conversions return -1.
54+
const int numRecords = reader->getRecordCount(streamId, vrs::Record::Type::DATA);
55+
ModeData modeData;
56+
modeData.player = player;
57+
modeData.samples.reserve(numRecords);
58+
modeData.indexTimeNs.reserve(numRecords);
59+
for (int index = 0; index < numRecords; ++index) {
5460
const vrs::IndexRecord::RecordInfo* recordInfo =
5561
reader->getRecord(streamId, vrs::Record::Type::DATA, static_cast<uint32_t>(index));
56-
checkAndThrow(
57-
recordInfo, fmt::format("getRecord failed for {}, index {}", streamId.getName(), index));
58-
const int errorCode = reader->readRecord(*recordInfo);
59-
if (errorCode != 0) {
60-
XR_LOGE(
61-
"Fail to read record {} from streamId {} with code {}",
62-
index,
63-
streamId.getNumericName(),
64-
errorCode);
62+
if (recordInfo == nullptr) {
6563
continue;
6664
}
67-
recordInfoTimeNs_[mode].push_back(static_cast<int64_t>(recordInfo->timestamp * 1e9));
68-
timeSyncData_[mode].push_back(player->getDataRecord());
65+
modeData.samples.push_back(Sample{.recordInfo = recordInfo});
66+
modeData.indexTimeNs.push_back(static_cast<int64_t>(recordInfo->timestamp * 1e9));
67+
}
68+
modes_.emplace(mode, std::move(modeData));
69+
timeSyncModes_.push_back(mode);
70+
}
71+
}
72+
73+
const TimeSyncData* TimeSyncMapper::sampleAt(ModeData& modeData, const size_t index) const {
74+
if (index >= modeData.samples.size()) {
75+
return nullptr;
76+
}
77+
Sample& sample = modeData.samples[index];
78+
if (sample.recordInfo == nullptr) {
79+
return nullptr;
80+
}
81+
if (!sample.loaded) {
82+
const int errorCode = reader_->readRecord(*sample.recordInfo);
83+
if (errorCode == 0) {
84+
sample.data = modeData.player->getDataRecord();
85+
} else {
86+
XR_LOGE(
87+
"Fail to read TimeSync record {} from streamId {} with code {}",
88+
index,
89+
modeData.player->getStreamId().getNumericName(),
90+
errorCode);
91+
sample.readFailed = true;
92+
}
93+
sample.loaded = true;
94+
}
95+
// A failed read leaves a zeroed sample behind. Handing it out would break the
96+
// ordering every search here relies on, so conversions fail instead.
97+
return sample.readFailed ? nullptr : &sample.data;
98+
}
99+
100+
std::optional<size_t> TimeSyncMapper::findBracketByDeviceTime(
101+
ModeData& modeData,
102+
const int64_t deviceTimeNs) const {
103+
const size_t last = modeData.samples.size() - 1;
104+
// A sequential sweep keeps landing in the bracket it used last, or the next
105+
// one; check those before paying for a search.
106+
if (modeData.cursor < last) {
107+
const TimeSyncData* left = sampleAt(modeData, modeData.cursor);
108+
const TimeSyncData* right = sampleAt(modeData, modeData.cursor + 1);
109+
if (left == nullptr || right == nullptr) {
110+
return std::nullopt;
111+
}
112+
if (left->monotonicTimestampNs <= deviceTimeNs && deviceTimeNs <= right->monotonicTimestampNs) {
113+
return modeData.cursor;
114+
}
115+
}
116+
// The index timestamps track the samples' own monotonic clock closely enough
117+
// to land on or beside the right bracket, but they are not the same numbers,
118+
// so the candidate is confirmed against the samples below.
119+
const auto it = std::ranges::upper_bound(modeData.indexTimeNs, deviceTimeNs);
120+
auto index = static_cast<size_t>(std::distance(modeData.indexTimeNs.begin(), it));
121+
index = index == 0 ? 0 : index - 1;
122+
while (index > 0) {
123+
const TimeSyncData* sample = sampleAt(modeData, index);
124+
if (sample == nullptr) {
125+
return std::nullopt;
126+
}
127+
if (sample->monotonicTimestampNs <= deviceTimeNs) {
128+
break;
129+
}
130+
--index;
131+
}
132+
while (index < last) {
133+
const TimeSyncData* next = sampleAt(modeData, index + 1);
134+
if (next == nullptr) {
135+
return std::nullopt;
136+
}
137+
if (next->monotonicTimestampNs > deviceTimeNs) {
138+
break;
139+
}
140+
++index;
141+
}
142+
modeData.cursor = index;
143+
return index;
144+
}
145+
146+
std::optional<size_t> TimeSyncMapper::findBracketBySyncTime(
147+
ModeData& modeData,
148+
const int64_t syncTimeNs) const {
149+
const size_t last = modeData.samples.size() - 1;
150+
if (modeData.cursor < last) {
151+
const TimeSyncData* left = sampleAt(modeData, modeData.cursor);
152+
const TimeSyncData* right = sampleAt(modeData, modeData.cursor + 1);
153+
if (left == nullptr || right == nullptr) {
154+
return std::nullopt;
155+
}
156+
if (left->realTimestampNs <= syncTimeNs && syncTimeNs <= right->realTimestampNs) {
157+
return modeData.cursor;
69158
}
70-
recordInfoTimeNs_[mode].shrink_to_fit();
71-
timeSyncData_[mode].shrink_to_fit();
72159
}
160+
// The record index carries no sync-clock timestamps, so the search reads the
161+
// samples it probes -- O(log n) records rather than the whole stream.
162+
size_t low = 0;
163+
size_t high = last;
164+
while (low < high) {
165+
const size_t mid = low + (high - low + 1) / 2;
166+
const TimeSyncData* sample = sampleAt(modeData, mid);
167+
if (sample == nullptr) {
168+
return std::nullopt;
169+
}
170+
if (sample->realTimestampNs <= syncTimeNs) {
171+
low = mid;
172+
} else {
173+
high = mid - 1;
174+
}
175+
}
176+
modeData.cursor = low;
177+
return low;
73178
}
74179

75180
int64_t TimeSyncMapper::convertFromSyncTimeToDeviceTimeNs(
76-
const int64_t timecodeTimeNs,
181+
const int64_t syncTimeNs,
77182
const TimeSyncMode mode) const {
78183
if (!supportsMode(mode)) {
79184
return -1;
80185
}
81-
auto timecodeData = timeSyncData_.at(mode);
186+
const std::scoped_lock lock(mutex_);
187+
const auto modeIt = modes_.find(mode);
188+
if (modeIt == modes_.end() || modeIt->second.samples.empty()) {
189+
return -1;
190+
}
191+
ModeData& modeData = modeIt->second;
192+
const size_t last = modeData.samples.size() - 1;
82193

83-
if (timecodeTimeNs <= timecodeData.front().realTimestampNs) {
84-
return timecodeData.front().monotonicTimestampNs - timecodeData.front().realTimestampNs +
85-
timecodeTimeNs;
194+
const TimeSyncData* front = sampleAt(modeData, 0);
195+
const TimeSyncData* back = sampleAt(modeData, last);
196+
if (front == nullptr || back == nullptr) {
197+
return -1;
86198
}
87-
if (timecodeTimeNs >= timecodeData.back().realTimestampNs) {
88-
return timecodeData.back().monotonicTimestampNs - timecodeData.back().realTimestampNs +
89-
timecodeTimeNs;
199+
if (syncTimeNs <= front->realTimestampNs) {
200+
return front->monotonicTimestampNs - front->realTimestampNs + syncTimeNs;
201+
}
202+
if (syncTimeNs >= back->realTimestampNs) {
203+
return back->monotonicTimestampNs - back->realTimestampNs + syncTimeNs;
90204
}
91205

92-
TimeSyncData query;
93-
query.realTimestampNs = timecodeTimeNs;
94-
auto timecodeIter = std::ranges::upper_bound( // finds first timestamp > query
95-
timecodeData,
96-
query,
97-
[&](const auto& lhs, const auto& rhs) { return lhs.realTimestampNs < rhs.realTimestampNs; });
98-
auto lastTimeCodeIter = timecodeIter - 1;
99-
int64_t monoTimeRight = timecodeIter->monotonicTimestampNs;
100-
int64_t monoTimeLeft = lastTimeCodeIter->monotonicTimestampNs;
101-
int64_t realTimeRight = timecodeIter->realTimestampNs;
102-
int64_t realTimeLeft = lastTimeCodeIter->realTimestampNs;
103-
104-
double ratioRight = double(timecodeTimeNs - realTimeLeft) / double(realTimeRight - realTimeLeft);
105-
double ratioLeft = 1 - ratioRight;
106-
107-
return static_cast<int64_t>(ratioLeft * monoTimeLeft + ratioRight * monoTimeRight);
206+
const std::optional<size_t> index = findBracketBySyncTime(modeData, syncTimeNs);
207+
if (!index.has_value()) {
208+
return -1;
209+
}
210+
const TimeSyncData* left = sampleAt(modeData, *index);
211+
const TimeSyncData* right = sampleAt(modeData, std::min(*index + 1, last));
212+
if (left == nullptr || right == nullptr) {
213+
return -1;
214+
}
215+
return interpolate(
216+
left->realTimestampNs,
217+
right->realTimestampNs,
218+
left->monotonicTimestampNs,
219+
right->monotonicTimestampNs,
220+
syncTimeNs);
108221
}
109222

110223
int64_t TimeSyncMapper::convertFromDeviceTimeToSyncTimeNs(
@@ -113,40 +226,41 @@ int64_t TimeSyncMapper::convertFromDeviceTimeToSyncTimeNs(
113226
if (!supportsMode(mode)) {
114227
return -1;
115228
}
116-
auto timecodeData = timeSyncData_.at(mode);
117-
118-
// Skip if this stream doesn't have any timecode data
119-
if (timecodeData.empty()) {
229+
const std::scoped_lock lock(mutex_);
230+
const auto modeIt = modes_.find(mode);
231+
if (modeIt == modes_.end() || modeIt->second.samples.empty()) {
120232
return -1;
121233
}
234+
ModeData& modeData = modeIt->second;
235+
const size_t last = modeData.samples.size() - 1;
122236

123-
if (deviceTimeNs <= timecodeData.front().monotonicTimestampNs) {
124-
return timecodeData.front().realTimestampNs - timecodeData.front().monotonicTimestampNs +
125-
deviceTimeNs;
237+
const TimeSyncData* front = sampleAt(modeData, 0);
238+
const TimeSyncData* back = sampleAt(modeData, last);
239+
if (front == nullptr || back == nullptr) {
240+
return -1;
126241
}
127-
if (deviceTimeNs >= timecodeData.back().monotonicTimestampNs) {
128-
return timecodeData.back().realTimestampNs - timecodeData.back().monotonicTimestampNs +
129-
deviceTimeNs;
242+
if (deviceTimeNs <= front->monotonicTimestampNs) {
243+
return front->realTimestampNs - front->monotonicTimestampNs + deviceTimeNs;
244+
}
245+
if (deviceTimeNs >= back->monotonicTimestampNs) {
246+
return back->realTimestampNs - back->monotonicTimestampNs + deviceTimeNs;
130247
}
131248

132-
TimeSyncData query;
133-
query.monotonicTimestampNs = deviceTimeNs;
134-
auto timecodeIter = std::ranges::upper_bound( // finds first timestamp > query
135-
timecodeData,
136-
query,
137-
[&](const auto& lhs, const auto& rhs) {
138-
return lhs.monotonicTimestampNs < rhs.monotonicTimestampNs;
139-
});
140-
auto lastTimeCodeIter = timecodeIter - 1;
141-
int64_t monoTimeRight = timecodeIter->monotonicTimestampNs;
142-
int64_t monoTimeLeft = lastTimeCodeIter->monotonicTimestampNs;
143-
int64_t realTimeRight = timecodeIter->realTimestampNs;
144-
int64_t realTimeLeft = lastTimeCodeIter->realTimestampNs;
145-
146-
double ratioRight = double(deviceTimeNs - monoTimeLeft) / double(monoTimeRight - monoTimeLeft);
147-
double ratioLeft = 1 - ratioRight;
148-
149-
return static_cast<int64_t>(ratioLeft * realTimeLeft + ratioRight * realTimeRight);
249+
const std::optional<size_t> index = findBracketByDeviceTime(modeData, deviceTimeNs);
250+
if (!index.has_value()) {
251+
return -1;
252+
}
253+
const TimeSyncData* left = sampleAt(modeData, *index);
254+
const TimeSyncData* right = sampleAt(modeData, std::min(*index + 1, last));
255+
if (left == nullptr || right == nullptr) {
256+
return -1;
257+
}
258+
return interpolate(
259+
left->monotonicTimestampNs,
260+
right->monotonicTimestampNs,
261+
left->realTimestampNs,
262+
right->realTimestampNs,
263+
deviceTimeNs);
150264
}
151265

152266
int64_t TimeSyncMapper::convertFromTimeCodeToDeviceTimeNs(const int64_t timecodeTimeNs) const {
@@ -158,7 +272,7 @@ int64_t TimeSyncMapper::convertFromDeviceTimeToTimeCodeNs(const int64_t deviceTi
158272
}
159273

160274
bool TimeSyncMapper::supportsMode(const TimeSyncMode mode) const {
161-
return (timesyncPlayers_.find(mode) != timesyncPlayers_.end()) &&
275+
return (std::ranges::find(timeSyncModes_, mode) != timeSyncModes_.end()) &&
162276
(mode == TimeSyncMode::TIMECODE || mode == TimeSyncMode::TIC_SYNC ||
163277
mode == TimeSyncMode::SUBGHZ || mode == TimeSyncMode::UTC);
164278
}

0 commit comments

Comments
 (0)