Page MenuHome

Power Sequencer: video editing tools
Closed, ResolvedPublic

Description

We have been working on an add-on for the blender video sequence editor with the community for a few years now. We are maintaining it and recently ported it to blender 2.80. I'd like to contribute it as a release add-on.

Power Sequencer adds dozens of operators to edit videos faster with the sequencer you can find here: https://www.gdquest.com/blender/power-sequencer/docs/
Here's the add-on repository with general info: https://github.com/GDquest/Blender-power-sequencer/

The operators are all documented: we write the docs in the source code and update them as we go

The current version tries to respect the vanilla Blender user experience and adds new tools to the program, like an interactive trim tool, operators to do fades, crossfades, move crossfades, and many others, all offering more flexibility than the operators available in the Kinoraw tools. We use the add-on to produce video tutorials with my team, and made it so we could edit as fast as possible.

We are at least several persons using it on a regular basis, including the two video editors I'm working with for our YouTube channel, so it’s gotten some good testing over the past 2 years.

A few persons asked me to submit it to Blender, here you go!

Zip file:

Notes:

  • I could not create a page on the add-ons catalogue as the wiki is archived now.
  • The plug-in ships with the add-on auto-updater script, I just put everything in the zip file but I can remove it if you need me too
  • We've renamed every operator to use the same format as the blender source code, and we follow the blender Py guidelines and pep8

Details

Type
Patch

Related Objects

Event Timeline

Brendon Murphy (meta-androcto) lowered the priority of this task from Needs Triage by Developer to Normal.Aug 17 2019, 1:10 AM

hi @Nathan Lovato (gdquest) good to see you again, It's worth mentioning you guys have been in blender.chat #vse and discussing the vse for some time. I think we could look at adding this if the vse team agree. Currently, as I'm sure your aware, the sequencer addons in Blender are broken. So I'm open to replacement from a known long term addons dev such as yourself and your addon.

Hi Brendon! Long-time no see.

Richard told me he'd review this task, and that he needed help to do add-on review as it's his first time.

Just updated the zip to PS 1.3, with a few bug fixes and other improvements.

Brendon Murphy (meta-androcto) triaged this task as Confirmed, Medium priority.Aug 24 2019, 1:12 PM

hi, I'm raising status, It would be a nice addition to 2.8 I think. Can you please check with nightly builds to make sure code is working correctly, I don't see any issue, but it's worth regular checking.
Have you looked at https://wiki.blender.org/wiki/Tools/Git and https://wiki.blender.org/wiki/Developer_Intro/Overview ? Are you prepared to maintain the addon within Blender through the 2.8 series? Let me know so we can look at your access to the addons repository. I've also noticed your actively talking with @Richard Antalik (ISS) and plan to work on the vse in general also.
Thanks.

Following is a template guide for asking commit rights:

hi, if you can join this mailing list: bf-committers@blender.org and mail:
Subject:

Re: Commit Access to Addons Repository: ATT: @ideasman42

Content:

Hi my name is: "your name"
My user name on developer.blender.org is: "your user name here"
I would like commit access to the addons repository.
I am the maintainer of/my addon is:
New to Blender and it's task is here: "inset addon name and task name".
or I'm the Author of an addon in Blender and I wish to update the addon: "inset addon name and task name if applicable".
or I'm a new maintainer of an addon in Blender and I wish to update the addon/s "inset addon/s name/s and task name/s if applicable".

I'm familiar with git usage and my repo is here: "your git hub" *not needed but why not if you have one. It's important to know git usage before committing to the Blender addons repo.

It's also useful to join this mailing list: bf-extensions-cvs@blender.org as this list mails out the commits.
https://wiki.blender.org/wiki/Developer_Intro/Overview is a good read.
https://blender.chat/channel/python and https://blender.chat/channel/blender-coders provide a good place to discuss your addon or issues if needed. It's good also to drop in and say hi and meet other developers and provides a good communication platform.
Final note: Addons in the Blender repository are updated by addons devs and Blender core devs help with api updates/fixes. As there's multiple people committing it's important to make sure your local folders are updated to current before committing your self.

Any issues please let us know.
Thanks for your interest and support.

Thanks Brendon. 🙂

Have you looked at https://wiki.blender.org/wiki/Tools/Git and https://wiki.blender.org/wiki/Developer_Intro/Overview ?

Yup, I use git daily on the job. Also already made small contributions to the VSE, looking to do more.

Are you prepared to maintain the addon within Blender through the 2.8 series?

Yes, we edit with Blender with my teammates anyway.

I started editing videos with the latest master and no problem so far.
When you're about to review the add-on, please ping me so I can update the code beforehand: I'm making changes almost daily lately. The code structure and style is going to be the same (one file per operator), but e.g. I put all utility functions in a single module and added a draw module recently.

Or you can find the most up-to-date code here: https://github.com/GDquest/Blender-power-sequencer

