Fix timeline corruption (some operations used a copy of master prod instead of track producer)

Causing corruption in bin effects

src/bin/projectclip.cpp

@@ -166,18 +166,12 @@ ProjectClip::~ProjectClip()
 
 void ProjectClip::connectEffectStack()
 {
-    connect(m_effectStack.get(), &EffectStackModel::modelChanged, this, &ProjectClip::updateChildProducers);
-    connect(m_effectStack.get(), &EffectStackModel::dataChanged, this, &ProjectClip::updateChildProducers);
     connect(m_effectStack.get(), &EffectStackModel::dataChanged, [&]() {
         if (auto ptr = m_model.lock()) {
             std::static_pointer_cast<ProjectItemModel>(ptr)->onItemUpdated(std::static_pointer_cast<ProjectClip>(shared_from_this()),
                                                                            AbstractProjectItem::IconOverlay);
         }
     });
-    /*connect(m_effectStack.get(), &EffectStackModel::modelChanged, [&](){
-        qDebug()<<"/ / / STACK CHANGED";
-        updateChildProducers();
-    });*/
 }
 
 QString ProjectClip::getToolTip() const
@@ -1410,22 +1404,6 @@ bool ProjectClip::isIncludedInTimeline()
     return m_registeredClips.size() > 0;
 }
 
