[Bf-blender-cvs] [4adbe31e2fc] master: Fix: VSE indexer seeking not working correctly

Sebastian Parborg noreply at git.blender.org
Fri Jun 11 14:05:26 CEST 2021


Commit: 4adbe31e2fc98f982aed3d97505513750ec348d4
Author: Sebastian Parborg
Date:   Wed Jun 9 21:03:07 2021 +0200
Branches: master
https://developer.blender.org/rB4adbe31e2fc98f982aed3d97505513750ec348d4

Fix: VSE indexer seeking not working correctly

Because of the added sanity checks in rB14508ef100c9 (D11492), seeking
in proxies would not work correctly any more. This is because it wasn't
working as intended before, but in most cases this wouldn't be
noticeable. However now when the sanity checks are tripped it is very
noticeable that something is wrong

The indexer tried to use dts values for time stamps when we used pts in
our decode functions to get the time positions. This would make it
start in the wrong GOP frames when searching. Now that we enforce no
crossing of GOP frames when decoding after seek, this would lead to
issues.

Now we correctly use pts (or dts if pts is not available) and thus we
don't have any seeking issues because of time stamp format missmatch.

Reviewed By: Richard Antalik

Differential Revision: http://developer.blender.org/D11561

===================================================================

M	intern/ffmpeg/ffmpeg_compat.h
M	source/blender/imbuf/intern/IMB_indexer.h
M	source/blender/imbuf/intern/anim_movie.c
M	source/blender/imbuf/intern/indexer.c

===================================================================

diff --git a/intern/ffmpeg/ffmpeg_compat.h b/intern/ffmpeg/ffmpeg_compat.h
index 0c22cf82688..ce4bfc7d5e2 100644
--- a/intern/ffmpeg/ffmpeg_compat.h
+++ b/intern/ffmpeg/ffmpeg_compat.h
@@ -113,22 +113,23 @@ void av_update_cur_dts(AVFormatContext *s, AVStream *ref_st, int64_t timestamp)
 }
 
 FFMPEG_INLINE
-int64_t av_get_pts_from_frame(AVFormatContext *avctx, AVFrame *picture)
+int64_t timestamp_from_pts_or_dts(int64_t pts, int64_t dts)
 {
-  int64_t pts;
-  pts = picture->pts;
-
-  if (pts == AV_NOPTS_VALUE) {
-    pts = picture->pkt_dts;
-  }
+  /* Some videos do not have any pts values, use dts instead in those cases if
+   * possible. Usually when this happens dts can act as pts because as all frames
+   * should then be presented in their decoded in order. IE pts == dts. */
   if (pts == AV_NOPTS_VALUE) {
-    pts = 0;
+    return dts;
   }
-
-  (void)avctx;
   return pts;
 }
 
+FFMPEG_INLINE
+int64_t av_get_pts_from_frame(AVFrame *picture)
+{
+  return timestamp_from_pts_or_dts(picture->pts, picture->pkt_dts);
+}
+
 /* -------------------------------------------------------------------- */
 /** \name Deinterlace code block
  *
diff --git a/source/blender/imbuf/intern/IMB_indexer.h b/source/blender/imbuf/intern/IMB_indexer.h
index 363ad45e0e6..37309ccc13a 100644
--- a/source/blender/imbuf/intern/IMB_indexer.h
+++ b/source/blender/imbuf/intern/IMB_indexer.h
@@ -49,6 +49,7 @@
 typedef struct anim_index_entry {
   int frameno;
   uint64_t seek_pos;
+  uint64_t seek_pos_pts;
   uint64_t seek_pos_dts;
   uint64_t pts;
 } anim_index_entry;
@@ -77,14 +78,19 @@ typedef struct anim_index_builder {
 } anim_index_builder;
 
 anim_index_builder *IMB_index_builder_create(const char *name);
-void IMB_index_builder_add_entry(
-    anim_index_builder *fp, int frameno, uint64_t seek_pos, uint64_t seek_pos_dts, uint64_t pts);
+void IMB_index_builder_add_entry(anim_index_builder *fp,
+                                 int frameno,
+                                 uint64_t seek_pos,
+                                 uint64_t seek_pos_pts,
+                                 uint64_t seek_pos_dts,
+                                 uint64_t pts);
 
 void IMB_index_builder_proc_frame(anim_index_builder *fp,
                                   unsigned char *buffer,
                                   int data_size,
                                   int frameno,
                                   uint64_t seek_pos,
+                                  uint64_t seek_pos_pts,
                                   uint64_t seek_pos_dts,
                                   uint64_t pts);
 
@@ -92,6 +98,7 @@ void IMB_index_builder_finish(anim_index_builder *fp, int rollback);
 
 struct anim_index *IMB_indexer_open(const char *name);
 uint64_t IMB_indexer_get_seek_pos(struct anim_index *idx, int frame_index);
+uint64_t IMB_indexer_get_seek_pos_pts(struct anim_index *idx, int frame_index);
 uint64_t IMB_indexer_get_seek_pos_dts(struct anim_index *idx, int frame_index);
 
 int IMB_indexer_get_frame_index(struct anim_index *idx, int frameno);
diff --git a/source/blender/imbuf/intern/anim_movie.c b/source/blender/imbuf/intern/anim_movie.c
index b65c3e364db..739ec988121 100644
--- a/source/blender/imbuf/intern/anim_movie.c
+++ b/source/blender/imbuf/intern/anim_movie.c
@@ -924,7 +924,7 @@ static int ffmpeg_decode_video_frame(struct anim *anim)
       anim->pFrameComplete = avcodec_receive_frame(anim->pCodecCtx, anim->pFrame) == 0;
 
       if (anim->pFrameComplete) {
-        anim->cur_pts = av_get_pts_from_frame(anim->pFormatCtx, anim->pFrame);
+        anim->cur_pts = av_get_pts_from_frame(anim->pFrame);
 
         if (anim->pFrame->key_frame) {
           anim->cur_key_frame_pts = anim->cur_pts;
@@ -949,7 +949,7 @@ static int ffmpeg_decode_video_frame(struct anim *anim)
     anim->pFrameComplete = avcodec_receive_frame(anim->pCodecCtx, anim->pFrame) == 0;
 
     if (anim->pFrameComplete) {
-      anim->cur_pts = av_get_pts_from_frame(anim->pFormatCtx, anim->pFrame);
+      anim->cur_pts = av_get_pts_from_frame(anim->pFrame);
 
       if (anim->pFrame->key_frame) {
         anim->cur_key_frame_pts = anim->cur_pts;
@@ -1164,6 +1164,7 @@ static int ffmpeg_generic_seek_workaround(struct anim *anim, int64_t *requested_
       if (anim->cur_packet->stream_index == anim->videoStream) {
         break;
       }
+      av_packet_unref(read_packet);
     }
 
     /* If this packet contains I-frame, exit loop. This should be the frame that we need. */
@@ -1197,14 +1198,17 @@ static int ffmpeg_seek_to_key_frame(struct anim *anim, int position, struct anim
       /* No need to seek, return early. */
       return 0;
     }
+    uint64_t pts;
     uint64_t dts;
 
     pos = IMB_indexer_get_seek_pos(tc_index, new_frame_index);
+    pts = IMB_indexer_get_seek_pos_pts(tc_index, new_frame_index);
     dts = IMB_indexer_get_seek_pos_dts(tc_index, new_frame_index);
 
-    anim->cur_key_frame_pts = pos;
+    anim->cur_key_frame_pts = timestamp_from_pts_or_dts(pts, dts);
 
     av_log(anim->pFormatCtx, AV_LOG_DEBUG, "TC INDEX seek pos = %" PRId64 "\n", pos);
+    av_log(anim->pFormatCtx, AV_LOG_DEBUG, "TC INDEX seek pts = %" PRIu64 "\n", pts);
     av_log(anim->pFormatCtx, AV_LOG_DEBUG, "TC INDEX seek dts = %" PRIu64 "\n", dts);
 
     if (ffmpeg_seek_by_byte(anim->pFormatCtx)) {
@@ -1214,8 +1218,9 @@ static int ffmpeg_seek_to_key_frame(struct anim *anim, int position, struct anim
       av_update_cur_dts(anim->pFormatCtx, v_st, dts);
     }
     else {
-      av_log(anim->pFormatCtx, AV_LOG_DEBUG, "... using DTS pos\n");
-      ret = av_seek_frame(anim->pFormatCtx, anim->videoStream, dts, AVSEEK_FLAG_BACKWARD);
+      av_log(anim->pFormatCtx, AV_LOG_DEBUG, "... using PTS pos\n");
+      ret = av_seek_frame(
+          anim->pFormatCtx, anim->videoStream, anim->cur_key_frame_pts, AVSEEK_FLAG_BACKWARD);
     }
   }
   else {
@@ -1243,24 +1248,29 @@ static int ffmpeg_seek_to_key_frame(struct anim *anim, int position, struct anim
         }
         av_packet_unref(current_gop_start_packet);
       }
