Power Sequencer: video editing tools #68584

New Issue

Nathan Lovato · 2019-08-12T20:19:20+02:00

Nathan Lovato commented

2019-08-12 20:19:20 +02:00

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: power_sequencer.zip

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

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: [power_sequencer.zip](https://archive.blender.org/developer/F7714069/power_sequencer.zip) 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

Nathan Lovato commented

2019-08-12 20:19:20 +02:00

Added subscriber: @GDQuest

Andy Cuccaro commented

2019-08-13 05:33:06 +02:00

Added subscriber: @AndyCuccaro

Stephen Swaney commented

2019-08-13 09:40:55 +02:00

Added subscriber: @StephenSwaney

Dalai Felinto commented

2019-08-13 12:19:11 +02:00

Added subscribers: @iss, @dfelinto

Dalai Felinto commented

2019-08-13 12:19:11 +02:00

@iss could you take a look at that?

Brendon Murphy commented

2019-08-17 01:10:12 +02:00

Added subscriber: @BrendonMurphy

Brendon Murphy commented

2019-08-17 01:10:12 +02:00

hi @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.

Nathan Lovato commented

2019-08-17 08:42:52 +02:00

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.

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.

Nathan Lovato commented

2019-08-18 15:07:57 +02:00

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

Brendon Murphy commented

2019-08-24 13:12:01 +02:00

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 @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.

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 @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.

Nathan Lovato commented

2019-08-25 08:38:13 +02:00

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.

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.

interlichtspielhaus commented

2019-08-25 12:05:52 +02:00

Added subscriber: @intrah

Nathan Lovato commented

2019-08-28 18:07:05 +02:00

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

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 commented

2019-09-02 05:34:12 +02:00

Added subscriber: @ideasman42

Campbell Barton commented

2019-09-02 05:34:12 +02:00

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 blender/blender#56165 (Add-On Repository).

> 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 blender/blender#56165 (Add-On Repository).

Campbell Barton commented

2019-09-02 05:37:20 +02:00

Changed status from 'Open' to: 'Resolved'

Campbell Barton closed this issue

2019-09-02 05:37:20 +02:00

Campbell Barton self-assigned this 2019-09-02 05:37:20 +02:00

Brendon Murphy commented

2019-09-02 05:57:48 +02:00

Changed status from 'Resolved' to: 'Open'

Brendon Murphy reopened this issue

2019-09-02 05:57:48 +02:00

Brendon Murphy commented

2019-09-02 05:57:48 +02:00

hi @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 @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.

Nathan Lovato commented

2019-09-02 09:30:03 +02:00

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

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

Hi, I updated the add-on above, here's a copy: [power_sequencer.zip](https://archive.blender.org/developer/F7713883/power_sequencer.zip) I removed the updater and tested that it was working. Should be good for review now. Thanks!

Campbell Barton commented

2019-09-02 09:50:48 +02:00

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.

Why is pyperclip module needed? Blender has it's own clipboard access.

https://docs.blender.org/api/master/bpy.types.WindowManager.html#bpy.types.WindowManager.clipboard

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.

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. ``` - Why is `pyperclip` module needed? Blender has it's own clipboard access. ``` https://docs.blender.org/api/master/bpy.types.WindowManager.html#bpy.types.WindowManager.clipboard ``` - 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.

Nathan Lovato commented

2019-09-02 12:23:34 +02:00

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

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.

I updated my build script to clean up all the extra files, added the license headers, and made all the changes: [power_sequencer.zip](https://archive.blender.org/developer/F7714069/power_sequencer.zip) > 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.

Campbell Barton commented

2019-09-02 12:34:52 +02:00

@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

@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`

Nathan Lovato commented

2019-09-02 12:39:36 +02:00

Sure, I'll do that after work! Thanks

Nathan Lovato commented

2019-09-03 19:13:59 +02:00

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/

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/

Campbell Barton commented

2019-09-04 14:34:56 +02:00

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.

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.

Nathan Lovato commented

2019-09-04 14:50:44 +02:00

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.

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.

Campbell Barton commented

2019-09-05 07:16:47 +02:00

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

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`

Nathan Lovato commented

2019-09-05 13:54:51 +02:00

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 :)

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 :)

Campbell Barton commented

2019-09-05 14:15:16 +02:00

In #68584#769812, @GDQuest wrote:

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.

> In #68584#769812, @GDQuest wrote: > - 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 commented

2019-09-05 17:25:08 +02:00

Done!