Page MenuHome

VSE: prefetching
Needs RevisionPublic

Authored by Richard Antalik (ISS) on Thu, Aug 1, 12:46 AM.

Details

Summary

A bit of refactoring was done from last "version", no cleanup of old code in this diff.

Short description:
When enabled prefetching(preview panel>view settings), a pernament running job is created, that will render frames in the background until the cache is full.
Cache recycling behavior will be changed to "free furthest frame to the left of playhead if possible, otherwise rightmost frame".
If the cache is not filled fast enough, prefetch job suspends itself at the last moment and will wait until it has chance to "catch up"
Effectively this will decouple rendering to separate thread, so rendering itself is a bit faster (more frequent technically).

Prefetch job contains a copy of scene in which sequencer is used. This copy is updated every time a cache is invalidated.

It should be possible to use prefetch API to create jobs running in in-house jobs system, so prefetch job manager was quite quickly dropped(also would introduced too much
ugly code). Because prefetch is pernament in it's nature, current job system is not usable.

There is still collision with blf library.
I am not aware of any other issues (but haven't really used this patch for a while).

Diff Detail

Repository
rB Blender
Branch
prefetch (branched from master)
Build Status
Buildable 4241
Build 4241: arc lint + arc unit

Event Timeline

Getting a build error with this patch.

/home/brecht/dev/worktree/source/blender/blenkernel/intern/seqprefetch.c: In function ‘BKE_sequencer_prefetch_update_scene’:
/home/brecht/dev/worktree/source/blender/blenkernel/intern/seqprefetch.c:248:3: error: implicit declaration of function ‘BKE_sequencer_prefetch_stop’; did you mean ‘BKE_sequencer_prefetch_start’? [-Werror=implicit-function-declaration]
   BKE_sequencer_prefetch_stop(pfjob);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   BKE_sequencer_prefetch_start
At top level:
/home/brecht/dev/worktree/source/blender/blenkernel/intern/seqprefetch.c:227:13: warning: ‘seq_prefetch_stop’ defined but not used [-Wunused-function]
 static void seq_prefetch_stop(PrefetchJob *pfjob)
             ^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
Brecht Van Lommel (brecht) requested changes to this revision.Wed, Aug 14, 6:33 PM

I didn't get to fully reviewing this today, mainly did just one pass reading through the code.

There's nothing problematic in the overall design that jumped out at me though.

source/blender/blenkernel/BKE_sequencer.h
101

Maybe rename to cache_id? Or at least something that explains a bit what this is for.

357

area -> time_range maybe a bit more clear terminology

source/blender/blenkernel/intern/seqcache.c
235–237

Can this comment explain what it does and why it's easier to fit it in here?

This function in general perhaps could use comments about what it does.

736

Nitpick: this line could be moved into its own function and code shared with BKE_sequencer_cache_recycle_item.

source/blender/blenkernel/intern/seqprefetch.c
41

Don't use .. for include paths.

Currently we don't call the window manager from blenkernel, which is why it's not part of the include paths. We should not add a dependency in this direction.

The simple solution would be to just loop through the screens in main, only active screens will have an animtimer or scrubbbing set.

72

Maybe renames to num_frames_prefetched or something like that?

It's not immediately obvious from the name that this is a number of frames.

75–78

We use volatile in a few more places like this for threading, but this doesn't actually do anything useful and can be left out.

scrubbing is unused.

358

One potential issues here, is that the depsgraph is built for a specific view layer. I'm not sure what happens if you for example delete a view layer and so also it associated depsgraph, if this pointer will remain valid.

source/blender/makesrna/intern/rna_sequencer.c
1934

Naming convention: use_prefetch.

This revision now requires changes to proceed.Wed, Aug 14, 6:33 PM