-      bool same_gop = current_gop_start_packet->pts == anim->cur_key_frame_pts;
+      int64_t gop_pts = timestamp_from_pts_or_dts(current_gop_start_packet->pts,
+                                                  current_gop_start_packet->dts);
+
+      av_packet_free(&current_gop_start_packet);
+      bool same_gop = gop_pts == anim->cur_key_frame_pts;
 
       if (same_gop && position > anim->cur_position) {
         /* Change back to our old frame position so we can simply continue decoding from there. */
         AVPacket *temp = av_packet_alloc();
         while (av_read_frame(anim->pFormatCtx, temp) >= 0) {
-          if (temp->stream_index == anim->videoStream && temp->pts == anim->cur_packet->pts) {
+          int64_t temp_pts = timestamp_from_pts_or_dts(temp->pts, temp->dts);
+          int64_t cur_pts = timestamp_from_pts_or_dts(anim->cur_packet->pts,
+                                                      anim->cur_packet->dts);
+          if (temp->stream_index == anim->videoStream && temp_pts == cur_pts) {
             break;
           }
           av_packet_unref(temp);
         }
-        av_packet_free(&current_gop_start_packet);
         av_packet_free(&temp);
         return 0;
       }
 
-      anim->cur_key_frame_pts = current_gop_start_packet->pts;
-      av_packet_free(&current_gop_start_packet);
+      anim->cur_key_frame_pts = gop_pts;
       /* Seek back so we are at the correct position after we decoded a frame. */
       av_seek_frame(anim->pFormatCtx, -1, pos, AVSEEK_FLAG_BACKWARD);
     }
@@ -1278,6 +1288,8 @@ static int ffmpeg_seek_to_key_frame(struct anim *anim, int position, struct anim
            pts_to_search,
            ret);
   }
+  /* Flush the internal buffers of ffmpeg. This needs to be done after seeking to avoid decoding
+   * errors. */
   avcodec_flush_buffers(anim->pCodecCtx);
 
   anim->cur_pts = -1;
diff --git a/source/blender/imbuf/intern/indexer.c b/source/blender/imbuf/intern/indexer.c
index a8733da39a7..a530acb15b0 100644
--- a/source/blender/imbuf/intern/indexer.c
+++ b/source/blender/imbuf/intern/indexer.c
@@ -50,7 +50,7 @@
 #  include <libavutil/imgutils.h>
 #endif
 
-static const char magic[] = "BlenMIdx";
+static const char binary_header_str[] = "BlenMIdx";
 static const char temp_ext[] = "_part";
 
 static const int proxy_sizes[] = {IMB_PROXY_25, IMB_PROXY_50, IMB_PROXY_75, IMB_PROXY_100};
@@ -65,7 +65,7 @@ static int tc_types[] = {
 };
 #endif
 
-#define INDEX_FILE_VERSION 1
+#define INDEX_FILE_VERSION 2
 
 /* ----------------------------------------------------------------------
  * - time code index functions
@@ -96,16 +96,25 @@ anim_index_builder *IMB_index_builder_create(const char *name)
     return NULL;
   }
 
-  fprintf(rv->fp, "%s%c%.3d", magic, (ENDIAN_ORDER == B_ENDIAN) ? 'V' : 'v', INDEX_FILE_VERSION);
+  fprintf(rv->fp,
+          "%s%c%.3d",
+          binary_header_str,
+          (ENDIAN_ORDER == B_ENDIAN) ? 'V' : 'v',
+          INDEX_FILE_VERSION);
 
   return rv;
 }
 
-void IMB_index_builder_add_entry(
-    anim_index_builder *fp, int frameno, uint64_t seek_pos, uint64_t seek_pos_dts, uint64_t pts)
+void IMB_index_builder_add_entry(anim_index_builder *fp,
+                                 int frameno,
+                                 uint64_t seek_pos,
+                                 uint64_t seek_pos_pts,
+                                 uint64_t seek_pos_dts,
+                                 uint64_t pts)
 {
   fwrite(&frameno, sizeof(int), 1, fp->fp);
   fwrite(&seek_pos, sizeof(uint64_t), 1, fp->fp);
+  fwrite(&seek_pos_pts, sizeof(uint64_t), 1, fp->fp);
   fwrite(&seek_pos_dts, sizeof(uint64_t), 1, fp->fp);
   fwrite(&pts, sizeof(uint64_t), 1, fp->fp);
 }
@@ -115,6 +124,7 @@ void IMB_index_builder_proc_frame(anim_index_builder *fp,
                                   int data_size,
                                   int frameno,
                                   uint64_t seek_pos,
+                                  uint64_t seek_pos_pts,
                                   uint64_t seek_pos_dts,
                                   uint64_t pts)
 {
@@ -122,13 +132,14 @@ void IMB_index_builder_proc_frame(anim_index_builder *

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list