Home

Add the VSE addon Power Sequencer

Authored by Nathan Lovato (gdquest) on Sep 5 2019, 5:22 PM.

Description

Add the VSE addon Power Sequencer

Details

Committed
Nathan Lovato (gdquest)Sep 5 2019, 5:22 PM
Parents
rBAda5a1175e30c: OscurarTools: new feature, select flipped uvs, like maya
Branches
Unknown
Tags
Unknown

Event Timeline

This causes test failures, in the automated test that tries to load all add-on modules individually:
https://builder.blender.org/admin/#/builders/1/builds/1283/steps/4/logs/stdio

For the scipy one you test if that module is available in the .py file itself instead of externally.

I see. The BPS* packages have scripts that are not meant to be imported as modules, they're scripts to use running blender headless from the command line. How can I prevent the test from importing these? They're two packages that add multithreaded video rendering and fast proxy generation with ffmpeg, including on the GPU.

I have to look at how to run these tests locally. I will fix these errors ASAP.

For the scipy one you test if that module is available in the .py file itself instead of externally.

In the operator that uses the faulty module, there is a try block that checks if scipy is available, and only calls into the module if scipy imports properly. So in the addon itself, the module is never imported if scipy isn't available.
How should I handle this instead? should I add a try block in the faulty file as well? Remove the feature altogether?

I see. The BPS* packages have scripts that are not meant to be imported as modules, they're scripts to use running blender headless from the command line. How can I prevent the test from importing these? They're two packages that add multithreaded video rendering and fast proxy generation with ffmpeg, including on the GPU.

There is a blacklist in tests/python/bl_load_py_modules.py, I can add folders or files there if you tell me which ones.

For add-ons in general it is recommended to only do things inside an if __name__ == "__main__": condition. This test helps find syntax errors and some other types of issues in scripts, even if they are not meant to be imported as modules. But I'm not sure if that's easy to do here, or if it's important.

In the operator that uses the faulty module, there is a try block that checks if scipy is available, and only calls into the module if scipy imports properly. So in the addon itself, the module is never imported if scipy isn't available.
How should I handle this instead? should I add a try block in the faulty file as well? Remove the feature altogether?

Simplest solution would be to move from scipy.io import wavfile inside the find_offset function.

I just pushed a commit that seem to have fixed every error. I downloaded the test suite but is there a way for me to run with my own version of the addons locally before pushing?
Also, this time I pushed the fixes straight, but should I e.g. open a branch and ask for review for the next update of the addon? I'll likely push all changes at once before the 2.81 release, for some final testing

I just pushed a commit that seem to have fixed every error. I downloaded the test suite but is there a way for me to run with my own version of the addons locally before pushing?

I committed some additional fixes now to make the tests pass.

To run the tests you need to have a Blender build. For testing Python scripts you have to rebuild Blender after every change, so that it install the scripts for the tests.
https://wiki.blender.org/wiki/Tools/Tests/Setup

Also, this time I pushed the fixes straight, but should I e.g. open a branch and ask for review for the next update of the addon? I'll likely push all changes at once before the 2.81 release, for some final testing

Add-on authors generally update their own add-ons without code review.

Thanks.

To run the tests you need to have a Blender build. For testing Python scripts you have to rebuild Blender after every change, so that it install the scripts for the tests.

Yes I got this, I already build blender and test the addon with latest master.

But my question was: is there a way to run the tests on my latest changes before I push commits to the blender-addons repository? So I don't push commits that break tests. I'll be careful with imports and all moving forward, but you never know when you're going to forget something.

Yes I got this, I already build blender and test the addon with latest master.

But my question was: is there a way to run the tests on my latest changes before I push commits to the blender-addons repository? So I don't push commits that break tests. I'll be careful with imports and all moving forward, but you never know when you're going to forget something.

If you make changes in release/scripts/addons git submodule of the Blender repository, those will be tested.

I guess you have a blender-addons repository checkout separate from that? If so you can either switch to doing all editing and pushing from release/scripts/addons, or copy over the changes manually for testing.

Ah I didn't know I could work directly inside a submodule, although that'd be convenient! Thanks.

A bit of feedback:

  • The Playback enum in the header, doesn't need to to labeled 'Playback' it already says, so inside the drop down.
  • The Playback enum could be appended to the Timeline header or moved to the right in the Sequencer Header(away from the menus).
  • The Power Sequencer should not be exposed in the VSE Preview Header, since most of the functions are irrelevant to the Preview.
  • 'Set Timeline Range' seems to set the inpoint of the Range - which already is exposed in View > Range > Set Start Frame
  • PS > Markers > Marker Go To Next - seems to be the same as Marker > Jump to Next Marker
  • Imo, there is no need to repeat the name of the sub-menu ex. Markers > Marker Delete Closest or Playback > Playback Speed Increase
  • The default Fade(and only one exposed) is Fade In & Out, imo, the default should be Fade In. And the other options should be exposed too.
  • Does the Crossfade functions differ from the existing ones? If so, maybe it could be a patch to sequencer.py instead?
  • In general menu entries are not plural ex. Strips, Markers, Transitions.
  • Maybe a little confusion could be caused by what to look for in Edit, Trim and Strips. Are all elements here exposed in the most logic places(or in consistency with the build in menus)? Ex. many of the Strips-menu entries properly would be in Transform-menu if they where build in.
  • Make Still Image on a video gave an error when both video and audio was selected. Maybe rename to Freeze Frame(or Hold Frame(as in Hold Cut))? Stills in film production are photos taken during the shoot.
  • PS > Edit > Snap Selection seems to be the same as Strip > Transform > Snaps Strips to Playhead.