Delete the last frame of a video

[2739480298184121]

Is it possible to delete the last frame of a video? I noticed that after setting A and B and cutting, frame B doesn't get deleted for some reason, only the frames before it. Copy+pasting a section of the video and pasting at the end of the video just puts it before the last frame, so that doesn't work either. I'm using Avidemux 2.6.12 with an AVI file.

[Jan Gruuthuse]

Old avi? Try with Avidemux 2.5.6 (frame based editor), 2.6 branch is time based to cope with newer codecs. Otherwise try re-encoding if you are using [Copy] for video

[leeyc0]

Is it possible to delete the last frame of a video? I noticed that after setting A and B and cutting, frame B doesn't get deleted for some reason, only the frames before it. Copy+pasting a section of the video and pasting at the end of the video just puts it before the last frame, so that doesn't work either. I'm using Avidemux 2.6.12 with an AVI file.

Just set A, leave B unset. In this way you will able to cut video from A to video end.

[eumagga0x2a]

I dig this thread out because both the issue and the workaround is probably a bug worth fixing. Many videos don't start at zero, the first frame has a small offset. The initial value for B is the total duration of the video which includes this initial offset, but navigating to the last frame (hitting the END key) shows the time without this offset. Setting the B marker now places it before the last frame.

[eumagga0x2a]

The initial value for B is the total duration of the video which includes this initial offset, but navigating to the last frame (hitting the END key) shows the time without this offset. Setting the B marker now places it before the last frame.

Other way round: loading a video and hitting PgDown results in "Error seeking to ........ ms" error dialog where the # of milliseconds is the time corresponding to the initial value of the B marker. This time, returned by ADM_Composer::getVideoDuration, reaches beyond the last frame of the video. Navigating to the last frame goes to the time which should be used as the initial value for B.

If ADM_EditorSegment::removeChunk would be extended to deal with a special case of the B marker being at the last frame of the video, everything would be fine.

[eumagga0x2a]

The following first rough approximation allows ADM_EditorSegment::removeChunk to delete the last frame(s) of a video:

Code Select Expand

diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index 2209c88..2c128e1 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -581,7 +581,16 @@ bool        ADM_EditorSegment::removeChunk(uint64_t from, uint64_t to)
     ADM_info("Start, seg %" PRIu32" Offset :%" PRIu64" ms\n",startSeg,startOffset);
     ADM_info("End  , seg %" PRIu32" Offset :%" PRIu64" ms\n",endSeg,endOffset);
     ListOfSegments tmp=segments;
