[Bf-blender-cvs] [fb1265c5cf2] master: Tracking: Fix movie file prefetch freezing interface

Sergey Sharybin noreply at git.blender.org
Mon Mar 22 16:27:51 CET 2021


Commit: fb1265c5cf24462985cd3debae9205ac78f5d6ab
Author: Sergey Sharybin
Date:   Mon Mar 22 16:24:03 2021 +0100
Branches: master
https://developer.blender.org/rBfb1265c5cf24462985cd3debae9205ac78f5d6ab

Tracking: Fix movie file prefetch freezing interface

The issue was caused by the prefetch code having LOCK_MOVIECLIP lock
acquired while reading frames from the movie files. The need of the
lock was coming from the fact that `clip->anim` can not be accessed
from multiple threads, so that was guarded by a lock. The side effect
of this lock was that the main thread (from which drawing is happening)
did not have any chance passing through it in the cache code because
the prefetch happens so quickly.

The solution is to create a local copy of the clip with its own
anim handler, so that read can happen without such lock.

The prefetch is slower by an absolute number in seconds (within 10%
in tests here), but it is interactive now.

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

M	source/blender/blenkernel/BKE_movieclip.h
M	source/blender/blenkernel/intern/movieclip.c
M	source/blender/editors/space_clip/clip_editor.c

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

