[Bf-blender-cvs] [7616f4ae577] blender-v2.93-release: Fix: VSE indexer seeking not working correctly

Sebastian Parborg noreply at git.blender.org
Wed Jun 30 09:47:16 CEST 2021


Commit: 7616f4ae57735a66720d440f42c476a256d02350
Author: Sebastian Parborg
Date:   Wed Jun 9 21:03:07 2021 +0200
Branches: blender-v2.93-release
https://developer.blender.org/rB7616f4ae57735a66720d440f42c476a256d02350

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 3f168894cd9..6fefa164dc3 100644
--- a/intern/ffmpeg/ffmpeg_compat.h
+++ b/intern/ffmpeg/ffmpeg_compat.h
@@ -113,25 +113,28 @@ 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;
 }
 
-/* --- Deinterlace code block begin --- */
+FFMPEG_INLINE
+int64_t av_get_pts_from_frame(AVFrame *picture)
+{
+  return timestamp_from_pts_or_dts(picture->pts, picture->pkt_dts);
+}
 
-/* NOTE: The code in this block are from FFmpeg 2.6.4, which is licensed by LGPL. */
+/* -------------------------------------------------------------------- */
+/** \name Deinterlace code block
+ *
+ * NOTE: The code in this block are from FFmpeg 2.6.4, which is licensed by LGPL.
+ * \{ */
 
 #define MAX_NEG_CROP 1024
 
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 21c7df277dd..aa8b59cb98a 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 e1995bf7c3e..da6bceda853 100644
--- a/source/blender/imbuf/intern/indexer.c
+++ b/source/blender/imbuf/intern/indexer.c
@@ -51,7 +51,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};
@@ -66,7 +66,7 @@ static int tc_types[] = {
 };
 #endif
 
-#define INDEX_FILE_VERSION 1
+#define INDEX_FILE_VERSION 2
 
 /* ----------------------------------------------------------------------
  * - time code index functions
@@ -97,16 +97,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);
 }
@@ -116,6 +125,7 @@ void IMB_index_builder_proc_frame(anim_index_builder *fp,
                                   int data_size,
                                   int frameno,
                                   uint64_t 

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list