Skip to content

GitLab

Commit 8258a85a authored by Jean-Baptiste Mardelle's avatar Jean-Baptiste Mardelle
Browse files

Fix crash and corruption on mix resize, add tests

parent 42d4c08f
Pipeline #151133 passed with stage
in 8 minutes and 42 seconds
Hide whitespace changes
Inline Side-by-side
src/timeline2/model/timelinemodel.cpp
View file @ 8258a85a
......@@ -3114,11 +3114,9 @@ int TimelineModel::requestItemResize(int itemId, int size, bool right, bool logU
offset -= size;
Fun undo = []() { return true; };
Fun redo = []() { return true; };
Fun sync_mix = []() { return true; };
Fun adjust_mix = []() { return true; };
Fun sync_end_mix = []() { return true; };
Fun sync_end_mix_undo = []() { return true; };
PUSH_LAMBDA(sync_mix, undo);
std::unordered_set<int> all_items;
QList <int> tracksWithMixes;
all_items.insert(itemId);
......@@ -3128,7 +3126,10 @@ int TimelineModel::requestItemResize(int itemId, int size, bool right, bool logU
if (getTrackById_const(tid)->hasEndMix(itemId)) {
tracksWithMixes << tid;
std::pair<MixInfo, MixInfo> mixData = getTrackById_const(tid)->getMixInfo(itemId);
if (in + size < mixData.second.secondClipInOut.first + m_allClips[mixData.second.secondClipId]->getMixDuration() - m_allClips[mixData.second.secondClipId]->getMixCutPosition()) {
int resizeOut = in + size;
int clipMixCut = mixData.second.secondClipInOut.first + m_allClips[mixData.second.secondClipId]->getMixDuration() - m_allClips[mixData.second.secondClipId]->getMixCutPosition();
int clipMixStart = mixData.second.secondClipInOut.first;
if (resizeOut < clipMixCut || resizeOut <= clipMixStart) {
// Clip resized outside of mix zone, mix will be deleted
bool res = removeMixWithUndo(mixData.second.secondClipId, undo, redo);
if (res) {
......@@ -3190,21 +3191,12 @@ int TimelineModel::requestItemResize(int itemId, int size, bool right, bool logU
std::pair<MixInfo, MixInfo> mixData = getTrackById_const(tid)->getMixInfo(itemId);
if (out - size >= mixData.first.firstClipInOut.second) {
// Moved outside mix, delete
Fun sync_mix_undo = [this, tid, mixData]() {
getTrackById_const(tid)->createMix(mixData.first, getTrackById_const(tid)->isAudioTrack());
getTrackById_const(tid)->syncronizeMixes(true);
return true;
};
bool switchPlaylist = getTrackById_const(tid)->hasEndMix(itemId) == false && m_allClips[itemId]->getSubPlaylistIndex() == 1;
if (switchPlaylist) {
sync_end_mix = [this, tid, mixData]() {
return getTrackById_const(tid)->switchPlaylist(mixData.first.secondClipId, m_allClips[mixData.first.secondClipId]->getPosition(), 1, 0);
};
sync_end_mix_undo = [this, tid, mixData]() {
return getTrackById_const(tid)->switchPlaylist(mixData.first.secondClipId, m_allClips[mixData.first.secondClipId]->getPosition(), 0, 1);
};
bool res = removeMixWithUndo(mixData.first.secondClipId, undo, redo);
if (res) {
size = m_allClips[itemId]->getPlaytime();
} else {
return -1;
}
PUSH_LAMBDA(sync_mix_undo, undo);
}
}
if (getTrackById_const(tid)->hasEndMix(itemId)) {
......@@ -3272,23 +3264,6 @@ int TimelineModel::requestItemResize(int itemId, int size, bool right, bool logU
if (end - offset <= mixData.second.secondClipInOut.first + m_allClips[mixData.second.secondClipId]->getMixDuration() - m_allClips[mixData.second.secondClipId]->getMixCutPosition()) {
// Resized outside mix
removeMixWithUndo(mixData.second.secondClipId, undo, redo);
Fun sync_mix_undo = [this, trackId, mixData]() {
getTrackById_const(trackId)->createMix(mixData.second, getTrackById_const(trackId)->isAudioTrack());
getTrackById_const(trackId)->syncronizeMixes(true);
return true;
};
bool switchPlaylist = getTrackById_const(trackId)->hasEndMix(mixData.second.secondClipId) == false && m_allClips[mixData.second.secondClipId]->getSubPlaylistIndex() == 1;
if (switchPlaylist) {
Fun sync_end_mix2 = [this, trackId, mixData]() {
return getTrackById_const(trackId)->switchPlaylist(mixData.second.secondClipId, mixData.second.secondClipInOut.first, 1, 0);
};
Fun sync_end_mix_undo2 = [this, trackId, mixData]() {
return getTrackById_const(trackId)->switchPlaylist(mixData.second.secondClipId, m_allClips[mixData.second.secondClipId]->getPosition(), 0, 1);
};
PUSH_LAMBDA(sync_end_mix2, sync_end_mix);
PUSH_LAMBDA(sync_end_mix_undo2, sync_end_mix_undo);
}
PUSH_LAMBDA(sync_mix_undo, undo);
} else {
// Mix was resized, update cut position
int currentMixDuration = m_allClips[mixData.second.secondClipId]->getMixDuration();
......@@ -3318,37 +3293,13 @@ int TimelineModel::requestItemResize(int itemId, int size, bool right, bool logU
std::pair<MixInfo, MixInfo> mixData = getTrackById_const(trackId)->getMixInfo(id);
if (start + offset >= mixData.first.firstClipInOut.second) {
// Moved outside mix, remove
Fun sync_mix_undo = [this, trackId, mixData]() {
getTrackById_const(trackId)->createMix(mixData.first, getTrackById_const(trackId)->isAudioTrack());
getTrackById_const(trackId)->syncronizeMixes(true);
return true;
};
bool switchPlaylist = getTrackById_const(trackId)->hasEndMix(id) == false && m_allClips[id]->getSubPlaylistIndex() == 1;
if (switchPlaylist) {
Fun sync_end_mix2 = [this, trackId, mixData]() {
return getTrackById_const(trackId)->switchPlaylist(mixData.first.secondClipId, m_allClips[mixData.first.secondClipId]->getPosition(), 1, 0);
};
Fun sync_end_mix_undo2 = [this, trackId, mixData]() {
return getTrackById_const(trackId)->switchPlaylist(mixData.first.secondClipId, m_allClips[mixData.first.secondClipId]->getPosition(), 0, 1);
};
PUSH_LAMBDA(sync_end_mix2, sync_end_mix);
PUSH_LAMBDA(sync_end_mix_undo2, sync_end_mix_undo);
}
PUSH_LAMBDA(sync_mix_undo, undo);
removeMixWithUndo(mixData.first.secondClipId, undo, redo);
}
}
}
}
}
}
if (logUndo && !tracksWithMixes.isEmpty()) {
sync_mix = [this, tracksWithMixes]() {
for (auto &t : tracksWithMixes) {
getTrackById_const(t)->syncronizeMixes(true);
}
return true;
};
}
bool result = true;
int finalPos = right ? in + size : out - size;
int finalSize;
......@@ -3382,14 +3333,9 @@ int TimelineModel::requestItemResize(int itemId, int size, bool right, bool logU
Q_ASSERT(undone);
} else if (logUndo) {
if (isClip(itemId)) {
sync_end_mix();
sync_mix();
adjust_mix();
PUSH_LAMBDA(sync_end_mix, redo);
PUSH_LAMBDA(adjust_mix, redo);
PUSH_LAMBDA(sync_mix, redo);
PUSH_LAMBDA(undo, sync_end_mix_undo);
PUSH_UNDO(sync_end_mix_undo, redo, i18n("Resize clip"))
PUSH_UNDO(undo, redo, i18n("Resize clip"))
} else if (isComposition(itemId)) {
PUSH_UNDO(undo, redo, i18n("Resize composition"))
} else if (isSubTitle(itemId)) {
......
src/timeline2/view/qml/ClipThumbs.qml
View file @ 8258a85a
......@@ -28,10 +28,10 @@ Row {
// 1: only show first thumbnail
// 0: will disable thumbnails
// Track trackThumbsFormat is: 2 = In frame only, 0 = in/out, 1 = All frames, 3 = No thumbs
model: parentTrack.trackThumbsFormat === 0 ? (container.width > thumbRow.thumbWidth ? 2 : 1) : parentTrack.trackThumbsFormat === 1 ? Math.ceil(container.width / thumbRow.thumbWidth) : parentTrack.trackThumbsFormat === 2 ? 1 : 0
model: parentTrack.trackThumbsFormat === 0 ? (parent.width > thumbRow.thumbWidth ? 2 : 1) : parentTrack.trackThumbsFormat === 1 ? Math.ceil(parent.width / thumbRow.thumbWidth) : parentTrack.trackThumbsFormat === 2 ? 1 : 0
property int startFrame: clipRoot.inPoint
property int endFrame: clipRoot.outPoint
property real imageWidth: Math.max(thumbRow.thumbWidth, container.width / thumbRepeater.count)
property real imageWidth: Math.max(thumbRow.thumbWidth, parent.width / thumbRepeater.count)
property int thumbStartFrame: fixedThumbs ? 0 :
(clipRoot.speed >= 0)
? Math.round(clipRoot.inPoint * thumbRow.initialSpeed)
......
tests/mixtest.cpp
View file @ 8258a85a
......@@ -113,10 +113,11 @@ TEST_CASE("Simple Mix", "[SameTrackMix]")
auto state2 = [&]() {
REQUIRE(timeline->getClipsCount() == 6);
REQUIRE(timeline->getClipPlaytime(cid3) > 20);
REQUIRE(timeline->getClipPlaytime(cid3) == 32);
REQUIRE(timeline->getClipPosition(cid3) == 500);
REQUIRE(timeline->getClipPlaytime(cid4) > 20);
REQUIRE(timeline->getClipPosition(cid4) < 520);
REQUIRE(timeline->getClipPlaytime(cid4) == 33);
REQUIRE(timeline->getClipPosition(cid4) == 507);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->getTrackById_const(tid1)->mixCount() == 0);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 1);
......@@ -362,7 +363,7 @@ TEST_CASE("Simple Mix", "[SameTrackMix]")
undoStack->undo();
state2();
// Resize right clip outside mix zone, should delete the mix
REQUIRE(timeline->requestItemResize(cid4, 4, false, true) == 4);
REQUIRE(timeline->requestItemResize(cid4, 4, false, true) == 20);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 0);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 0);
......@@ -383,6 +384,35 @@ TEST_CASE("Simple Mix", "[SameTrackMix]")
undoStack->undo();
state2();
// Before mix: CID 3 length=20, pos=500, CID4 length=20, pos=520
// Default mix duration = 25 frames (12 before / 13 after)
// Resize left clip before right clip start, then right clip outside left clip, should delete the mix
REQUIRE(timeline->requestItemResize(cid3, 20, true, true) == 20);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 1);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->requestItemResize(cid4, 20, false, true) == 20);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 0);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 0);
undoStack->undo();
undoStack->undo();
state2();
// Resize right clip after left clip end, then left clip outside right clip, should delete the mix
REQUIRE(timeline->requestItemResize(cid4, 20, false, true) == 20);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 1);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->requestItemResize(cid3, 20, true, true) == 20);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 0);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 0);
undoStack->undo();
undoStack->undo();
state2();
undoStack->undo();
state0();
}
......@@ -428,7 +458,7 @@ TEST_CASE("Simple Mix", "[SameTrackMix]")
undoStack->undo();
state3();
// Resize right clip outside mix zone, should delete the mix
REQUIRE(timeline->requestItemResize(cid2, 4, false, true) == 4);
REQUIRE(timeline->requestItemResize(cid2, 4, false, true) == 30);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 0);
REQUIRE(timeline->getTrackById_const(tid3)->mixCount() == 0);
REQUIRE(timeline->m_allClips[cid1]->getSubPlaylistIndex() == 0);
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment