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

1 files changed, 54 insertions, 20 deletions

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 *fp,
     anim_index_entry e;
     e.frameno = frameno;
     e.seek_pos = seek_pos;
+    e.seek_pos_pts = seek_pos_pts;
     e.seek_pos_dts = seek_pos_dts;
     e.pts = pts;
 
     fp->proc_frame(fp, buffer, data_size, &e);
   }
   else {
-    IMB_index_builder_add_entry(fp, frameno, seek_pos, seek_pos_dts, pts);
+    IMB_index_builder_add_entry(fp, frameno, seek_pos, seek_pos_pts, seek_pos_dts, pts);
   }
 }
 
@@ -159,22 +170,26 @@ struct anim_index *IMB_indexer_open(const char *name)
   int i;
 
   if (!fp) {
+    fprintf(stderr, "Couldn't open indexer file: %s\n", name);
     return NULL;
   }
 
   if (fread(header, 12, 1, fp) != 1) {
+    fprintf(stderr, "Couldn't read indexer file: %s\n", name);
     fclose(fp);
     return NULL;
   }
 
   header[12] = 0;
 
-  if (memcmp(header, magic, 8) != 0) {
+  if (memcmp(header, binary_header_str, 8) != 0) {
+    fprintf(stderr, "Error reading %s: Binary file type string missmatch\n", name);
     fclose(fp);
     return NULL;
   }
 
   if (atoi(header + 9) != INDEX_FILE_VERSION) {
+    fprintf(stderr, "Error reading %s: File version missmatch\n", name);
     fclose(fp);
     return NULL;
   }
@@ -187,6 +202,7 @@ struct anim_index *IMB_indexer_open(const char *name)
 
   idx->num_entries = (ftell(fp) - 12) / (sizeof(int) +      /* framepos */
                                          sizeof(uint64_t) + /* seek_pos */
+                                         sizeof(uint64_t) + /* seek_pos_pts */
                                          sizeof(uint64_t) + /* seek_pos_dts */
                                          sizeof(uint64_t)   /* pts */
                                         );
@@ -200,12 +216,13 @@ struct anim_index *IMB_indexer_open(const char *name)
   for (i = 0; i < idx->num_entries; i++) {
     items_read += fread(&idx->entries[i].frameno, sizeof(int), 1, fp);
     items_read += fread(&idx->entries[i].seek_pos, sizeof(uint64_t), 1, fp);
+    items_read += fread(&idx->entries[i].seek_pos_pts, sizeof(uint64_t), 1, fp);
     items_read += fread(&idx->entries[i].seek_pos_dts, sizeof(uint64_t), 1, fp);
     items_read += fread(&idx->entries[i].pts, sizeof(uint64_t), 1, fp);
   }
 
-  if (UNLIKELY(items_read != idx->num_entries * 4)) {
-    perror("error reading animation index file");
+  if (UNLIKELY(items_read != idx->num_entries * 5)) {
+    fprintf(stderr, "Error: Element data size missmatch in: %s\n", name);
     MEM_freeN(idx->entries);
     MEM_freeN(idx);
     fclose(fp);
@@ -216,6 +233,7 @@ struct anim_index *IMB_indexer_open(const char *name)
     for (i = 0; i < idx->num_entries; i++) {
       BLI_endian_switch_int32(&idx->entries[i].frameno);
       BLI_endian_switch_uint64(&idx->entries[i].seek_pos);
+      BLI_endian_switch_uint64(&idx->entries[i].seek_pos_pts);
       BLI_endian_switch_uint64(&idx->entries[i].seek_pos_dts);
       BLI_endian_switch_uint64(&idx->entries[i].pts);
     }
@@ -237,6 +255,17 @@ uint64_t IMB_indexer_get_seek_pos(struct anim_index *idx, int frame_index)
   return idx->entries[frame_index].seek_pos;
 }
 
+uint64_t IMB_indexer_get_seek_pos_pts(struct anim_index *idx, int frame_index)
+{
+  if (frame_index < 0) {
+    frame_index = 0;
+  }
+  if (frame_index >= idx->num_entries) {
+    frame_index = idx->num_entries - 1;
+  }
+  return idx->entries[frame_index].seek_pos_pts;
+}
+
 uint64_t IMB_indexer_get_seek_pos_dts(struct anim_index *idx, int frame_index)
 {
   if (frame_index < 0) {
@@ -776,9 +805,10 @@ typedef struct FFmpegIndexBuilderContext {
   IMB_Proxy_Size proxy_sizes_in_use;
 
   uint64_t seek_pos;
-  uint64_t last_seek_pos;
-  uint64_t seek_pos_dts;
   uint64_t seek_pos_pts;
+  uint64_t seek_pos_dts;
+  uint64_t last_seek_pos;
+  uint64_t last_seek_pos_pts;
   uint64_t last_seek_pos_dts;
   uint64_t start_pts;
   double frame_rate;
@@ -931,8 +961,9 @@ static void index_rebuild_ffmpeg_proc_decoded_frame(FFmpegIndexBuilderContext *c
 {
   int i;
   uint64_t s_pos = context->seek_pos;
+  uint64_t s_pts = context->seek_pos_pts;
   uint64_t s_dts = context->seek_pos_dts;
-  uint64_t pts = av_get_pts_from_frame(context->iFormatCtx, in_frame);
+  uint64_t pts = av_get_pts_from_frame(in_frame);
 
   for (i = 0; i < context->num_proxy_sizes; i++) {
     add_to_proxy_output_ffmpeg(context->proxy_ctx[i], in_frame);
@@ -946,15 +977,15 @@ static void index_rebuild_ffmpeg_proc_decoded_frame(FFmpegIndexBuilderContext *c
   context->frameno = floor(
       (pts - context->start_pts) * context->pts_time_base * context->frame_rate + 0.5);
 
-  /* decoding starts *always* on I-Frames,
-   * so: P-Frames won't work, even if all the
-   * information is in place, when we seek
-   * to the I-Frame presented *after* the P-Frame,
-   * but located before the P-Frame within
-   * the stream */
+  int64_t seek_pos_pts = timestamp_from_pts_or_dts(context->seek_pos_pts, context->seek_pos_dts);
 
-  if (pts < context->seek_pos_pts) {
+  if (pts < seek_pos_pts) {
+    /* Decoding starts *always* on I-Frames. In this case our position is
+     * before our seek I-Frame. So we need to pick the previous available
+     * I-Frame to be able to decode this one properly.
+     */
     s_pos = context->last_seek_pos;
+    s_pts = context->last_seek_pos_pts;
     s_dts = context->last_seek_pos_dts;
   }
 
@@ -971,6 +1002,7 @@ static void index_rebuild_ffmpeg_proc_decoded_frame(FFmpegIndexBuilderContext *c
                                    curr_packet->size,
                                    tc_frameno,
                                    s_pos,
+                                   s_pts,
                                    s_dts,
                                    pts);
     }
@@ -1009,10 +1041,12 @@ static int index_rebuild_ffmpeg(FFmpegIndexBuilderContext *context,
     if (next_packet->stream_index == context->videoStream) {
       if (next_packet->flags & AV_PKT_FLAG_KEY) {
         context->last_seek_pos = context->seek_pos;
+        context->last_seek_pos_pts = context->seek_pos_pts;
         context->last_seek_pos_dts = context->seek_pos_dts;
+
         context->seek_pos = next_packet->pos;
-        context->seek_pos_dts = next_packet->dts;
         context->seek_pos_pts = next_packet->pts;
+        context->seek_pos_dts = next_packet->dts;
       }
 
       int ret = avcodec_send_packet(context->iCodecCtx, next_packet);