Delete the last frame of a video

[eumagga0x2a]

A tentative fix for ADM_Composer::getLastKeyFramePts:

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..51f2837 100644
--- a/avidemux/common/ADM_editor/src/ADM_edSearch.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edSearch.cpp
@@ -372,12 +372,19 @@ uint64_t    ADM_Composer::getLastKeyFramePts(void)
     int lastSeg=_segments.getNbSegments();
     int seg;
     uint64_t pts,dts;
+    int lastRefFrameInSegment;
     if(!lastSeg) return ADM_NO_PTS;
     for(seg=lastSeg-1;seg>=0;seg--)
     {
           _SEGMENT *s=_segments.getSegment(seg);
           _VIDEOS  *v=_segments.getRefVideo(s->_reference);
-          int nbFrame=v->_nb_video_frames;
+          uint64_t endTimeInRef=(s->_refStartTimeUs)+(s->_durationUs);
+          if(!getFrameNumFromPtsOrBefore(v,endTimeInRef,lastRefFrameInSegment))
+          {
+              ADM_warning("Cannot map the last frame in segment to reference at PTS=%s\n",ADM_us2plain(endTimeInRef));
+              lastRefFrameInSegment=v->_nb_video_frames;
+          }
+          int nbFrame=lastRefFrameInSegment;
           if(!nbFrame) break;

           for(int frame=nbFrame-1;frame>=0;frame--)
@@ -388,12 +395,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");

The idea is to find out the time in reference corresponding to the end of the last segment first, then find out the frame number at or before that PTS in reference, then search for the last keyframe < this frame as it is done currently. (I'd like to add that I am at a loss about declaring nbFrames as int while everywhere else in the source it is uint32_t, I simply preserved it in the patch)

This fixes deleting to the end of a video and makes ACT_End work for me in all SD mpeg2 videos and all HD h264 videos with a single reference video, but doesn't fix Avidemux hanging for a while for yet unknown reason when hitting the End key with appended HD h264 videos I tested so far (it hangs also without the patch, so it doesn't make things worse).

I've attached the patch for the mentioned workaround without a tailored function for truncating too.

[eumagga0x2a]

This fixes deleting to the end of a video and makes ACT_End work for me in all SD mpeg2 videos and all HD h264 videos with a single reference video, but doesn't fix Avidemux hanging for a while for yet unknown reason when hitting the End key with appended HD h264 videos I tested so far (it hangs also without the patch, so it doesn't make things worse).

FYI: The hang seeking to linear time within the segment representing the appended video is specific to VDPAU decoder. ADM_Composer::switchToSegment fails in ADM_Composer::DecodePictureUpToIntra then.

[mean]

I've seen that one too
i.e. you decode from 0 to 10 -> all fails, because some ref frames are missing
BUT if you do it again, it will succeed (recovery time ? you have created fake missing ref frames ?)

That trick might not work with vdpau

Some of the patches are a bit kuldgy, so they fit well within avidemux
But I'm a bit at loss regarding which ones are needed

(hint toward pull request...)

[eumagga0x2a]

I'm very sorry for delaying the overhead with GitHub account etc., I have > 10 patches which are very much in flux in my setup and I don't see how this can be managed with pull requests, especially as dealing with my attempts to contribute requires a lot of babysitting on your part anyway.

The both patches in http://avidemux.org/smif/index.php/topic,16935.msg77382.html#msg77382 are needed to allow deleting a part of the video including the last frame with the B marker at the last frame.

Thank you very much for your patience.

[mean]

I finally understood the issue , and yes it was the classic mistake of mixing reference to a thing and the thing itself
First part committed
Thanks!
It must have been painful to narrow that down.

[eumagga0x2a]

Thank you very much!

Now I'd like to find out if it is necessary to decode anything at the beginning of a segment representing a new reference video if we're looking for a spot much later (e.g. near the end)

[mean]

Depends

[mean]

Normally, unless there is mix betwen I & IDR, the editor will rewind and decode from the previous intra (even if it is not part of the segment) and then go forward til the actual start of the segment

* edit : clarify

[eumagga0x2a]

Thank you. I see now that DecodePictureUpToIntra with VDPAU really reliably succeeds on the second try as you explained before.

edit: fits Window not repainted after navigation very well.

[eumagga0x2a]

Maybe it is worth mentioning that every failed navigation or segment switch attempt with VDPAU is accompanied by a

Code Select Expand

[lavc] Unhandled colorspace: 109 (AV_PIX_FMT_YUV444P10BE=77)

message from ADM_ffmp43.cpp:507 in the console.

[mean]

Probably an error that is wrongly interpreted

[eumagga0x2a]

The following patch fixes ADM_Composer::goToTimeVideo with VDPAU and appended videos for me and seems not to cause any side effects:

Code Select Expand

diff --git a/avidemux/common/ADM_editor/src/ADM_edRender.cpp b/avidemux/common/ADM_editor/src/ADM_edRender.cpp
index 0b29667..5a60f9f 100644
--- a/avidemux/common/ADM_editor/src/ADM_edRender.cpp
+++ b/avidemux/common/ADM_editor/src/ADM_edRender.cpp
@@ -190,8 +190,11 @@ uint32_t seg;
     uint64_t to=segTime+s->_refStartTimeUs;
     if(false==seektoTime(s->_reference,to))
     {
+        if(false==seektoTime(s->_reference,to)) // try twice because the first attempt may fail with VDPAU
+        {
             ADM_warning("Cannot seek to beginning of segment %" PRIu32" at  %" PRIu64" ms\n",seg,to/1000);
             return false;
+        }
     }
     _currentSegment=seg;
     int64_t newTime=(int64_t)v->lastDecodedPts+(int64_t)s->_startTimeUs-(int64_t)s->_refStartTimeUs;


[eumagga0x2a]

The goToTimeVideo workaround committed as [editor] If seek fails, try a second time. It helps with h26* + hw accel, thanks.

WRT other parts of a possible solution or workaround to allow deleting the last frame with marker B at the last frame: Would you like to pursue the path with a custom function (cleaner, but pretty much ado about only a little more than nothing) or the hack resetting the marker B silently to the reported duration of the video?

WRT the reported total duration of the video: I have not looked into this calculation, but ffprobe reports actually the PTS of the last frame as the total duration according to my tests. With total duration beyond the last frame as detected by Avidemux, a workaround is necessary for ACT_GotoMarkB to avoid error when navigating to the B marker upon loading a video. I'd rather start a new topic for this workaround though.

[mean]

There a general design flaw when the video does not start at zero
It might be worth it to fix for good
Patch welcome

[eumagga0x2a]

1. Are you thinking of making all videos start at zero?

2. Are you thinking of making the PTS of the last frame minus PTS of the first frame be the future total duration of a video?

3. Are these infos from demuxer and decoder correct as of now?

In any case such design flaw is not the reason we can't easily cut off the last frame of video now, but the hackish workaround won't work if (2) is the way to go. (2) will make a workaround for readjusting the B marker obsolete though. With (3) both workarounds are viable and the second is necessary.