-void ProjectClip::updateChildProducers()
-{
-    // TODO refac: the effect should be managed by an effectstack on the master
-    /*
-    // pass effect stack on all child producers
-    QMutexLocker locker(&m_producerMutex);
-    for (const auto &clip : m_timelineProducers) {
-        if (auto producer = clip.second) {
-            Clip clp(producer->parent());
-            clp.deleteEffects();
-            clp.replaceEffects(*m_masterProducer);
-        }
-    }
-    */
-}
-
 void ProjectClip::replaceInTimeline()
 {
     for (const auto &clip : m_registeredClips) {

src/bin/projectclip.h

@@ -236,8 +236,6 @@ protected:
     void deregisterTimelineClip(int clipId);
 
     void emitProducerChanged(const QString &id, const std::shared_ptr<Mlt::Producer> &producer) override { emit producerChanged(id, producer); };
-    /** @brief Replace instance of this clip in timeline */
-    void updateChildProducers();
     void replaceInTimeline();
     void connectEffectStack() override;
 

src/timeline2/model/builders/meltBuilder.cpp

@@ -286,7 +286,7 @@ bool constructTrackFromMelt(const std::shared_ptr<TimelineItemModel> &timeline,
             if (pCore->bin()->getBinClip(binId)) {
                 PlaylistState::ClipState st = inferState(clip, audioTrack);
                 cid = ClipModel::construct(timeline, binId, clip, st);
-                ok = timeline->requestClipMove(cid, tid, position, true, false, undo, redo);
+                ok = timeline->requestClipMove(cid, tid, position, true, false, true, undo, redo);
             } else {
                 qDebug() << "// Cannot find bin clip: " << binId << " - " << clip->get("id");
             }

src/timeline2/model/timelinefunctions.cpp

@@ -131,7 +131,7 @@ bool TimelineFunctions::processClipCut(const std::shared_ptr<TimelineItemModel>
     res = res && timeline->requestItemResize(newId, duration - newDuration, false, true, undo, redo);
     // The next requestclipmove does not check for duration change since we don't invalidate timeline, so check duration change now
     bool durationChanged = trackDuration != timeline->getTrackById_const(trackId)->trackDuration();
-    res = res && timeline->requestClipMove(newId, trackId, position, true, false, undo, redo);
+    res = res && timeline->requestClipMove(newId, trackId, position, true, false, true, undo, redo);
     if (durationChanged) {
         // Track length changed, check project duration
         Fun updateDuration = [timeline]() {
@@ -254,7 +254,7 @@ bool TimelineFunctions::requestSpacerEndOperation(const std::shared_ptr<Timeline
         } else {
             // only 1 clip to be moved
             if (isClip) {
-                final = timeline->requestClipMove(itemId, track, endPosition, true, true, undo, redo);
+                final = timeline->requestClipMove(itemId, track, endPosition, true, true, true, undo, redo);
             } else {
                 final = timeline->requestCompositionMove(itemId, track, -1, endPosition, true, true, undo, redo);
             }
@@ -412,7 +412,7 @@ bool TimelineFunctions::removeSpace(const std::shared_ptr<TimelineItemModel> &ti
         } else {
             // only 1 clip to be moved
             int clipStart = timeline->getItemPosition(clipId);
-            result = timeline->requestClipMove(clipId, timeline->getItemTrackId(clipId), clipStart - (zone.y() - zone.x()), true, true, undo, redo);
+            result = timeline->requestClipMove(clipId, timeline->getItemTrackId(clipId), clipStart - (zone.y() - zone.x()), true, true, true, undo, redo);
         }
     }
     return result;
@@ -454,7 +454,7 @@ bool TimelineFunctions::requestInsertSpace(const std::shared_ptr<TimelineItemMod
         result =
             result && timeline->requestGroupMove(itemId, timeline->m_groups->getRootId(itemId), 0, zone.y() - zone.x(), true, true, local_undo, local_redo);
     } else if (timeline->isClip(itemId)) {
-        result = result && timeline->requestClipMove(itemId, targetTrackId, targetPos, true, true, local_undo, local_redo);
+        result = result && timeline->requestClipMove(itemId, targetTrackId, targetPos, true, true, true, local_undo, local_redo);
     } else {
         result = result && timeline->requestCompositionMove(itemId, targetTrackId, timeline->m_allCompositions[itemId]->getForcedTrack(), targetPos, true, true,
                                                             local_undo, local_redo);
@@ -493,7 +493,7 @@ bool TimelineFunctions::requestItemCopy(const std::shared_ptr<TimelineItemModel>
             std::advance(it, target_track_position);
             int target_track = (*it)->getId();
             if (timeline->isClip(id)) {
-                res = res && timeline->requestClipMove(newId, target_track, target_position, true, true, undo, redo);
+                res = res && timeline->requestClipMove(newId, target_track, target_position, true, true, true, undo, redo);
             } else {
                 const QString &transitionId = timeline->m_allCompositions[id]->getAssetId();
                 std::unique_ptr<Mlt::Properties> transProps(timeline->m_allCompositions[id]->properties());
@@ -617,7 +617,7 @@ bool TimelineFunctions::requestSplitAudio(const std::shared_ptr<TimelineItemMode
         bool success = false;
         while (!success && !possibleTracks.isEmpty()) {
             int newTrack = possibleTracks.takeFirst();
-            success = timeline->requestClipMove(newId, newTrack, position, true, false, undo, redo);
+            success = timeline->requestClipMove(newId, newTrack, position, true, false, true, undo, redo);
         }
         TimelineFunctions::changeClipState(timeline, cid, PlaylistState::VideoOnly, undo, redo);
         success = success && timeline->m_groups->createGroupAtSameLevel(cid, std::unordered_set<int>{newId}, GroupType::AVSplit, undo, redo);
@@ -668,7 +668,7 @@ bool TimelineFunctions::requestSplitVideo(const std::shared_ptr<TimelineItemMode
         bool success = false;
         while (!success && !possibleTracks.isEmpty()) {
             int newTrack = possibleTracks.takeFirst();
-            success = timeline->requestClipMove(newId, newTrack, position, true, false, undo, redo);
+            success = timeline->requestClipMove(newId, newTrack, position, true, true, true, undo, redo);
         }
         TimelineFunctions::changeClipState(timeline, cid, PlaylistState::AudioOnly, undo, redo);
         success = success && timeline->m_groups->createGroupAtSameLevel(cid, std::unordered_set<int>{newId}, GroupType::AVSplit, undo, redo);

src/timeline2/model/timelinemodel.cpp

@@ -503,9 +503,9 @@ bool TimelineModel::requestFakeClipMove(int clipId, int trackId, int position, b
     return false;
 }
 
-bool TimelineModel::requestClipMove(int clipId, int trackId, int position, bool updateView, bool invalidateTimeline, Fun &undo, Fun &redo)
+bool TimelineModel::requestClipMove(int clipId, int trackId, int position, bool updateView, bool invalidateTimeline, bool finalMove, Fun &undo, Fun &redo)
 {
-    // qDebug() << "// FINAL MOVE: " << invalidateTimeline << ", UPDATE VIEW: " << updateView;
+    // qDebug() << "// FINAL MOVE: " << invalidateTimeline << ", UPDATE VIEW: " << updateView<<", FINAL: "<<finalMove;
     if (trackId == -1) {
         return false;
     }
@@ -533,10 +533,10 @@ bool TimelineModel::requestClipMove(int clipId, int trackId, int position, bool
         // Move on same track, simply inform the view
         updateView = false;
         notifyViewOnly = true;
-        update_model = [clipId, this, invalidateTimeline]() {
+        update_model = [clipId, this, trackId, invalidateTimeline]() {
             QModelIndex modelIndex = makeClipIndexFromID(clipId);
             notifyChange(modelIndex, modelIndex, StartRole);
-            if (invalidateTimeline) {
+            if (invalidateTimeline && !getTrackById_const(trackId)->isAudioTrack()) {
                 int in = getClipPosition(clipId);
                 emit invalidateZone(in, in + getClipPlaytime(clipId));
             }
@@ -547,14 +547,14 @@ bool TimelineModel::requestClipMove(int clipId, int trackId, int position, bool
         if (notifyViewOnly) {
             PUSH_LAMBDA(update_model, local_undo);
         }
-        ok = getTrackById(old_trackId)->requestClipDeletion(clipId, updateView, invalidateTimeline, local_undo, local_redo);
+        ok = getTrackById(old_trackId)->requestClipDeletion(clipId, updateView, finalMove, local_undo, local_redo);
         if (!ok) {
             bool undone = local_undo();
             Q_ASSERT(undone);
             return false;
         }
     }
-    ok = ok & getTrackById(trackId)->requestClipInsertion(clipId, position, updateView, invalidateTimeline, local_undo, local_redo);
+    ok = ok & getTrackById(trackId)->requestClipInsertion(clipId, position, updateView, finalMove, local_undo, local_redo);
     if (!ok) {
         qDebug() << "-------------\n\nINSERTION FAILED, REVERTING\n\n-------------------";
         bool undone = local_undo();
@@ -621,7 +621,7 @@ bool TimelineModel::requestClipMove(int clipId, int trackId, int position, bool
     }
     std::function<bool(void)> undo = []() { return true; };
     std::function<bool(void)> redo = []() { return true; };
-    bool res = requestClipMove(clipId, trackId, position, updateView, invalidateTimeline, undo, redo);
+    bool res = requestClipMove(clipId, trackId, position, updateView, invalidateTimeline, logUndo, undo, redo);
     if (res && logUndo) {
         PUSH_UNDO(undo, redo, i18n("Move clip"));
     }
@@ -649,7 +649,7 @@ bool TimelineModel::requestClipMoveAttempt(int clipId, int trackId, int position
         int delta_pos = position - m_allClips[clipId]->getPosition();
         res = requestGroupMove(clipId, groupId, delta_track, delta_pos, false, false, undo, redo, false);
     } else {
-        res = requestClipMove(clipId, trackId, position, false, false, undo, redo);
+        res = requestClipMove(clipId, trackId, position, false, false, false, undo, redo);
     }
     if (res) {
         undo();
@@ -987,7 +987,7 @@ bool TimelineModel::requestClipInsertion(const QString &binClipId, int trackId,
         }
         bool audioDrop = getTrackById_const(trackId)->isAudioTrack();
         res = requestClipCreation(binClipId, id, getTrackById_const(trackId)->trackType(), 1.0, local_undo, local_redo);
-        res = res && requestClipMove(id, trackId, position, refreshView, logUndo, local_undo, local_redo);
+        res = res && requestClipMove(id, trackId, position, refreshView, logUndo, logUndo, local_undo, local_redo);
         int target_track;
         if (audioDrop) {
             target_track = m_videoTarget == -1 ? -1 : getTrackById_const(m_videoTarget)->isLocked() ? -1 : m_videoTarget;
@@ -1023,7 +1023,7 @@ bool TimelineModel::requestClipInsertion(const QString &binClipId, int trackId,
                     bool move = false;
                     while (!move && !possibleTracks.isEmpty()) {
                         int newTrack = possibleTracks.takeFirst();
-                        move = requestClipMove(newId, newTrack, position, true, false, audio_undo, audio_redo);
+                        move = requestClipMove(newId, newTrack, position, true, true, true, audio_undo, audio_redo);
                     }
                     // use lazy evaluation to group only if move was successful
                     res = res && move && requestClipsGroup({id, newId}, audio_undo, audio_redo, GroupType::AVSplit);
@@ -1054,7 +1054,7 @@ bool TimelineModel::requestClipInsertion(const QString &binClipId, int trackId,
             normalisedBinId.remove(0, 1);
         }
         res = requestClipCreation(normalisedBinId, id, dropType, 1.0, local_undo, local_redo);
-        res = res && requestClipMove(id, trackId, position, refreshView, logUndo, local_undo, local_redo);
+        res = res && requestClipMove(id, trackId, position, refreshView, logUndo, logUndo, local_undo, local_redo);
     }
     if (!res) {
         bool undone = local_undo();
@@ -1431,7 +1431,7 @@ bool TimelineModel::requestGroupMove(int itemId, int groupId, int delta_track, i
             int target_track = (*it)->getId();
             int target_position = old_position[item] + delta_pos;
             if (isClip(item)) {
-                ok = ok && requestClipMove(item, target_track, target_position, updateThisView, finalMove, local_undo, local_redo);
+                ok = ok && requestClipMove(item, target_track, target_position, updateThisView, finalMove, finalMove, local_undo, local_redo);
             } else {
                 ok = ok &&
                      requestCompositionMove(item, target_track, old_forced_track[item], target_position, updateThisView, finalMove, local_undo, local_redo);

src/timeline2/model/timelinemodel.hpp

@@ -345,7 +345,7 @@ public:
 
     /* Same function, but accumulates undo and redo, and doesn't check
        for group*/
-    bool requestClipMove(int clipId, int trackId, int position, bool updateView, bool invalidateTimeline, Fun &undo, Fun &redo);
+    bool requestClipMove(int clipId, int trackId, int position, bool updateView, bool invalidateTimeline, bool finalMove, Fun &undo, Fun &redo);
     bool requestCompositionMove(int transid, int trackId, int compositionTrack, int position, bool updateView, bool finalMove, Fun &undo, Fun &redo);
 
     /* When timeline edit mode is insert or overwrite, we fake the move (as it will overlap existing clips, and only process the real move on drop */

src/timeline2/view/timelinecontroller.cpp

@@ -1909,9 +1909,9 @@ void TimelineController::editItemDuration(int id)
         bool result = true;
         if (newPos < start) {
             if (!isComposition) {
-                result = m_model->requestClipMove(id, trackId, newPos, true, true, undo, redo);
+                result = m_model->requestClipMove(id, trackId, newPos, true, true, true, undo, redo);
                 if (result && partner > -1) {
-                    result = m_model->requestClipMove(partner, m_model->getItemTrackId(partner), newPos, true, true, undo, redo);
+                    result = m_model->requestClipMove(partner, m_model->getItemTrackId(partner), newPos, true, true, true, undo, redo);
                 }
             } else {
                 result = m_model->requestCompositionMove(id, trackId, newPos, m_model->m_allCompositions[id]->getForcedTrack(), true, true, undo, redo);
@@ -1944,9 +1944,9 @@ void TimelineController::editItemDuration(int id)
             }
             if (start != newPos || newIn != in) {
                 if (!isComposition) {
-                    result = result && m_model->requestClipMove(id, trackId, newPos, true, true, undo, redo);
+                    result = result && m_model->requestClipMove(id, trackId, newPos, true, true, true, undo, redo);
                     if (result && partner > -1) {
-                        result = m_model->requestClipMove(partner, m_model->getItemTrackId(partner), newPos, true, true, undo, redo);
+                        result = m_model->requestClipMove(partner, m_model->getItemTrackId(partner), newPos, true, true, true, undo, redo);
                     }
                 } else {
                     result = result &&
@@ -2197,7 +2197,7 @@ bool TimelineController::endFakeGroupMove(int clipId, int groupId, int delta_tra
         if (m_model->isClip(item)) {
             int target_track = new_track_ids[item];
             int target_position = old_position[item] + delta_pos;
-            ok = ok && m_model->requestClipMove(item, target_track, target_position, updateView, finalMove, undo, redo);
+            ok = ok && m_model->requestClipMove(item, target_track, target_position, updateView, finalMove, finalMove, undo, redo);
         } else {
             // ok = ok && requestCompositionMove(item, target_track, old_forced_track[item], target_position, updateThisView, local_undo, local_redo);
         }

tests/modeltest.cpp

@@ -1681,7 +1681,7 @@ TEST_CASE("Undo and Redo", "[ClipModel]")
         {
             std::function<bool(void)> undo = []() { return true; };
             std::function<bool(void)> redo = []() { return true; };
-            REQUIRE(timeline->requestClipMove(cid6, tid1, 7, true, true, undo, redo));
+            REQUIRE(timeline->requestClipMove(cid6, tid1, 7, true, true, true, undo, redo));
             pCore->pushUndo(undo, redo, QString());
         }
         auto state3 = [&]() {