Campbell Barton (campbellbarton) lowered the priority of this task from Confirmed, Medium to Needs Information from User.EditedSep 2 2019, 5:34 AM

The plug-in ships with the add-on auto-updater script, I just put everything in the zip file but I can remove it if you need me too

While it's understandable add-ons maintained externally add this themselves, having this for bundled add-ons has is a different issue.

This should be removed / disabled before inclusion in Blender since it opens
it would be better to spend effort on T56165: Add-On Repository.

hi @Nathan Lovato (gdquest)
Welcome and thanks!
All seems well here apart from the mentioned updater, if you can remove the updater and link in the new file before any commits. We can catch up in blender.chat to finalize any issues.
Opening task until fully resolved and committed in case there are any other issues discovered before the commit.

Hi, I updated the add-on above, here's a copy:

I removed the updater and tested that it was working. Should be good for review now. Thanks!

This is fairly large for an add-on so I only did a high-level review.
Here are some issues I've run into.

  • Includes files not part of the add-on:
    • Binary file GTAGS, other files in this directory too ... )
    • pyproject.toml
  • Don't include the full GPL LICENSE, use a short header at start of all files. As we do everywhere else in Blender.

    ... this goes for sub-modules that are including their own LICENSE files too.
  • No need for changelog.md, we normally rely on git commit history.
  • README.md - not sure of it's purpose when bundled with Blender, it's unlikely users will even find this. Better to have the add-on link to good online documentation.
  • In handlers the functions are converted to a string to check their identity. It would be better to store them in a list and remove them directly, or if thats not possible - check their __module__ and __name__, so you don't accidentally have naming collisions w/ other add-ons.
  • Noticed redundant object subclasses, this is redundant and was only needed for Python2 compatibility.

I updated my build script to clean up all the extra files, added the license headers, and made all the changes:

Noticed redundant object subclasses

I only found one with grep in ProgressBar, removed it. Were there any others?

Why is pyperclip module needed?

It was old code, I didn't know at the time, that's fixed.

@Nathan Lovato (gdquest) could you commit to a temporary branch? tracking changes between updates to a zip-file doesn't work well, it's inconvenient to see what's changed.

Use temp- in the prefix, eg temp-power-sequencer

Sure, I'll do that after work! Thanks

It's up in the addons-contrib repo, branch temp-power-sequencer, I added a power_sequencer directory: https://developer.blender.org/diffusion/BAC/browse/temp-power-sequencer/

Went over the code in some more detail, generally seems OK.

Note that there are checks in the poll functions we wouldn't accept in Blenders code (C or Python):

For example:

@classmethod
def poll(cls, context):
    try:
        next(s for s in context.selected_sequences if s.type == "META")
    except StopIteration:
        return False
    return context.sequences

The reason for this is an operator may be exposed in a toolbar on in a panel, making every redraw have to loop over all sequence strips (potentially twice in this case), just to check if the button should show as enabled or not.

When the operators are called from menu items it's not an issue in practice, however UI layouts change and it's sometimes useful to show buttons in panels/toolbars.

Occasionally we have reports that Blender runs slow and it turns out to be add-ons doing inefficient tests like this.

Don't think this is a blocker for committing to add-ons though.

Thanks, I'm aware of that and wouldn't do it in the official source code. This looks like something I did a long time ago, I'll clean it up.

While I'm around, is it okay to check the length of lists of sequences? Richard told me that in Blender's sources we can't use the sequence lists at all for performance reasons. E.g. this wouldn't be okay: return len(context.selected_sequences) > 1

I'm asking because I'm looking to contribute some code from the add-on to the VSE, so if it's not okay I can start changing that in the addon, which is great to get people to test the tools and find bugs before creating patches.

Re: checking lengths:

  • from contexts all lists are created fully before access. So context.selected_sequences will create and free a everything list no matter what you do with it.
  • in the case of RNA collections, we use a sequence type, where:
    • bool(view_layer.objects) is O(1)
    • len(view_layer.objects) is O(N)

... so always use bool(seq) instead of len(seq) > 0

Got it, makes total sense. You tend to forget how things work at a lower level working with Python, at least until you hit performance issues. So even if poll gets context as an argument, until you access a given list or collection, it isn't even initialized, right. I'll be mindful of that when contributing to the VSE.

I just updated the code in the temp branch to remove all the len() checks and other try/except in operator polls, tested that the addon works:

  • Is there anything left to clean up, or can I delete the temp branch and push the addon?
  • If so, can I push the addon straight to the blender-addons repository?

Thanks for taking the time to review my work :)

  • Is there anything left to clean up, or can I delete the temp branch and push the addon?
  • If so, can I push the addon straight to the blender-addons repository?

Yes, think this is fine to push now. The temp branch can be deleted.

Nathan Lovato (gdquest) closed this task as Resolved.Sep 5 2019, 5:25 PM