diff --git a/source/blender/blenkernel/BKE_movieclip.h b/source/blender/blenkernel/BKE_movieclip.h
index d33eabeb7e0..067dc694109 100644
--- a/source/blender/blenkernel/BKE_movieclip.h
+++ b/source/blender/blenkernel/BKE_movieclip.h
@@ -102,8 +102,11 @@ float BKE_movieclip_remap_clip_to_scene_frame(const struct MovieClip *clip, floa
 void BKE_movieclip_filename_for_frame(struct MovieClip *clip,
                                       struct MovieClipUser *user,
                                       char *name);
-struct ImBuf *BKE_movieclip_anim_ibuf_for_frame(struct MovieClip *clip,
-                                                struct MovieClipUser *user);
+
+/* Read image buffer from the given movie clip without acquiring the `LOCK_MOVIECLIP` lock.
+ * Used by a prefetch job which takes care of creating a local copy of the clip. */
+struct ImBuf *BKE_movieclip_anim_ibuf_for_frame_no_lock(struct MovieClip *clip,
+                                                        struct MovieClipUser *user);
 
 bool BKE_movieclip_has_cached_frame(struct MovieClip *clip, struct MovieClipUser *user);
 bool BKE_movieclip_put_frame_if_possible(struct MovieClip *clip,
diff --git a/source/blender/blenkernel/intern/movieclip.c b/source/blender/blenkernel/intern/movieclip.c
index 017a73593ee..9c2cd03dbc2 100644
--- a/source/blender/blenkernel/intern/movieclip.c
+++ b/source/blender/blenkernel/intern/movieclip.c
@@ -1958,14 +1958,12 @@ void BKE_movieclip_filename_for_frame(MovieClip *clip, MovieClipUser *user, char
   }
 }
 
-ImBuf *BKE_movieclip_anim_ibuf_for_frame(MovieClip *clip, MovieClipUser *user)
+ImBuf *BKE_movieclip_anim_ibuf_for_frame_no_lock(MovieClip *clip, MovieClipUser *user)
 {
   ImBuf *ibuf = NULL;
 
   if (clip->source == MCLIP_SRC_MOVIE) {
-    BLI_thread_lock(LOCK_MOVIECLIP);
     ibuf = movieclip_load_movie_file(clip, user, user->framenr, clip->flag);
-    BLI_thread_unlock(LOCK_MOVIECLIP);
   }
 
   return ibuf;
diff --git a/source/blender/editors/space_clip/clip_editor.c b/source/blender/editors/space_clip/clip_editor.c
index 36375ad1ef3..2da13646a8b 100644
--- a/source/blender/editors/space_clip/clip_editor.c
+++ b/source/blender/editors/space_clip/clip_editor.c
@@ -67,6 +67,8 @@
 
 #include "clip_intern.h" /* own include */
 
+#include "PIL_time.h"
+
 /* -------------------------------------------------------------------- */
 /** \name Operator Poll Functions
  * \{ */
@@ -651,7 +653,16 @@ void ED_space_clip_set_mask(bContext *C, SpaceClip *sc, Mask *mask)
  * \{ */
 
 typedef struct PrefetchJob {
+  /* Clip into which cache the frames will be prefetched into. */
   MovieClip *clip;
+
+  /* Local copy of the clip which is used to decouple reading in a way which does not require
+   * threading lock which might "conflict" with the main thread,
+   *
+   * Used, for example, for animation prefetching (`clip->anim` can not be used from multiple
+   * threads and main thread might need it). */
+  MovieClip *clip_local;
+
   int start_frame, current_frame, end_frame;
   short render_size, render_flag;
 } PrefetchJob;
@@ -903,11 +914,15 @@ static void start_prefetch_threads(MovieClip *clip,
   BLI_spin_end(&queue.spin);
 }
 
-static bool prefetch_movie_frame(
-    MovieClip *clip, int frame, short render_size, short render_flag, short *stop)
+/* NOTE: Reading happens from `clip_local` into `clip->cache`. */
+static bool prefetch_movie_frame(MovieClip *clip,
+                                 MovieClip *clip_local,
+                                 int frame,
+                                 short render_size,
+                                 short render_flag,
+                                 short *stop)
 {
   MovieClipUser user = {0};
-  ImBuf *ibuf;
 
   if (check_prefetch_break() || *stop) {
     return false;
@@ -918,7 +933,7 @@ static bool prefetch_movie_frame(
   user.render_flag = render_flag;
 
   if (!BKE_movieclip_has_cached_frame(clip, &user)) {
-    ibuf = BKE_movieclip_anim_ibuf_for_frame(clip, &user);
+    ImBuf *ibuf = BKE_movieclip_anim_ibuf_for_frame_no_lock(clip_local, &user);
 
     if (ibuf) {
       int result;
@@ -942,6 +957,7 @@ static bool prefetch_movie_frame(
 }
 
 static void do_prefetch_movie(MovieClip *clip,
+                              MovieClip *clip_local,
                               int start_frame,
                               int current_frame,
                               int end_frame,
@@ -956,7 +972,7 @@ static void do_prefetch_movie(MovieClip *clip,
 
   /* read frames starting from current frame up to scene end frame */
   for (frame = current_frame; frame <= end_frame; frame++) {
-    if (!prefetch_movie_frame(clip, frame, render_size, render_flag, stop)) {
+    if (!prefetch_movie_frame(clip, clip_local, frame, render_size, render_flag, stop)) {
       return;
     }
 
@@ -968,7 +984,7 @@ static void do_prefetch_movie(MovieClip *clip,
 
   /* read frames starting from current frame up to scene start frame */
   for (frame = current_frame; frame >= start_frame; frame--) {
-    if (!prefetch_movie_frame(clip, frame, render_size, render_flag, stop)) {
+    if (!prefetch_movie_frame(clip, clip_local, frame, render_size, render_flag, stop)) {
       return;
     }
 
@@ -998,6 +1014,7 @@ static void prefetch_startjob(void *pjv, short *stop, short *do_update, float *p
   else if (pj->clip->source == MCLIP_SRC_MOVIE) {
     /* read movie in a single thread */
     do_prefetch_movie(pj->clip,
+                      pj->clip_local,
                       pj->start_frame,
                       pj->current_frame,
                       pj->end_frame,
@@ -1016,6 +1033,13 @@ static void prefetch_freejob(void *pjv)
 {
   PrefetchJob *pj = pjv;
 
+  MovieClip *clip_local = pj->clip_local;
+  if (clip_local != NULL) {
+    BKE_libblock_free_datablock(&clip_local->id, 0);
+    BKE_libblock_free_data(&clip_local->id, false);
+    MEM_freeN(clip_local);
+  }
+
   MEM_freeN(pj);
 }
 
@@ -1103,6 +1127,12 @@ void clip_start_prefetch_job(const bContext *C)
   pj->render_size = sc->user.render_size;
   pj->render_flag = sc->user.render_flag;
 
+  /* Create a local copy of the clip, so that video file (clip->anim) access can happen without
+   * acquiring the lock which will interfere with the main thread. */
+  if (pj->clip->source == MCLIP_SRC_MOVIE) {
+    BKE_id_copy_ex(NULL, (ID *)&pj->clip->id, (ID **)&pj->clip_local, LIB_ID_COPY_LOCALIZE);
+  }
+
   WM_jobs_customdata_set(wm_job, pj, prefetch_freejob);
   WM_jobs_timer(wm_job, 0.2, NC_MOVIECLIP | ND_DISPLAY, 0);
   WM_jobs_callbacks(wm_job, prefetch_startjob, NULL, NULL, NULL);



More information about the Bf-blender-cvs mailing list