-   
+
+    // Special case at the end of the video
+    _VIDEOS *v=getRefVideo(0);
+    uint64_t pts=v->firstFramePts;
+    if(pts<50000) pts=50000; // kill the last segment < 50ms for sure
+    bool eov=false;
+    if(to>=getTotalDuration()-pts)
+    {
+        eov=true;
+    }

     if(startSeg==endSeg)
     {
@@ -599,14 +608,21 @@ bool        ADM_EditorSegment::removeChunk(uint64_t from, uint64_t to)
     // 3- Shorten last segment
     _SEGMENT *last=getSegment(endSeg);
     last->_refStartTimeUs+=endOffset;
-    last->_durationUs-=endOffset;
+    if(!eov)
+    {
+        last->_durationUs-=endOffset;
+    }
+    else
+    {
+        last->_durationUs=0;
+    }
     // 2- Kill the segment in between
     for(int i=startSeg+1;i<endSeg;i++)
     {
         segments.erase(segments.begin()+startSeg+1);
     }
-    updateStartTime();
     removeEmptySegments();
+    updateStartTime();
     if(isEmpty())
     {
         GUI_Error_HIG(QT_TRANSLATE_NOOP("adm","Error"),QT_TRANSLATE_NOOP("adm","You cannot remove *all* the video\n"));

The currently necessary hack with minimum value of 50ms for pts is terrible, it should really find out and use the PTS of the last frame instead.

[mean]

I'll need a bit of time to check that one

[eumagga0x2a]

Unfortunately, the first part of the patch ââ,¬â€œ determining if we are going to delete everything past the marker A ââ,¬â€œ can't go into production. I already ran into enough videos with total duration much higher than the PTS of the last frame (160ms max. difference so far) with a very small non-zero start PTS, so assuming that 50ms might be enough is wrong.

[eumagga0x2a]

It looks for me like there is no easy way to teach ADM_EditorSegment::removeChunk to decide on its own whether it should remove a chunk or rather truncate a video. The logic would have to move a level or two higher then and probably require a custom function.

With regard to the nasty error when trying to go to marker B value automatically set after loading a video, I would suggest to work around the issue via something like

Code Select Expand

diff --git a/avidemux/common/gui_navigate.cpp b/avidemux/common/gui_navigate.cpp
index 23df368..29e6c0c 100644
--- a/avidemux/common/gui_navigate.cpp
+++ b/avidemux/common/gui_navigate.cpp
@@ -90,8 +93,26 @@ static int ignore_change=0;
                 uint64_t pts;
                 if(action==ACT_GotoMarkA) pts=video_body->getMarkerAPts();
                         else  pts=video_body->getMarkerBPts();
-                GUI_GoToTime(pts);
-                 
+                if(false==video_body->goToTimeVideo(pts))
+                {
+                    if(action==ACT_GotoMarkA)
+                    {
+                        ADM_warning("Go to Marker A: Seek to time %s ms failed\n",ADM_us2plain(pts));
+                    }else // PTS returned by getMarkerBPts() may be beyond the last frame.
+                          // Go to the last frame then.
+                    {
+                        pts=video_body->getLastKeyFramePts();
+                        if(pts==ADM_NO_PTS) break;           
+                        admPreview::deferDisplay(1);
+                        GUI_GoToTime(pts);
+                        while(admPreview::nextPicture())
+                        {
+                        }
+                        admPreview::deferDisplay(0);
+                    }
+                }
+                admPreview::samePicture();
+                GUI_setCurrentFrameAndTime();
             }
             break;


in gui_navigate.cpp. The present GUI_GoToTime function can't be used in the test here because it always returns true. The rest is copypasted from the ACT_End action.

[eumagga0x2a]

The first of the two patches adds a function ADM_Composer::truncate and the second one extends the logic of ACT_delete to handle the special case of the B marker being at or beyond the last frame and to choose between remove(from,to) and truncate(from) accordingly.

The only but important issue I have found so far is that getLastKeyFramePts still somehow sees and reports last keyframe beyond the end of the truncated video, messing up functions like ACT_End after truncating. It means, the patches are not yet ripe for production. Apart from that, they work for me.

Code Select Expand

diff --git a/avidemux/common/ADM_editor/include/ADM_edit.hxx b/avidemux/common/ADM_editor/include/ADM_edit.hxx
index 3a29198..5c18f16 100644
--- a/avidemux/common/ADM_editor/include/ADM_edit.hxx
+++ b/avidemux/common/ADM_editor/include/ADM_edit.hxx
@@ -275,6 +275,7 @@ public:
/******************************* /Post Processing ************************************/
/******************************* Editing ************************************/
                     bool                remove(uint64_t start,uint64_t end);
+                    bool                truncate(uint64_t start);
                     bool                addSegment(uint32_t ref, uint64_t startRef, uint64_t duration);
                     bool                clearSegment(void);
                     uint32_t            getNbSegment(void)
diff --git a/avidemux/common/ADM_editor/include/ADM_segment.h b/avidemux/common/ADM_editor/include/ADM_segment.h
index c6f16f5..ce3d37a 100644
--- a/avidemux/common/ADM_editor/include/ADM_segment.h
+++ b/avidemux/common/ADM_editor/include/ADM_segment.h
@@ -189,6 +189,7 @@ public:
             bool        isKeyFrameByTime(uint32_t refVideo,uint64_t seekTime);

             bool        removeChunk(uint64_t from, uint64_t to);
+            bool        truncateVideo(uint64_t from);
             bool        dtsFromPts(uint32_t refVideo,uint64_t pts,uint64_t *dts);

             bool        LinearToRefTime(int segNo,uint64_t linear,uint64_t *refTime);
diff --git a/avidemux/common/ADM_editor/src/ADM_edit.cpp b/avidemux/common/ADM_editor/src/ADM_edit.cpp
index dab497f..9268698 100644
--- a/avidemux/common/ADM_editor/src/ADM_edit.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edit.cpp
@@ -682,6 +682,14 @@ bool            ADM_Composer::remove(uint64_t start,uint64_t end)
     return _segments.removeChunk(start,end);
}
/**
+    \fn truncate
+    \brief Truncate video
+*/
+bool ADM_Composer::truncate(uint64_t start)
+{
+    return _segments.truncateVideo(start);
+}
+/**
     \fn dumpEditing
     \brief Dump segment, video & al
*/
diff --git a/avidemux/common/ADM_editor/src/ADM_segment.cpp b/avidemux/common/ADM_editor/src/ADM_segment.cpp
index 2209c88..b100f6f 100644
--- a/avidemux/common/ADM_editor/src/ADM_segment.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_segment.cpp
@@ -619,8 +619,48 @@ bool        ADM_EditorSegment::removeChunk(uint64_t from, uint64_t to)
     dump();
     return true;
}
+/**
+    \fn truncateVideo
+    \brief Remove a part of the video from the given time to the end
+*/
+bool ADM_EditorSegment::truncateVideo(uint64_t from)
+{
+    uint32_t startSeg;
+    uint64_t startOffset;

+    ADM_info("Truncating from %" PRIu64" ms\n",from/1000);
+    dump();
+    if(false==convertLinearTimeToSeg(from,&startSeg,&startOffset))
+    {
+        ADM_warning("Cannot get starting point for linear time %" PRIu64" ms\n",from/1000);
+        return false;
+    }

+    ADM_info("Start in segment %" PRIu32" at offset :%" PRIu64" ms\n",startSeg,startOffset);
+    ListOfSegments tmp=segments;
+
+    _SEGMENT *first=getSegment(startSeg);
+    // shorten the start segment
+    first->_durationUs=startOffset;
+    // remove following segments
+    int n=segments.size();
+    for(int i=startSeg+1;i<n;i++)
+    {
+        segments.erase(segments.begin()+startSeg+1);
+    }
+    updateStartTime();
+    removeEmptySegments();
+    if(isEmpty())
+    {
+        GUI_Error_HIG(QT_TRANSLATE_NOOP("adm","Error"),QT_TRANSLATE_NOOP("adm","You cannot remove *all* the video\n"));
+        segments=tmp;
+        updateStartTime();
+        return false;
+    }
+    undoSegments.push_back(tmp);
+    dump();
+    return true;
+}
/**
     \fn dump
     \brief Dump the segment content

Code Select Expand

diff --git a/avidemux/common/gui_main.cpp b/avidemux/common/gui_main.cpp
index 1a73404..d72045f 100644
--- a/avidemux/common/gui_main.cpp
+++ b/avidemux/common/gui_main.cpp
@@ -560,7 +560,24 @@ void HandleAction (Action action)
                 video_body->copyToClipBoard(a,b);
             }

-            if(false==video_body->remove(a,b))
+            // Special case of B being at or beyond the last frame
+            bool atTheEnd=false;
+            uint64_t pts=video_body->getLastKeyFramePts();
+            if(pts!=ADM_NO_PTS)
+            {
+                if(b>=pts) // don't waste time if B is before the last keyframe
+                {
+                    admPreview::deferDisplay(1);
+                    GUI_GoToTime(pts);
+                    while(admPreview::nextPicture())
+                    {
+                    }
+                    admPreview::deferDisplay(0);
+                    uint64_t lastFramePts=video_body->getCurrentFramePts();
+                    if(b>=lastFramePts) atTheEnd=true; // B is at or beyond the last frame
+                }
+            }
+            if((atTheEnd==true && false==video_body->truncate(a)) || (atTheEnd==false && false==video_body->remove(a,b)))
             {
                 GUI_Error_HIG(QT_TRANSLATE_NOOP("adm","Cutting"),QT_TRANSLATE_NOOP("adm","Error while cutting out."));
                 break;

[mean]

I'm not sure i get that line
first->_durationUs=startOffset;

[eumagga0x2a]

Thank you for looking at the patch. This part is copypasted from ADM_EditorSegment::removeChunk straight above and says that the segment where the marker A happens to be should not extend beyond this point (represented by startOffset), the duration of the segment must be equal the marker A offset in relation to the beginning of this segment.

I still haven't understood why ADM_Composer::getLastKeyFramePts gets messed up once ADM_EditorSegment::truncateVideo has been executed (it returns the same PTS as before truncating).

[mean]

The thing is convertLinear will give the the segment + offset inside the segment of the new "end"
That segment duration will be equal to the offset only if the segment is taken from its beginning

Example :
Linear  0..10  => actually it's segment 0 from 10 to 20

Truncate ( => segment 0, end point = 18

Duration=18

Where we want to adjust the duration i.e. correct duration=8

(Unless i'm not full awake)

[eumagga0x2a]

The thing is convertLinear will give the the segment + offset inside the segment of the new "end"
That segment duration will be equal to the offset only if the segment is taken from its beginning

Example :
Linear  0..10  => actually it's segment 0 from 10 to 20

Wouldn't it imply _startTimeUs, which is declared as uint64_t, going negative? In my very limited understanding the example above may apply to time in reference (_refStartTimeUs) which can be e.g. 10 at linear 0 while time in the first segment must be 0 at linear 0. I do see question marks in

Code Select Expand

    if(!frameTime && segments.size()) // pick the first one
    {
        ADM_info("Frame time=0, taking first segment \n");
        *seg=0;
        *segTime=0; // ??
        return true;
    }

in the body of ADM_EditorSegment::convertLinearTimeToSeg at ADM_segment.cpp:450 though...

In any case the current removeChunk does exactly the same for the segment containing the A marker apparently without any issues.

Truncate ( => segment 0, end point = 18

Duration=18

Where we want to adjust the duration i.e. correct duration=8

In my again extremely limited understanding there is no need to safeguard for time in segment > linear.

The real problem seems to exist in ADM_Composer::getLastKeyFramePts, because it currently can't handle the case of end of the video != the end of the reference video (apart from misleading ADM_info which doesn't report what the function actually returns). WRT to the info printed to the console I use currently a private patch

Code Select Expand

diff --git a/avidemux/common/ADM_editor/src/ADM_edSearch.cpp b/avidemux/common/ADM_editor/src/ADM_edSearch.cpp
index df4385f..0edbe05 100644
--- a/avidemux/common/ADM_editor/src/ADM_edSearch.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edSearch.cpp
@@ -388,12 +388,14 @@ uint64_t    ADM_Composer::getLastKeyFramePts(void)
               if(!(flags & AVI_KEY_FRAME)) continue;
               v->_aviheader->getPtsDts(frame,&pts,&dts);
               if(pts==ADM_NO_PTS) continue;         
-              ADM_info("found last keyframe at %d, time=%s\n",frame,ADM_us2plain(pts));
+              ADM_info("found last keyframe at %d, time in reference=%s\n",frame,ADM_us2plain(pts));
               break;
           }
           if(pts!=ADM_NO_PTS)
           {
-              return pts+s->_startTimeUs-s->_refStartTimeUs;
+              pts+=s->_startTimeUs-s->_refStartTimeUs;
+              ADM_info("found last keyframe at %s\n",ADM_us2plain(pts));
+              return pts;
           }
      }
     ADM_info("Cannot find lastKeyFrame with a valid PTS\n");

Dropping the idea of a tailored function for truncating video and simply using

Code Select Expand

diff --git a/avidemux/common/gui_main.cpp b/avidemux/common/gui_main.cpp
index 1a73404..ef29170 100644
--- a/avidemux/common/gui_main.cpp
+++ b/avidemux/common/gui_main.cpp
@@ -560,6 +560,20 @@ void HandleAction (Action action)
                 video_body->copyToClipBoard(a,b);
             }

+            // Special case of B being at the last frame
+            uint64_t pts=video_body->getLastKeyFramePts();
+            if(pts!=ADM_NO_PTS && b>=pts) // don't waste time if B is before the last keyframe
+            {
+                admPreview::deferDisplay(1);
+                GUI_GoToTime(pts);
+                while(admPreview::nextPicture())
+                {
+                }
+                admPreview::deferDisplay(0);
+                uint64_t lastFramePts=video_body->getCurrentFramePts();
+                // put B beyond the last frame to ensure that it will be removed too
+                if(b>=lastFramePts) b=video_body->getVideoDuration();
+            }
             if(false==video_body->remove(a,b))
             {
                 GUI_Error_HIG(QT_TRANSLATE_NOOP("adm","Cutting"),QT_TRANSLATE_NOOP("adm","Error while cutting out."));

(edited: nonsensical nested ifs in the patch removed, sorry) to handle the case when B is at the last frame results in the same issues originating from the current implementation of ADM_Composer::getLastKeyFramePts.

[mean]

After rechecking, you are right
Sorry for that