Page MenuHome

Cleanup sequencer proxy properties panel
Needs RevisionPublic

Authored by Guillaume Matheron (mathers) on Sat, Jan 26, 11:16 PM.

Details

Summary

Global options are moved to a separate panel, that is always active in the sequencer, while strip-specific options are kept in the old panel.

I believe this creates a nice separation and helps understand how the proxy system works, especially for beginners.

This diff is based on 25772c9e1d2d5239e1f7f83f6ea0c4d7d1f1df4a

Diff Detail

Event Timeline

Richard Antalik (ISS) requested changes to this revision.Sun, Jan 27, 1:39 AM

I am not absolutely sure, if I am happy with splitting of proxy panel.
Will apply this patch and try to work with it and think about it a bit more.

I have one suggestion though:
Before user clicks on rebuild proxies(sequencer.enable_proxies) or set proxies (sequencer.rebuild_proxy) he should see number of video strips(or supported files) in selection(or in timeline if rebuild all strips option is available) and sum of playback time for affected files (ideally at original FPS rate).
I would add this info as a 1-2 line label near those buttons

release/scripts/startup/bl_ui/space_sequencer.py
1148

Panel with this label already exists. Specify, that this is panel for "global" settings. I would rather use "Proxy settings" or "Sequencer proxy settings".
I see, that you have changed label of the other panel, but still purpose/designation of panel should be as clear as possible.

1149

I would create a new category for both proxy panels. "Proxy" would be good name.

  • It makes sense for these panels to be close together
  • I will be introducing a panel for cache settings. It seems logical to have "Proxy/cache" panel, rather then dig through strip panel
1153–1155

You can do return cls.has_sequencer(context)
Haven't seen explicit return True in this file. But I am not absolutely sure.

1157

This method placeholder should be removed, if not used

1171–1172

Issue here is, that these operators do work on selection. Even worse is, that they don't have poll function defined, so user can click it and it makes a visible action, that has no effect.

There are few ways to solve this:

  • Write poll function - just check if there is movie type in selection
  • Add parameter to operator, so it can do action for all strips
  • I am not aware of method to grey out this button, hiding it is not an option

Any of first two options will do for me.

1174

I would also change class name SEQUENCER_PT_strip_proxy

1176

I would create a new category for both proxy panels. "Proxy" would be good name.

1187

Include also image and meta type.

Images:

  • it has to be supported somehow
  • it works and this functionality may be missing in meantime

Meta will build proxies for included movies.

This revision now requires changes to proceed.Sun, Jan 27, 1:39 AM
Guillaume Matheron (mathers) marked 8 inline comments as done.

Added changes suggested by @Richard Antalik (ISS) : selection-based operators are greyed out when no compatible strips are selected, and other cosmetic changes.

I'm not sure how to add info about the number of affected strips, especially with meta strips

Richard Antalik (ISS) added inline comments.
release/scripts/startup/bl_ui/space_sequencer.py
1147

These are details, but keep class names and displayed text consistent - not necessarily verbatim, but at least word order

1187

These are details, but keep class names and displayed text consistent - not necessarily verbatim, but at least word order

source/blender/editors/space_sequencer/sequencer_edit.c
3505

Both poll functions are identical.
Use one function for both operators, I would name it sequencer_selection_proxy_supported_poll

Guillaume Matheron (mathers) marked 3 inline comments as done.

Minor code cleanups

Can not update diff to add selection info label.
So here's the code I would add.
I will accept this anyway, will see what other devs will say about this, since it's not critical.

def get_selected_movie_proxy_recursive_unique(strips):
    buildable = {s.filepath: s for s in strips if s.type =='MOVIE' and s.use_proxy}
    metas = [s for s in strips if s.type == 'META' and s.use_proxy]

    for meta in metas:
        buildable = {**buildable, **get_selected_movie_proxy_recursive_unique(meta.strips)}

    return buildable

def get_selected_image_proxy_recursive_unique(strips):
    images = [s for s in strips if s.type =='IMAGE' and s.use_proxy]
    metas = [s for s in strips if s.type == 'META' and s.use_proxy]
    buildable = set()

    for image in images:
        buildable |= set([image.directory + s.filename for s in image.elements])

    for meta in metas:
        buildable |= get_selected_image_proxy_recursive_unique(meta.strips)

    return buildable


def draw_selection_info(layout, context):
    movies = get_selected_movie_proxy_recursive_unique(context.selected_sequences)
    images = get_selected_image_proxy_recursive_unique(context.selected_sequences)
    total_len = 0

    for movie in movies.values():
        total_len += movie.frame_final_duration
        total_len += movie.animation_offset_start
        total_len += movie.animation_offset_end
        total_len += movie.frame_offset_start
        total_len += movie.frame_offset_end

    total_len += len(images)

    text_num = str(len(context.selected_sequences)) + " Selected"
    time = bpy.utils.smpte_from_frame(total_len)
    text_time = time + " Total time to rebuild"

    layout.label(text=text_num)
    layout.label(text=text_time)


class SEQUENCER_PT_proxy_settings(SequencerButtonsPanel, Panel):
    bl_label = "Proxy settings"
    bl_category = "Proxy"
    @classmethod
    def poll(cls, context):
        return cls.has_sequencer(context)

    def draw(self, context):
        layout = self.layout

        ed = context.scene.sequence_editor

        flow = layout.column_flow()
        flow.prop(ed, "proxy_storage", text="Storage")
        if ed.proxy_storage == 'PROJECT':
            flow.prop(ed, "proxy_dir", text="Directory")

        col = layout.column()
        draw_selection_info(col, context)
        col.operator("sequencer.enable_proxies")
        col.operator("sequencer.rebuild_proxy")
This revision is now accepted and ready to land.Thu, Feb 7, 7:51 AM
Campbell Barton (campbellbarton) requested changes to this revision.Mon, Feb 11, 6:28 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/space_sequencer/sequencer_edit.c
3506

Poll functions should never perform searches on all selected items. Just use the active as is done elsewhere.

This revision now requires changes to proceed.Mon, Feb 11, 6:28 AM
source/blender/editors/space_sequencer/sequencer_edit.c
3506

But the operator is applied to all selected items, not just the active one. Is there a faster way to iterate over selected items ?

source/blender/editors/space_sequencer/sequencer_edit.c
3506

I would understand concern of accessing selection from python as a python object, as it has to be built.

Most users will hit a and rebuild proxies button, but they can't because it's not visible.

Sorry, I really can not help myself here...

Can we have managed list of selected sequences, that will be updated when user updates the selection so we don't have to iterate over all elements?

Will this be OK then?
If no, then this shall be reverted.