[Bf-committers] New VSE proxies, feedback appreciated

[Peter Schlaile]

Hi Brecht,

> Am I right that if you don't enable proxies, this patch basically has
> no effect?

Yupe, that's true. It has a very conservative approach.

> In that case it should be pretty safe to commit before
> 2.58, but still think this could use some good testing on complex
> files.

Agreed.

> * Job name in header is "Seq Proxy", maybe name it "Building Proxies"
> or something like that?

updated.

> * On running rebuild proxy, it does not redraw the sequencer header
> immediately, makes it unclear if the job has started.

added an ED_area_tag_redraw (which hopefully does the job, never noticed 
that problem...)

> * Do we need the option to disable building time codes for proxies?
> All 3 are enabled by default, and it seems like there isn't much
> reason not to build them, since you can still afterwards decide to use
> them or not.

Good point, removed from UI.

> * Timecode enum: the items here could use descriptions. Also there is
> no need to repeat "TC" in the item names, and generally abbreviations
> like that should be avoided in the UI. Would just go with "Record
> Run", "Free Run", ..

updated.

> * If timecode options other than Record Run are not supported, I guess
> they should not be exposed in the UI?

Uhm well, I'm still in the hope to include them before merge :)
Otherwise: you are right.

> * These timecode indexes are not used by default, is there a reason for this?

Debugging reasons. Otherwise: no good reason, indeed. (should default to
record run)

> * One thing I don't understand about this is how it can be a proxy
> level setting. Doesn't this also affect non-proxy renders and the
> length of the strip in the sequencer?

You are right.

I first thought, that there is a fixed relationship between 
proxies and the index in use. That isn't really the case (there is only 
a one way dependency: proxies don't really work in general without a time code, 
since you can't address frames correctly with variable FPS e.g.)

So: time code indices could be moved one layer up in theory (say: into 
Strip structure).

What makes this a problem: I wanted to use the same directory 
structure for both, so that proxy files end up in the same directory as 
the index files (otherwise, users have to select *two* directories, one 
for proxies, one for indices, which is really silly.) And: those 
directory paths are currently stored in StripProxy.

So we could shuffle things around in DNA to make that move possible 
(thereby breaking upward compatibility.).

I don't know, if that is worth the effort, since for all real world cases, 
those two features are really nearly always used in tandem.

(Please tell me, if you think otherwise.)

To at least clarify things in UI, I just renamed the label of the Tab from 
"Proxy" to "Proxy / Timecode", and "Use Proxy" to "Use Proxy / Timecode".

> * What happens when you remove a strip while the proxy for it is being
> built? Didn't find any checks for that.

Since we work on (deep) copies of the original strips, nothing will 
happen at all, if you delete a strip.

Blender will continue to build the proxy in the background on the 
hidden copy.

Don't know, if that's a bug or a feature UI wise (I personally consider it 
a feature. If you want to stop proxy building, deleting a strip isn't 
necessarily the brightest thing to do. Hitting the stop button seems 
more *uhm* straight forward :) )

Cheers,
Peter

P.S.: Thanks for reviewing!