Page MenuHome

Add Delete Proxy to Video Sequencer
Needs RevisionPublic

Authored by Eitan (EitanSomething) on Mon, Oct 28, 1:58 AM.

Details

Summary

This patch adds the ability to delete the Proxy of the active sequencer strip.

Diff Detail

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

Event Timeline

Richard Antalik (ISS) requested changes to this revision.Mon, Oct 28, 6:42 AM

This action can not be undone, so confirmation dialog must be used.
"All proxy files for this strip will be permanently removed. Do you want to continue?"
Or something in that note.

release/scripts/startup/bl_operators/sequencer.py
379–382

It doesn't seem, that this operator requires invoke method, it could be removed.

410–411

Please use a bit more descriptive names

421

also descriptive names

479

I mean it may be correct naming scheme, but it doesn't look right in context of this file. Other editors have this quite mixed, so I don't know. Don't want to nitpick too much, but on the other hand I want to point to potential issues...

This revision now requires changes to proceed.Mon, Oct 28, 6:42 AM
release/scripts/startup/bl_operators/sequencer.py
479

Maybe it is time to rename these operators to be in consistency with 2.8 class naming?

SequencerCrossfadeSounds,
SequencerCutMulticam,
SequencerDeinterlaceSelectedMovies,
SequencerFadesClear,
SequencerFadesAdd,

Really great you're having a go at this!
I'm getting an error when trying to delete proxy, when no proxy has been generated for the active Movie Strip, same thing with a sound strip.
record_run_no_gaps.blen_tc is left in the folder, after deleting.
Having more strips selected makes the operator unable to work. The operator should be able to work in two modes: one delete proxies of the active strip and another one delete proxies from all selected strips(the panel suggestion is having different panels for active and for full selection), and in the Sidebar Strip panel +Alt should make the operator work on the full selection.
Maybe a report on how many proxy files has been deleted could be nice in the Statusbar?

record_run_no_gaps.blen_tc being left in the folder is what happens when deleted proxy in the clip editor

  • Fix bug when multiple strips sellected
  • Fix bug where record_run_no_gaps.blen_tc and folders aren't deleted
  • Added Option: Delete Active selected strip proxy
Eitan (EitanSomething) marked 2 inline comments as done.Mon, Oct 28, 8:38 PM
  • More Descriptive Variable names
  • Fix Crash
release/scripts/startup/bl_operators/sequencer.py
479

I can fix the rest of the naming Scheme in another patch

Eitan (EitanSomething) marked an inline comment as done.Mon, Oct 28, 8:55 PM

Digging through WM API, I realized, that the invoke method, was actually used as a confirmation.
It was quite poor in execution, so it would be better if we could explain to user, what is going to happen. Not quite sure if that is possible

Digging through WM API, I realized, that the invoke method, was actually used as a confirmation.
It was quite poor in execution, so it would be better if we could explain to user, what is going to happen. Not quite sure if that is possible

I will look into that.
Deleting proxy in the clip editor doesn’t have a confirmation and has the invoke

Campbell Barton (campbellbarton) requested changes to this revision.Tue, Oct 29, 2:26 PM
Campbell Barton (campbellbarton) added inline comments.
release/scripts/startup/bl_operators/sequencer.py
373–380

This doesn't seem necessary or consistent with other operators.

Nearly any operator that uses the selection could also use the active-only, however we don't provide this option.

388–391

Poll function should not loop over selection, it's likely to backfire when drawing the interface and slow down interaction.

Also, was this tested? it's accessing self which isn't defined in classmethods.

412–463

This copy-pastes code from clip.py, did you look into sharing logic between these two functions? It seems like this is a case it could be done.

This revision now requires changes to proceed.Tue, Oct 29, 2:26 PM
release/scripts/startup/bl_operators/sequencer.py
373–380

The built-in proxy function allows for both building of the active strip and building of proxies for selected strips. Currently there is a Build Proxies function in the menu - so there should also be a delete proxies function. In operators executed from the VSE Menus pretty much all operate on selection, whereas operators executed from the Strip tab in the sidebar are typically only affecting the active strip unless +alt is pressed. If the following reworking of the current proxy panel, which separates the proxy functions into Active in the Strip panel and Selection in the Proxies & Cache tab, should be accepted, then separated delete proxy/proxies will also be necessary: https://developer.blender.org/D6077

So for those reasons, I suggested that Eitan added options for both active and selected strips.

release/scripts/startup/bl_operators/sequencer.py
412–463

I could definitely share code but which file should I put it in

release/scripts/startup/bl_operators/sequencer.py
412–463

For the moment you could make a staticmethod on the class CLIP_OT_delete_proxy in clip.py, import and call that.

Later we could move to ./release/scripts/modules/bpy_extras/video_utils.py.

  • Report proxy deletion information
  • Add warnings
Campbell Barton (campbellbarton) requested changes to this revision.Tue, Oct 29, 7:01 PM
Campbell Barton (campbellbarton) added inline comments.
release/scripts/startup/bl_operators/sequencer.py
372–373

These should be set in the classes execute function. Also use Python naming convention (lower case w/ underscore separators).

This revision now requires changes to proceed.Tue, Oct 29, 7:01 PM
release/scripts/startup/bl_operators/sequencer.py
412–463

I can put it in ./release/scripts/modules/bpy_extras/video_utils.py right now