From bab8cdfdef025b242150fb855e210ffcadc83a30 Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Mon, 22 Apr 2024 13:34:24 +0800 Subject: [PATCH] Cabana: bug fixes (#32272) * Fix SIGSEGV due to thread race conditions after seeking * adding ID as a secondary sorting factor * fix gaps --- tools/cabana/messageswidget.cc | 12 ++++++------ tools/cabana/videowidget.cc | 3 ++- tools/replay/replay.cc | 14 ++++++-------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/tools/cabana/messageswidget.cc b/tools/cabana/messageswidget.cc index 8043b99a70..4c3efa0385 100644 --- a/tools/cabana/messageswidget.cc +++ b/tools/cabana/messageswidget.cc @@ -255,12 +255,12 @@ void MessageListModel::dbcModified() { void MessageListModel::sortItems(std::vector &items) { auto compare = [this](const auto &l, const auto &r) { switch (sort_column) { - case Column::NAME: return l.name < r.name; - case Column::SOURCE: return l.id.source < r.id.source; - case Column::ADDRESS: return l.id.address < r.id.address; - case Column::NODE: return l.node < r.node; - case Column::FREQ: return can->lastMessage(l.id).freq < can->lastMessage(r.id).freq; - case Column::COUNT: return can->lastMessage(l.id).count < can->lastMessage(r.id).count; + case Column::NAME: return std::tie(l.name, l.id) < std::tie(r.name, r.id); + case Column::SOURCE: return std::tie(l.id.source, l.id.address) < std::tie(r.id.source, r.id.address); + case Column::ADDRESS: return std::tie(l.id.address, l.id.source) < std::tie(r.id.address, r.id.source); + case Column::NODE: return std::tie(l.node, l.id) < std::tie(r.node, r.id); + case Column::FREQ: return std::tie(can->lastMessage(l.id).freq, l.id) < std::tie(can->lastMessage(r.id).freq, r.id); + case Column::COUNT: return std::tie(can->lastMessage(l.id).count, l.id) < std::tie(can->lastMessage(r.id).count, r.id); default: return false; // Default case to suppress compiler warning } }; diff --git a/tools/cabana/videowidget.cc b/tools/cabana/videowidget.cc index 261c540340..cd412f7271 100644 --- a/tools/cabana/videowidget.cc +++ b/tools/cabana/videowidget.cc @@ -151,6 +151,7 @@ QWidget *VideoWidget::createCameraWidget() { setMaximumTime(can->totalSeconds()); QObject::connect(slider, &QSlider::sliderReleased, [this]() { can->seekTo(slider->currentSecond()); }); QObject::connect(slider, &Slider::updateMaximumTime, this, &VideoWidget::setMaximumTime, Qt::QueuedConnection); + QObject::connect(can, &AbstractStream::eventsMerged, [this]() { slider->update(); }); QObject::connect(static_cast(can), &ReplayStream::qLogLoaded, slider, &Slider::parseQLog); QObject::connect(cam_widget, &CameraWidget::clicked, []() { can->pause(!can->isPaused()); }); QObject::connect(cam_widget, &CameraWidget::vipcAvailableStreamsUpdated, this, &VideoWidget::vipcAvailableStreamsUpdated); @@ -405,7 +406,7 @@ void InfoLabel::paintEvent(QPaintEvent *event) { font.setPixelSize(11); p.setFont(font); } - QRect text_rect = rect().adjusted(2, 2, -2, -2); + QRect text_rect = rect().adjusted(1, 1, -1, -1); QRect r = p.fontMetrics().boundingRect(text_rect, Qt::AlignTop | Qt::AlignHCenter | Qt::TextWordWrap, text); p.fillRect(text_rect.left(), r.top(), text_rect.width(), r.height(), color); p.drawText(text_rect, Qt::AlignTop | Qt::AlignHCenter | Qt::TextWordWrap, text); diff --git a/tools/replay/replay.cc b/tools/replay/replay.cc index e7657f3531..f5339050a8 100644 --- a/tools/replay/replay.cc +++ b/tools/replay/replay.cc @@ -93,10 +93,9 @@ void Replay::updateEvents(const std::function &update_events_function) { } void Replay::seekTo(double seconds, bool relative) { - seeking_to_seconds_ = relative ? seconds + currentSeconds() : seconds; - seeking_to_seconds_ = std::max(double(0.0), seeking_to_seconds_); - updateEvents([&]() { + seeking_to_seconds_ = relative ? seconds + currentSeconds() : seconds; + seeking_to_seconds_ = std::max(double(0.0), seeking_to_seconds_); int target_segment = (int)seeking_to_seconds_ / 60; if (segments_.count(target_segment) == 0) { rWarning("can't seek to %d s segment %d is invalid", (int)seeking_to_seconds_, target_segment); @@ -302,18 +301,17 @@ void Replay::mergeSegments(const SegmentMap::iterator &begin, const SegmentMap:: if (stream_thread_) { emit segmentsMerged(); + } + updateEvents([&]() { + events_.swap(new_events); + merged_segments_ = segments_to_merge; // Check if seeking is in progress int target_segment = int(seeking_to_seconds_ / 60); if (seeking_to_seconds_ >= 0 && segments_to_merge.count(target_segment) > 0) { emit seekedTo(seeking_to_seconds_); seeking_to_seconds_ = -1; // Reset seeking_to_seconds_ to indicate completion of seek } - } - - updateEvents([&]() { - events_.swap(new_events); - merged_segments_ = segments_to_merge; // Wake up the stream thread if the current segment is loaded or invalid. return isSegmentMerged(current_segment_) || (segments_.count(current_segment_) == 0); });