Add SnapInterface class, add/remove marker on setCurrentTrack and update on setPositon and setInOut

src/bin/model/markerlistmodel.cpp

@@ -29,7 +29,6 @@
 #include "macros.hpp"
 #include "project/projectmanager.h"
 #include "timeline2/model/snapmodel.hpp"
-#include "timeline2/model/clipsnapmodel.hpp"
 
 #include <QDebug>
 #include <QJsonArray>
@@ -244,7 +243,7 @@ QHash<int, QByteArray> MarkerListModel::roleNames() const
 void MarkerListModel::addSnapPoint(GenTime pos)
 {
     QWriteLocker locker(&m_lock);
-    std::vector<std::weak_ptr<SnapModel>> validSnapModels;
+    std::vector<std::weak_ptr<SnapInterface>> validSnapModels;
     for (const auto &snapModel : m_registeredSnaps) {
         if (auto ptr = snapModel.lock()) {
             validSnapModels.push_back(snapModel);
@@ -253,21 +252,12 @@ void MarkerListModel::addSnapPoint(GenTime pos)
     }
     // Update the list of snapModel known to be valid
     std::swap(m_registeredSnaps, validSnapModels);
-    std::vector<std::weak_ptr<ClipSnapModel>> validClipSnapModels;
-    for (const auto &snapModel : m_registeredClipSnaps) {
-        if (auto ptr = snapModel.lock()) {
-            validClipSnapModels.push_back(snapModel);
-            ptr->addPoint(pos.frames(pCore->getCurrentFps()));
-        }
-    }
-    // Update the list of snapModel known to be valid
-    std::swap(m_registeredClipSnaps, validClipSnapModels);
 }
 
 void MarkerListModel::removeSnapPoint(GenTime pos)
 {
     QWriteLocker locker(&m_lock);
-    std::vector<std::weak_ptr<SnapModel>> validSnapModels;
+    std::vector<std::weak_ptr<SnapInterface>> validSnapModels;
     for (const auto &snapModel : m_registeredSnaps) {
         if (auto ptr = snapModel.lock()) {
             validSnapModels.push_back(snapModel);
@@ -276,16 +266,6 @@ void MarkerListModel::removeSnapPoint(GenTime pos)
     }
     // Update the list of snapModel known to be valid
     std::swap(m_registeredSnaps, validSnapModels);
-    
-    std::vector<std::weak_ptr<ClipSnapModel>> validClipSnapModels;
-    for (const auto &snapModel : m_registeredClipSnaps) {
-        if (auto ptr = snapModel.lock()) {
-            validClipSnapModels.push_back(snapModel);
-            ptr->removePoint(pos.frames(pCore->getCurrentFps()));
-        }
-    }
-    // Update the list of snapModel known to be valid
-    std::swap(m_registeredClipSnaps, validClipSnapModels);
 }
 
 QVariant MarkerListModel::data(const QModelIndex &index, int role) const
@@ -362,7 +342,7 @@ bool MarkerListModel::hasMarker(int frame) const
     return m_markerList.count(GenTime(frame, pCore->getCurrentFps())) > 0;
 }
 
-void MarkerListModel::registerSnapModel(const std::weak_ptr<SnapModel> &snapModel)
+void MarkerListModel::registerSnapModel(const std::weak_ptr<SnapInterface> &snapModel)
 {
     READ_LOCK();
     // make sure ptr is valid
@@ -372,27 +352,8 @@ void MarkerListModel::registerSnapModel(const std::weak_ptr<SnapModel> &snapMode
 
         // we now add the already existing markers to the snap
         for (const auto &marker : m_markerList) {
-            GenTime pos = marker.first;
-            ptr->addPoint(pos.frames(pCore->getCurrentFps()));
-        }
-    } else {
-        qDebug() << "Error: added snapmodel is null";
-        Q_ASSERT(false);
-    }
-}
-
-void MarkerListModel::registerClipSnapModel(const std::weak_ptr<ClipSnapModel> &snapModel)
-{
-    READ_LOCK();
-    // make sure ptr is valid
-    if (auto ptr = snapModel.lock()) {
-        // ptr is valid, we store it
-        m_registeredClipSnaps.push_back(snapModel);
-
-        // we now add the already existing markers to the snap
-        for (const auto &marker : m_markerList) {
-            GenTime pos = marker.first;
-            ptr->addPoint(pos.frames(pCore->getCurrentFps()));
+            qDebug()<<" *- *-* REGISTEING MARKER: "<<marker.first.frames(pCore->getCurrentFps());
+            ptr->addPoint(marker.first.frames(pCore->getCurrentFps()));
         }
     } else {
         qDebug() << "Error: added snapmodel is null";

src/bin/model/markerlistmodel.hpp

@@ -35,8 +35,7 @@
 
 class ClipController;
 class DocUndoStack;
-class SnapModel;
-class ClipSnapModel;
+class SnapInterface;
 
 /* @brief This class is the model for a list of markers.
    A marker is defined by a time, a type (the color used to represent it) and a comment string.
@@ -112,8 +111,7 @@ public:
        The snap logic for clips is managed from the Timeline
        Note that no deregistration is necessary, the weak_ptr will be discarded as soon as it becomes invalid.
     */
-    void registerSnapModel(const std::weak_ptr<SnapModel> &snapModel);
-    void registerClipSnapModel(const std::weak_ptr<ClipSnapModel> &snapModel);
+    void registerSnapModel(const std::weak_ptr<SnapInterface> &snapModel);
 
     /* @brief Exports the model to json using format above */
     QString toJson() const;
@@ -177,8 +175,7 @@ private:
     mutable QReadWriteLock m_lock; // This is a lock that ensures safety in case of concurrent access
 
     std::map<GenTime, std::pair<QString, int>> m_markerList;
-    std::vector<std::weak_ptr<SnapModel>> m_registeredSnaps;
-    std::vector<std::weak_ptr<ClipSnapModel>> m_registeredClipSnaps;
+    std::vector<std::weak_ptr<SnapInterface>> m_registeredSnaps;
 
 signals:
     void modelChanged();

src/timeline2/model/clipmodel.cpp

@@ -82,9 +82,8 @@ int ClipModel::construct(const std::shared_ptr<TimelineModel> &parent, const QSt
     std::shared_ptr<ClipModel> clip(new ClipModel(parent, cutProducer, binClipId, id, state, speed));
     TRACE_CONSTR(clip.get(), parent, binClipId, id, state, speed);
     clip->setClipState_lambda(state)();
-    //binClip->getMarkerModel()->registerClipSnapModel(clip->m_clipMarkerModel);
-    clip->m_clipMarkerModel->setReferenceModel(binClip->getMarkerModel());
     parent->registerClip(clip);
+    clip->m_clipMarkerModel->setReferenceModel(binClip->getMarkerModel());
     return id;
 }
 
@@ -113,9 +112,8 @@ int ClipModel::construct(const std::shared_ptr<TimelineModel> &parent, const QSt
     std::shared_ptr<ClipModel> clip(new ClipModel(parent, result.first, binClipId, id, state, speed));
     clip->setClipState_lambda(state)();
     clip->m_effectStack->importEffects(producer, state, result.second);
-    //binClip->getMarkerModel()->registerClipSnapModel(clip->m_clipMarkerModel);
-    clip->m_clipMarkerModel->setReferenceModel(binClip->getMarkerModel());
     parent->registerClip(clip);
+    clip->m_clipMarkerModel->setReferenceModel(binClip->getMarkerModel());
     return id;
 }
 
@@ -190,7 +188,7 @@ bool ClipModel::requestResize(int size, bool right, Fun &undo, Fun &redo, bool l
     }
     Fun operation = [this, inPoint, outPoint, track_operation]() {
         if (track_operation()) {
-            m_producer->set_in_and_out(inPoint, outPoint);
+            setInOut(inPoint, outPoint);
             return true;
         }
         return false;
@@ -214,7 +212,7 @@ bool ClipModel::requestResize(int size, bool right, Fun &undo, Fun &redo, bool l
         }
         Fun reverse = [this, old_in, old_out, track_reverse]() {
             if (track_reverse()) {
-                m_producer->set_in_and_out(old_in, old_out);
+                setInOut(old_in, old_out);
                 return true;
             }
             return false;
@@ -516,12 +514,36 @@ void ClipModel::setShowKeyframes(bool show)
     service()->set("kdenlive:hide_keyframes", (int)!show);
 }
 
+void ClipModel::setPosition(int pos)
+{
+    MoveableItem::setPosition(pos);
+    m_clipMarkerModel->updateSnapModelPos(pos);
+}
+
+void ClipModel::setInOut(int in, int out)
+{
+    MoveableItem::setInOut(in, out);
+    m_clipMarkerModel->updateSnapModelInOut(std::pair<int, int>(in, out));
+}
+
 void ClipModel::setCurrentTrackId(int tid, bool finalMove)
 {
     if (tid == m_currentTrackId) {
         return;
     }
+    bool registerSnap = m_currentTrackId == -1 && tid > -1;
+    
+    if (m_currentTrackId > -1 && tid == -1) {
+        // Removing clip
+        m_clipMarkerModel->deregisterSnapModel();
+    }
     MoveableItem::setCurrentTrackId(tid, finalMove);
+    if (registerSnap) {
+        if (auto ptr = m_parent.lock()) {
+            m_clipMarkerModel->registerSnapModel(ptr->m_snaps, getPosition(), getIn(), getOut());
+        }
+    }
+
     if (finalMove && tid != -1) {
         refreshProducerFromBin(m_currentState);
     }
@@ -704,16 +726,3 @@ void ClipModel::setSubPlaylistIndex(int index)
     m_subPlaylistIndex = index;
 }
 
-void ClipModel::registerMarkerModel(int position)
-{
-    if (auto ptr = m_parent.lock()) {
-        m_clipMarkerModel->registerSnapModel(ptr->m_snaps, position, getIn(), getOut());
-    }
-}
-
-void ClipModel::unregisterMarkerModel()
-{
-    if (auto ptr = m_parent.lock()) {
-        m_clipMarkerModel->unregisterSnapModel();
-    }
-}

src/timeline2/model/clipmodel.hpp

@@ -163,6 +163,8 @@ protected:
     bool requestResize(int size, bool right, Fun &undo, Fun &redo, bool logUndo = true) override;
 
     void setCurrentTrackId(int tid, bool finalMove = true) override;
+    void setPosition(int pos) override;
+    void setInOut(int in, int out) override;
 
     /* @brief This function change the global (timeline-wise) enabled state of the effects
      */
@@ -195,10 +197,6 @@ protected:
 
     /*@brief This is a debug function to ensure the clip is in a valid state */
     bool checkConsistency();
-    
-    /*@brief Register the clip marker model to timeline */
-    void registerMarkerModel(int position);
-    void unregisterMarkerModel();
 
 protected:
     std::shared_ptr<Mlt::Producer> m_producer;

src/timeline2/model/clipsnapmodel.cpp

@@ -21,7 +21,6 @@
 
 #include "bin/model/markerlistmodel.hpp"
 #include "clipsnapmodel.hpp"
-#include "snapmodel.hpp"
 #include <QDebug>
 #include <climits>
 #include <cstdlib>
@@ -29,8 +28,9 @@
 
 ClipSnapModel::ClipSnapModel() = default;
 
-void ClipSnapModel::addPoint(size_t position)
+void ClipSnapModel::addPoint(int position)
 {
+    m_snapPoints.insert(position);
     if (position <= m_inPoint || position >= m_outPoint) {
         return;
     }
@@ -39,8 +39,9 @@ void ClipSnapModel::addPoint(size_t position)
     }
 }
 
-void ClipSnapModel::removePoint(size_t position)
+void ClipSnapModel::removePoint(int position)
 {
+    m_snapPoints.erase(position);
     if (position <= m_inPoint || position >= m_outPoint) {
         return;
     }
@@ -49,63 +50,67 @@ void ClipSnapModel::removePoint(size_t position)
     }
 }
 
-void ClipSnapModel::registerSnapModel(const std::weak_ptr<SnapModel> &snapModel, size_t position, size_t in, size_t out)
+void ClipSnapModel::updateSnapModelPos(int newPos)
 {
-    // make sure ptr is valid
-    m_inPoint = in;
-    m_outPoint = out;
-    m_position = position;
-    auto ptr = m_registeredSnap.lock();
-    if (!ptr) {
-        m_registeredSnap = snapModel;
-        ptr = m_registeredSnap.lock();
+    if (newPos == m_position) {
+        return;
     }
-    if (ptr) {
-        // we now add the already existing markers to the snap
-        if (auto ptr2 = m_parentModel.lock()) {
-        std::vector<size_t> snaps = ptr2->getSnapPoints();
-            for (const auto &snap : snaps) {
-                if (snap < m_inPoint) {
-                    continue;
-                }
-                if (snap > m_outPoint) {
-                    break;
-                }
+    removeAllSnaps();
+    m_position = newPos;
+    addAllSnaps();
+}
+
+void ClipSnapModel::updateSnapModelInOut(std::pair<int, int> newInOut)
+{
+    removeAllSnaps();
+    m_inPoint = newInOut.first;
+    m_outPoint = newInOut.second;
+    addAllSnaps();
+}  
+
+void ClipSnapModel::addAllSnaps()
+{
+    if (auto ptr = m_registeredSnap.lock()) {
+        for (const auto &snap : m_snapPoints) {
+            if (snap >= m_inPoint && snap < m_outPoint) {
                 ptr->addPoint(m_position + snap - m_inPoint);
             }
         }
-    } else {
-        qDebug() << "Error: added snapmodel is null";
-        Q_ASSERT(false);
     }
 }
 
-void ClipSnapModel::unregisterSnapModel()
+void ClipSnapModel::removeAllSnaps()
 {
-    // make sure ptr is valid
-    auto ptr = m_registeredSnap.lock();
-    if (!ptr) {
-        return;
-    }
-    // we now add the already existing markers to the snap
-    if (auto ptr2 = m_parentModel.lock()) {
-        std::vector<size_t> snaps = ptr2->getSnapPoints();
-        for (const auto &snap : snaps) {
-            if (snap < m_inPoint) {
-                continue;
-            }
-            if (snap > m_outPoint) {
-                break;
+    if (auto ptr = m_registeredSnap.lock()) {
+        for (const auto &snap : m_snapPoints) {
+            if (snap >= m_inPoint && snap < m_outPoint) {
+                ptr->removePoint(m_position + snap - m_inPoint);
             }
-            ptr->removePoint(m_position + snap - m_inPoint);
         }
     }
 }
 
+void ClipSnapModel::registerSnapModel(const std::weak_ptr<SnapModel> &snapModel, int position, int in, int out)
+{
+    // make sure ptr is valid
+    m_inPoint = in;
+    m_outPoint = out;
+    m_position = qMax(0, position);
+    m_registeredSnap = snapModel;
+    addAllSnaps();
+}
+
+void ClipSnapModel::deregisterSnapModel()
+{
+    // make sure ptr is valid
+    removeAllSnaps();
+    m_registeredSnap.reset();
+}
+
 void ClipSnapModel::setReferenceModel(const std::weak_ptr<MarkerListModel> &markerModel)
 {
     m_parentModel = markerModel;
     if (auto ptr = m_parentModel.lock()) {
-        ptr->registerClipSnapModel(shared_from_this());
+        ptr->registerSnapModel(std::static_pointer_cast<SnapInterface>(shared_from_this()));
     }
 }

src/timeline2/model/clipsnapmodel.hpp

@@ -22,41 +22,48 @@
 #ifndef CLIPSNAPMODEL_H
 #define CLIPSNAPMODEL_H
 
+#include "snapmodel.hpp"
+
 #include <map>
-#include <vector>
 #include <memory>
+#include <unordered_set>
 
-class SnapModel;
 class MarkerListModel;
 
 /** @brief This class represents the snap points of a clip of the timeline.
-    Basically, one can add or remove snap points, and query the closest snap point to a given location
+    Basically, one can add or remove snap points
  *
  */
 
-class ClipSnapModel : public enable_shared_from_this_virtual<ClipSnapModel>
+class ClipSnapModel : public virtual SnapInterface, public std::enable_shared_from_this<SnapInterface>
 {
 public:
     ClipSnapModel();
 
     /* @brief Adds a snappoint at given position */
-    void addPoint(size_t position);
+    void addPoint(int position) override;
 
     /* @brief Removes a snappoint from given position */
-    void removePoint(size_t position);
+    void removePoint(int position) override;
 
-    void registerSnapModel(const std::weak_ptr<SnapModel> &snapModel, size_t position, size_t in, size_t out);
-    void unregisterSnapModel();
+    void registerSnapModel(const std::weak_ptr<SnapModel> &snapModel, int position, int in, int out);
+    void deregisterSnapModel();
     
     void setReferenceModel(const std::weak_ptr<MarkerListModel> &markerModel);
+    
+    void updateSnapModelPos(int newPos);
+    void updateSnapModelInOut(std::pair<int, int> newInOut);
 
 
 private:
     std::weak_ptr<SnapModel> m_registeredSnap;
     std::weak_ptr<MarkerListModel> m_parentModel;
-    size_t m_inPoint;
-    size_t m_outPoint;
-    size_t m_position;
+    std::unordered_set<int> m_snapPoints;
+    int m_inPoint;
+    int m_outPoint;
+    int m_position;
+    void addAllSnaps();
+    void removeAllSnaps();
 
 };
 

src/timeline2/model/moveableItem.hpp

@@ -99,7 +99,7 @@ protected:
       This function is meant to be called by the trackmodel, not directly by the user.
       If you wish to actually move the item, use the requestMove slot.
     */
-    void setPosition(int position);
+    virtual void setPosition(int position);
     /* Updates the stored track id of the item
        This function is meant to be called by the timeline, not directly by the user.
        If you wish to actually change the track the item, use the slot in the timeline

src/timeline2/model/snapmodel.cpp

@@ -23,6 +23,10 @@
 #include <climits>
 #include <cstdlib>
 
+
+SnapInterface::SnapInterface() = default;
+SnapInterface::~SnapInterface() = default;
+
 SnapModel::SnapModel() = default;
 
 void SnapModel::addPoint(int position)

src/timeline2/model/snapmodel.hpp

@@ -25,21 +25,38 @@
 #include <map>
 #include <vector>
 
+/** @brief This is a base class for snap models (timeline, clips)
+    Implements only basic functions like add or remove snap points
+ */
+
+class SnapInterface
+{
+public:
+    SnapInterface();
+    virtual ~SnapInterface();
+    /* @brief Adds a snappoint at given position */
+    virtual void addPoint(int position) = 0;
+
+    /* @brief Removes a snappoint from given position */
+    virtual void removePoint(int position) = 0;
+};
+
+
 /** @brief This class represents the snap points of the timeline.
     Basically, one can add or remove snap points, and query the closest snap point to a given location
  *
  */
 
-class SnapModel
+class SnapModel : public virtual SnapInterface
 {
 public:
     SnapModel();
 
     /* @brief Adds a snappoint at given position */
-    void addPoint(int position);
+    void addPoint(int position) override;
 
     /* @brief Removes a snappoint from given position */
-    void removePoint(int position);
+    void removePoint(int position) override;
 
     /* @brief Retrieves closest point. Returns -1 if there is no snappoint available */
     int getClosestPoint(int position);

src/timeline2/model/timelineitemmodel.cpp

@@ -31,6 +31,7 @@
 #include "kdenlivesettings.h"
 #include "macros.hpp"
 #include "trackmodel.hpp"
+#include "snapmodel.hpp"
 #include "transitions/transitionsrepository.hpp"
 #include <QDebug>
 #include <QFileInfo>
@@ -62,7 +63,7 @@ void TimelineItemModel::finishConstruct(const std::shared_ptr<TimelineItemModel>
 {
     ptr->weak_this_ = ptr;
     ptr->m_groups = std::make_unique<GroupsModel>(ptr);
-    guideModel->registerSnapModel(ptr->m_snaps);
+    guideModel->registerSnapModel(std::static_pointer_cast<SnapInterface>(ptr->m_snaps));
 }
 
 std::shared_ptr<TimelineItemModel> TimelineItemModel::construct(Mlt::Profile *profile, std::shared_ptr<MarkerListModel> guideModel,

src/timeline2/model/trackmodel.cpp

@@ -145,7 +145,6 @@ Fun TrackModel::requestClipInsertion_lambda(int clipId, int position, bool updat
             int new_out = new_in + clip->getPlaytime();
             ptr->m_snaps->addPoint(new_in);
             ptr->m_snaps->addPoint(new_out);
-            m_allClips[clipId]->registerMarkerModel(new_in);
             if (updateView) {
                 int clip_index = getRowfromClip(clipId);
                 ptr->_beginInsertRows(ptr->makeTrackIndexFromID(m_id), clip_index, clip_index);
@@ -302,7 +301,6 @@ Fun TrackModel::requestClipDeletion_lambda(int clipId, bool updateView, bool fin
         Q_ASSERT(!m_playlists[target_track].is_blank(target_clip));
         auto prod = m_playlists[target_track].replace_with_blank(target_clip);
         if (prod != nullptr) {
-            m_allClips[clipId]->unregisterMarkerModel();
             m_playlists[target_track].consolidate_blanks();
             m_allClips[clipId]->setCurrentTrackId(-1);
             m_allClips[clipId]->setSubPlaylistIndex(-1);
@@ -485,7 +483,6 @@ Fun TrackModel::requestClipResize_lambda(int clipId, int in, int out, bool right
             ptr->m_snaps->removePoint(old_out);
             ptr->m_snaps->addPoint(new_in);
             ptr->m_snaps->addPoint(new_out);
-            m_allClips[clipId]->registerMarkerModel(new_in);
             if (checkRefresh) {
                 ptr->checkRefresh(old_in, old_out);
                 ptr->checkRefresh(new_in, new_out);
@@ -504,7 +501,6 @@ Fun TrackModel::requestClipResize_lambda(int clipId, int in, int out, bool right
     // qDebug() << "RESIZING CLIP: " << clipId << " FROM: " << delta;
     if (delta > 0) { // we shrink clip
         return [right, target_clip, target_track, clip_position, delta, in, out, clipId, update_snaps, this]() {
-            m_allClips[clipId]->unregisterMarkerModel();
             int target_clip_mutable = target_clip;
             int blank_index = right ? (target_clip_mutable + 1) : target_clip_mutable;
             // insert blank to space that is going to be empty
@@ -537,7 +533,6 @@ Fun TrackModel::requestClipResize_lambda(int clipId, int in, int out, bool right
             // clip is last, it can always be extended
             return [this, target_clip, target_track, in, out, update_snaps, clipId]() {
                 // color, image and title clips can have unlimited resize
-                m_allClips[clipId]->unregisterMarkerModel();
                 QScopedPointer<Mlt::Producer> clip(m_playlists[target_track].get_clip(target_clip));
                 if (out >= clip->get_length()) {
                     clip->parent().set("length", out + 1);
@@ -571,7 +566,6 @@ Fun TrackModel::requestClipResize_lambda(int clipId, int in, int out, bool right
         int blank_length = m_playlists[target_track].clip_length(blank);
         if (blank_length + delta >= 0 && other_blank_end >= out) {
             return [blank_length, blank, right, clipId, delta, update_snaps, this, in, out, target_clip, target_track]() {
-                m_allClips[clipId]->unregisterMarkerModel();
                 int target_clip_mutable = target_clip;
                 int err = 0;
                 if (blank_length + delta == 0) {

src/timeline2/view/timelinecontroller.cpp

@@ -586,6 +586,10 @@ void TimelineController::setOutPoint()
 void TimelineController::editMarker(int cid, int position)
 {
     Q_ASSERT(m_model->isClip(cid));
+    if (position < m_model->getClipPosition(cid) || position > (m_model->getClipPosition(cid) + m_model->getClipPlaytime(cid))) {
+        pCore->displayMessage(i18n("Cannot find clip to edit marker"), InformationMessage, 500);
+        return;
+    }
     std::shared_ptr<ProjectClip> clip = pCore->bin()->getBinClip(getClipBinId(cid));
     GenTime pos(position - m_model->getClipPosition(cid) + m_model->getClipIn(cid), pCore->getCurrentFps());
     clip->getMarkerModel()->editMarkerGui(pos, qApp->activeWindow(), false, clip.get());
@@ -594,6 +598,10 @@ void TimelineController::editMarker(int cid, int position)
 void TimelineController::addMarker(int cid, int position)
 {
     Q_ASSERT(m_model->isClip(cid));
+    if (position < m_model->getClipPosition(cid) || position > (m_model->getClipPosition(cid) + m_model->getClipPlaytime(cid))) {
+        pCore->displayMessage(i18n("Cannot find clip to add marker"), InformationMessage, 500);
+        return;
+    }
     std::shared_ptr<ProjectClip> clip = pCore->bin()->getBinClip(getClipBinId(cid));
     GenTime pos(position - m_model->getClipPosition(cid) + m_model->getClipIn(cid), pCore->getCurrentFps());
     clip->getMarkerModel()->editMarkerGui(pos, qApp->activeWindow(), true, clip.get());
@@ -602,6 +610,10 @@ void TimelineController::addMarker(int cid, int position)
 void TimelineController::addQuickMarker(int cid, int position)
 {
     Q_ASSERT(m_model->isClip(cid));
+    if (position < m_model->getClipPosition(cid) || position > (m_model->getClipPosition(cid) + m_model->getClipPlaytime(cid))) {
+        pCore->displayMessage(i18n("Cannot find clip to add marker"), InformationMessage, 500);
+        return;
+    }
     std::shared_ptr<ProjectClip> clip = pCore->bin()->getBinClip(getClipBinId(cid));
     GenTime pos(position - m_model->getClipPosition(cid) + m_model->getClipIn(cid), pCore->getCurrentFps());
     CommentedTime marker(pos, pCore->currentDoc()->timecode().getDisplayTimecode(pos, false), KdenliveSettings::default_marker_type());
@@ -611,6 +623,10 @@ void TimelineController::addQuickMarker(int cid, int position)
 void TimelineController::deleteMarker(int cid, int position)
 {
     Q_ASSERT(m_model->isClip(cid));
+    if (position < m_model->getClipPosition(cid) || position > (m_model->getClipPosition(cid) + m_model->getClipPlaytime(cid))) {
+        pCore->displayMessage(i18n("Cannot find clip to remove marker"), InformationMessage, 500);
+        return;
+    }
     std::shared_ptr<ProjectClip> clip = pCore->bin()->getBinClip(getClipBinId(cid));
     GenTime pos(position - m_model->getClipPosition(cid) + m_model->getClipIn(cid), pCore->getCurrentFps());
     clip->getMarkerModel()->removeMarker(pos);