[Editor] Handle pasting from clipboard to the end of video better

Started by eumagga0x2a, October 19, 2016, 05:20:52 PM

Previous topic - Next topic

eumagga0x2a

When pasting from clipboard to the end (to the last frame) of a video, this last frame ends all alone in a new segment past the pasted content of the clipboard. This doesn't look like something a user could want.

While implementing a true append from clipboard functionality turned out to be a major pain (you can't just add something like

/**
* \fn appendFromClipBoard
* \brief append clipboard at the end of video
* @return
*/
bool ADM_EditorSegment::appendFromClipBoard(void)
{
    aprintf("Appending from clipboard\n");
    ListOfSegments tmp=segments;
    for(int i=0;i<clipboard.size();i++) segments.push_back(clipboard[i]);
    updateStartTime();
    undoSegments.push_back(tmp);
    dump();
    return true;
}


to ADM_segment.cpp because you get a perceptible gap between the last frame and the beginning of the appended clipboard content due to total duration of video extending way beyond the last frame), it is easy to discard the lonely last frame automatically. I'd suggest to change the API of ADM_EditorSegment::pasteFromClipBoard for this purpose. It is still not a perfect append functionality because the last frame gets lost, but it would be IMHO better than what we have now.

diff --git a/avidemux/common/ADM_editor/include/ADM_edit.hxx b/avidemux/common/ADM_editor/include/ADM_edit.hxx
index 5c18f16..b0122e4 100644
--- a/avidemux/common/ADM_editor/include/ADM_edit.hxx
+++ b/avidemux/common/ADM_editor/include/ADM_edit.hxx
@@ -198,7 +198,7 @@ virtual                         ~ADM_Composer();
                                 }
                     uint8_t     resetSeg( void );
                     bool        copyToClipBoard(uint64_t startTime, uint64_t endTime);
-                    bool        pasteFromClipBoard(uint64_t currentTime);
+                    bool        pasteFromClipBoard(uint64_t currentTime, bool append);
                     bool      addFile (const char *name);
int         appendFile(const char *name);
void closeFile(void);
diff --git a/avidemux/common/ADM_editor/include/ADM_segment.h b/avidemux/common/ADM_editor/include/ADM_segment.h
index ce3d37a..76b1714 100644
--- a/avidemux/common/ADM_editor/include/ADM_segment.h
+++ b/avidemux/common/ADM_editor/include/ADM_segment.h
@@ -195,7 +195,7 @@ public:
             bool        LinearToRefTime(int segNo,uint64_t linear,uint64_t *refTime);
             
             bool        copyToClipBoard(uint64_t startTime, uint64_t endTime);
-            bool        pasteFromClipBoard(uint64_t currentTime);
+            bool        pasteFromClipBoard(uint64_t currentTime, bool append);
             bool        dumpClipBoard();
protected:
             void        dumpSegmentsInternal(ListOfSegments &l);
diff --git a/avidemux/common/ADM_editor/src/ADM_edit.cpp b/avidemux/common/ADM_editor/src/ADM_edit.cpp
index 9268698..8a1b915 100644
--- a/avidemux/common/ADM_editor/src/ADM_edit.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edit.cpp
@@ -76,9 +76,9 @@ bool        ADM_Composer::copyToClipBoard(uint64_t startTime, uint64_t endTime)
     return _segments.copyToClipBoard(startTime,endTime);
     
}
-bool        ADM_Composer::pasteFromClipBoard(uint64_t currentTime)
+bool        ADM_Composer::pasteFromClipBoard(uint64_t currentTime, bool append)
{
-    return _segments.pasteFromClipBoard(currentTime);
+    return _segments.pasteFromClipBoard(currentTime,append);
     
}

diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index eda4d62..b7066be 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -886,13 +886,19 @@ bool        ADM_EditorSegment::dumpClipBoard()

/**
  * \fn pasteFromClipBoard
- * \brief instert clipboard at currentTime position
+ * \brief insert or append clipboard at currentTime position
  * @param currentTime
  * @return
  */
-bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime)
+bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime, bool append)
{
-    ADM_info("Pasting from clipboard to %s\n",ADM_us2plain(currentTime));
+    if(append)
+    {
+        ADM_info("Appending from clipboard to %s\n",ADM_us2plain(currentTime));
+    }else
+    {
+        ADM_info("Pasting from clipboard to %s\n",ADM_us2plain(currentTime));
+    }
     uint32_t startSeg;
     uint64_t startSegTime;
     convertLinearTimeToSeg(  currentTime, &startSeg,&startSegTime);   
@@ -919,6 +925,7 @@ bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime)
                  post._durationUs-=offset;
                  newSegs.push_back(pre);
                  for(int j=0;j<clipboard.size();j++) newSegs.push_back(clipboard[j]);
