69 lines
3.3 KiB
Diff
69 lines
3.3 KiB
Diff
# UNDF: UNDF-2026-000001215
|
|
# UNDF: (leave blank)
|
|
# Defect: kdenlive-0010
|
|
# Component: src/assets/keyframes/model/keyframemodellist.cpp
|
|
# Pattern: CWE-407 — QList<GenTime>::contains() O(K) called inside loops in checkConsistency()
|
|
# Severity: LOW-MEDIUM — O(P * K^2) at clip load for multi-parameter effects with many keyframes
|
|
#
|
|
# checkConsistency() builds a union list of keyframe positions across all parameters (P),
|
|
# then verifies each parameter has all positions. Both phases call QList::contains() inside
|
|
# a for loop — O(K) linear scan per element.
|
|
#
|
|
# Phase 1 (building fullList):
|
|
# for param in m_parameters: O(P)
|
|
# for time in param.getKeyframePos(): O(K)
|
|
# if !fullList.contains(time): O(K) — QList linear scan
|
|
# Total: O(P * K^2)
|
|
#
|
|
# Phase 2 (checking consistency):
|
|
# for param in m_parameters: O(P)
|
|
# for time in fullList: O(K)
|
|
# if !list.contains(time): O(K) — QList linear scan
|
|
# Total: O(P * K^2)
|
|
#
|
|
# P = number of linked parameters (2-10, typically 2-3 for position/scale/opacity effects)
|
|
# K = number of keyframes (can reach 1000+ in motion-tracked or animated clips)
|
|
#
|
|
# Fix: use std::set<GenTime> (GenTime has operator<) for O(log K) lookup in both phases.
|
|
# QSet<GenTime> cannot be used directly because GenTime equality uses a floating-point delta
|
|
# that is incompatible with a hash-based set. std::set<GenTime> with operator< is safe.
|
|
#
|
|
# At K=500, P=3: 750,000 comparisons -> 4,500 comparisons (log2(500) ~= 9)
|
|
# Speedup: ~166x at K=500, ~330x at K=1000
|
|
--- a/src/assets/keyframes/model/keyframemodellist.cpp
|
|
+++ b/src/assets/keyframes/model/keyframemodellist.cpp
|
|
@@ -1,5 +1,6 @@
|
|
#include "keyframemodellist.hpp"
|
|
+#include <set>
|
|
// ... other includes ...
|
|
|
|
@@ -900,18 +901,21 @@ void KeyframeModelList::checkConsistency()
|
|
if (m_parameters.size() < 2) {
|
|
return;
|
|
}
|
|
- // Check keyframes in all parameters
|
|
- QList<GenTime> fullList;
|
|
+ // Phase 1: build union of all keyframe positions using O(log K) std::set for dedup
|
|
+ std::set<GenTime> fullSet;
|
|
+ QList<GenTime> fullList;
|
|
for (const auto ¶m : m_parameters) {
|
|
QList<GenTime> list = param.second->getKeyframePos();
|
|
for (auto &time : list) {
|
|
- if (!fullList.contains(time)) {
|
|
+ if (fullSet.insert(time).second) {
|
|
fullList << time;
|
|
}
|
|
}
|
|
}
|
|
+ // Phase 2: verify each parameter has all positions — O(P * K * log K)
|
|
Fun local_update = []() { return true; };
|
|
auto type = KeyframeType::KeyframeEnum(KdenliveSettings::defaultkeyframeinterp());
|
|
for (const auto ¶m : m_parameters) {
|
|
QList<GenTime> list = param.second->getKeyframePos();
|
|
+ const std::set<GenTime> listSet(list.begin(), list.end());
|
|
for (auto &time : fullList) {
|
|
- if (!list.contains(time)) {
|
|
+ if (listSet.find(time) == listSet.end()) {
|
|
qDebug() << " = = = \n\n = = = = \n\nWARNING; MISSING KF DETECTED AT: " << time.seconds() << "\n\n= = =";
|
|
pCore->displayMessage(i18n("Missing keyframe detected at %1, automatically re-added", time.seconds()), ErrorMessage);
|
|
QVariant missingVal = param.second->getInterpolatedValue(time);
|