+                 if(append) break; // discard the rest
                  newSegs.push_back(post);
                  continue;
             }
diff --git a/avidemux/common/gui_main.cpp b/avidemux/common/gui_main.cpp
index bef7e7d..57d4aa3 100644
--- a/avidemux/common/gui_main.cpp
+++ b/avidemux/common/gui_main.cpp
@@ -497,7 +497,16 @@ void HandleAction (Action action)
                   markA=markB;
                   markB=p;
               }
-              video_body->pasteFromClipBoard(currentPts);
+              // Special case if we are at the last frame
+              bool lastFrame=false;
+              uint64_t pts=video_body->getLastKeyFramePts();
+              if(pts!=ADM_NO_PTS && currentPts>=pts) // save time if we are not at or beyond the last keyframe
+              {
+                  GUI_infiniteForward(pts);
+                  uint64_t lastFramePts=video_body->getCurrentFramePts();
+                  if(currentPts==lastFramePts) lastFrame=true;
+              }
+              video_body->pasteFromClipBoard(currentPts,lastFrame);
               video_body->getVideoInfo (avifileinfo);
               d=video_body->getVideoDuration()-d;
               if(markA>=currentPts)


By the way, as already noticed, clipboard survives closing a video, but no checks ensure the format compatibility. On one hand, an ability to copy a chunk of a video to clipboard, close this video, load a new one and paste a part of the previous video looks like a valuable feature. On the other, this may lead to trouble if formats don't match. Any ideas?

mean

1- Let me look at it
2- Better to purge the clipboard also. Else it might point to orpheaned video

eumagga0x2a

To 2.: Here comes the patch above enhanced by a) clear clipboard on close and 2) do not split segment when clipboard is empty. My apologies for this all-in-one approach instead of isolated, stacked patches.

diff --git a/avidemux/common/ADM_editor/include/ADM_edit.hxx b/avidemux/common/ADM_editor/include/ADM_edit.hxx
index 5c18f16..b0122e4 100644
--- a/avidemux/common/ADM_editor/include/ADM_edit.hxx
+++ b/avidemux/common/ADM_editor/include/ADM_edit.hxx
@@ -198,7 +198,7 @@ virtual                         ~ADM_Composer();
                                 }
                     uint8_t     resetSeg( void );
                     bool        copyToClipBoard(uint64_t startTime, uint64_t endTime);
-                    bool        pasteFromClipBoard(uint64_t currentTime);
+                    bool        pasteFromClipBoard(uint64_t currentTime, bool append);
                     bool      addFile (const char *name);
int         appendFile(const char *name);
void closeFile(void);
diff --git a/avidemux/common/ADM_editor/include/ADM_segment.h b/avidemux/common/ADM_editor/include/ADM_segment.h
index ce3d37a..76b1714 100644
--- a/avidemux/common/ADM_editor/include/ADM_segment.h
+++ b/avidemux/common/ADM_editor/include/ADM_segment.h
@@ -195,7 +195,7 @@ public:
             bool        LinearToRefTime(int segNo,uint64_t linear,uint64_t *refTime);
             
             bool        copyToClipBoard(uint64_t startTime, uint64_t endTime);
-            bool        pasteFromClipBoard(uint64_t currentTime);
+            bool        pasteFromClipBoard(uint64_t currentTime, bool append);
             bool        dumpClipBoard();
protected:
             void        dumpSegmentsInternal(ListOfSegments &l);
diff --git a/avidemux/common/ADM_editor/src/ADM_edit.cpp b/avidemux/common/ADM_editor/src/ADM_edit.cpp
index 9268698..8a1b915 100644
--- a/avidemux/common/ADM_editor/src/ADM_edit.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edit.cpp
@@ -76,9 +76,9 @@ bool        ADM_Composer::copyToClipBoard(uint64_t startTime, uint64_t endTime)
     return _segments.copyToClipBoard(startTime,endTime);
     
}
-bool        ADM_Composer::pasteFromClipBoard(uint64_t currentTime)
+bool        ADM_Composer::pasteFromClipBoard(uint64_t currentTime, bool append)
{
-    return _segments.pasteFromClipBoard(currentTime);
+    return _segments.pasteFromClipBoard(currentTime,append);
     
}

diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index eda4d62..bf9f213 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -250,6 +250,7 @@ bool ADM_EditorSegment::deleteAll (void)

     }

+    clipboard.clear();
     videos.clear();
     segments.clear();
     undoSegments.clear();
@@ -886,13 +887,20 @@ bool        ADM_EditorSegment::dumpClipBoard()

/**
  * \fn pasteFromClipBoard
- * \brief instert clipboard at currentTime position
+ * \brief insert or append clipboard at currentTime position
  * @param currentTime
  * @return
  */
-bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime)
+bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime, bool append)
{
-    ADM_info("Pasting from clipboard to %s\n",ADM_us2plain(currentTime));
+    if(!clipboard.size()) return true;
+    if(append)
+    {
+        ADM_info("Appending from clipboard to %s\n",ADM_us2plain(currentTime));
+    }else
+    {
+        ADM_info("Pasting from clipboard to %s\n",ADM_us2plain(currentTime));
+    }
     uint32_t startSeg;
     uint64_t startSegTime;
     convertLinearTimeToSeg(  currentTime, &startSeg,&startSegTime);   
@@ -919,6 +927,7 @@ bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime)
                  post._durationUs-=offset;
                  newSegs.push_back(pre);
                  for(int j=0;j<clipboard.size();j++) newSegs.push_back(clipboard[j]);
+                 if(append) break; // discard the rest
                  newSegs.push_back(post);
                  continue;
             }
diff --git a/avidemux/common/gui_main.cpp b/avidemux/common/gui_main.cpp
index bef7e7d..57d4aa3 100644
--- a/avidemux/common/gui_main.cpp
+++ b/avidemux/common/gui_main.cpp
@@ -497,7 +497,16 @@ void HandleAction (Action action)
                   markA=markB;
                   markB=p;
               }
-              video_body->pasteFromClipBoard(currentPts);
+              // Special case if we are at the last frame
+              bool lastFrame=false;
+              uint64_t pts=video_body->getLastKeyFramePts();
+              if(pts!=ADM_NO_PTS && currentPts>=pts) // save time if we are not at or beyond the last keyframe
+              {
+                  GUI_infiniteForward(pts);
+                  uint64_t lastFramePts=video_body->getCurrentFramePts();
+                  if(currentPts==lastFramePts) lastFrame=true;
+              }
+              video_body->pasteFromClipBoard(currentPts,lastFrame);
               video_body->getVideoInfo (avifileinfo);
               d=video_body->getVideoDuration()-d;
               if(markA>=currentPts)


eumagga0x2a

In case you are not at ease with the API change or need more time (I'd be okay with a dedicated function as well, the approach outlined in http://avidemux.org/smif/index.php/topic,17223.0.html has exactly the same effect WRT to the post-last-frame gap as appending a video), I have isolated the easy bits (and added an info message), which could be pushed ahead of more intrusive changes.

a) Clear clipboard when closing a video:

diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index eda4d62..daf8a2c 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -250,6 +250,7 @@ bool ADM_EditorSegment::deleteAll (void)

     }

+    clipboard.clear();
     videos.clear();
     segments.clear();
     undoSegments.clear();


b) Do not split segment on paste when the clipboard is empty:

diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index eda4d62..e6bd728 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -893,6 +893,11 @@ bool        ADM_EditorSegment::dumpClipBoard()
  */
bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime)
{
+    if(!clipboard.size())
+    {
+        ADM_info("The clipboard is empty, nothing to do\n");
+        return true;
+    }
     ADM_info("Pasting from clipboard to %s\n",ADM_us2plain(currentTime));
     uint32_t startSeg;
     uint64_t startSegTime;


The offset in the second patch assumes that the first patch has been already applied.

Edit: now with patches attached, oops.
Edit again: the second patch with the right offset.

eumagga0x2a

Here comes an alternative approach introducing a new function ADM_EditorSegment::appendFromClipBoard and not altering pasteFromClipboard to choose from:

diff --git a/avidemux/common/ADM_editor/include/ADM_edit.hxx b/avidemux/common/ADM_editor/include/ADM_edit.hxx
index 5c18f16..a3f965c 100644
--- a/avidemux/common/ADM_editor/include/ADM_edit.hxx
+++ b/avidemux/common/ADM_editor/include/ADM_edit.hxx
@@ -199,6 +199,7 @@ virtual                         ~ADM_Composer();
                     uint8_t     resetSeg( void );
                     bool        copyToClipBoard(uint64_t startTime, uint64_t endTime);
                     bool        pasteFromClipBoard(uint64_t currentTime);
+                    bool        appendFromClipBoard(void);
                     bool      addFile (const char *name);
int         appendFile(const char *name);
void closeFile(void);
diff --git a/avidemux/common/ADM_editor/include/ADM_segment.h b/avidemux/common/ADM_editor/include/ADM_segment.h
index ce3d37a..67b498c 100644
--- a/avidemux/common/ADM_editor/include/ADM_segment.h
+++ b/avidemux/common/ADM_editor/include/ADM_segment.h
@@ -196,6 +196,7 @@ public:
             
             bool        copyToClipBoard(uint64_t startTime, uint64_t endTime);
             bool        pasteFromClipBoard(uint64_t currentTime);
+            bool        appendFromClipBoard(void);
             bool        dumpClipBoard();
protected:
             void        dumpSegmentsInternal(ListOfSegments &l);
diff --git a/avidemux/common/ADM_editor/src/ADM_edit.cpp b/avidemux/common/ADM_editor/src/ADM_edit.cpp
index 9268698..21ce23b 100644
--- a/avidemux/common/ADM_editor/src/ADM_edit.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edit.cpp
@@ -81,6 +81,14 @@ bool        ADM_Composer::pasteFromClipBoard(uint64_t currentTime)
     return _segments.pasteFromClipBoard(currentTime);
     
}
+/**
+    \fn appendFromClipBoard
+    \brief Append the content of the clipboard to the end of the video
+*/
+bool ADM_Composer::appendFromClipBoard(void)
+{
+    return _segments.appendFromClipBoard();
+}

/**
     \fn resetSeg
diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index eda4d62..b1d8fb9 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -939,4 +939,20 @@ bool        ADM_EditorSegment::pasteFromClipBoard(uint64_t currentTime)
     return true;
}

+/**
+ * \fn appendFromClipBoard
+ * \brief append clipboard at the end of video
+ * @return
+ */
+bool ADM_EditorSegment::appendFromClipBoard(void)
+{
+    ADM_info("Appending from clipboard\n");
+    ListOfSegments tmp=segments;
+    for(int i=0;i<clipboard.size();i++) segments.push_back(clipboard[i]);
+    updateStartTime();
+    undoSegments.push_back(tmp);
+    dump();
+    return true;
+}
+
//EOF
diff --git a/avidemux/common/gui_main.cpp b/avidemux/common/gui_main.cpp
index bef7e7d..ff1f664 100644
--- a/avidemux/common/gui_main.cpp
+++ b/avidemux/common/gui_main.cpp
@@ -497,7 +497,22 @@ void HandleAction (Action action)
                   markA=markB;
                   markB=p;
               }
-              video_body->pasteFromClipBoard(currentPts);
+              // Special case if we are at the last frame
+              bool lastFrame=false;
+              uint64_t pts=video_body->getLastKeyFramePts();
+              if(pts!=ADM_NO_PTS && currentPts>=pts) // save time if we are not at or beyond the last keyframe
+              {
+                  GUI_infiniteForward(pts);
+                  uint64_t lastFramePts=video_body->getCurrentFramePts();
+                  if(currentPts==lastFramePts) lastFrame=true;
+              }
+              if(lastFrame)
+              {
+                  video_body->appendFromClipBoard();
+              }else
+              {
+                  video_body->pasteFromClipBoard(currentPts);
+              }
               video_body->getVideoInfo (avifileinfo);
               d=video_body->getVideoDuration()-d;
               if(markA>=currentPts)


Edit: I'm sorry, I've minimally modified the original version adding dump(); to get useful debug output and replacing aprintf with ADM_info.

eumagga0x2a

Thank you for taking the last patch ([Editor] Fix pasting clipboard at the end , through append function (euma)).

I just wanted to remind that the both small patches clear-clipboard-on-close.patch and do-not-paste-empty-clipboard.patch from http://avidemux.org/smif/index.php/topic,17223.msg77640.html#msg77640 should be included as well, because they are really needed (unless you discover that there is something wrong with them I have missed).

Without the second patch, every time Ctrl+V is pressed, the current segment gets split in two, even with an empty clipboard.

edit: commit reference added.

mean