Sequencer: restructuring the code layout #77580

New Issue

Richard Antalik · 2020-06-07T20:59:49+02:00

Richard Antalik commented

2020-06-07 20:59:49 +02:00

Note: This is initial proposal aimed to gather feedback. It is subject to change.

This is task to coordinate restructurization of majority of VSE code.
Goal is to break existing code to smaller files with relatively narrow scope, create layer isolating low-level BKE code from functions close to user (operators, RNA, drawing). There should be minimum functional changes in first stages, followed by cleanups with minor functional changes if necessary (logic consolidation).

Motivation:

Maintainability in general
To allow work on #59540, where it is important for drawing, RNA and operator code to follow exact same logic to produce correct results.
Make it easier and cleaner to make changes in rendering code

Since this will be relatively large operation, it will have to be done in few stages:

Move sequencer code from BKE.
18d7aeacf2
Split sequencer code into smaller chunks
3baf65975c
0f43fe7fa6
01db805a82
a5a302bd18
e5df87b1b9
Rename / refactor sequencer code
ea1c5a6c15
2a6a26bbd7
046ca0749a
Move as much as possible lower-level code from operators and RNA.
8ec6b34b8e
9e4a4c2e99
TODO:
sequencer_add.c (D9760: VSE: Refactor VSE strip loading code)
sequencer_edit.c (got multiple smaller patches for this file as well)
sequencer_select.c
rna_sequencer.c
Refactor operator and RNA code moving remaining low-level logic. Aim for limiting logic to iteration and calling sets of functions from "libraries".
13ca11ac52
TODO:
Add RNA API functions to sequencer API

Note: This is initial proposal aimed to gather feedback. It is subject to change. This is task to coordinate restructurization of majority of VSE code. Goal is to break existing code to smaller files with relatively narrow scope, create layer isolating low-level BKE code from functions close to user (operators, RNA, drawing). There should be minimum functional changes in first stages, followed by cleanups with minor functional changes if necessary (logic consolidation). Motivation: - Maintainability in general - To allow work on #59540, where it is important for drawing, RNA and operator code to follow exact same logic to produce correct results. - Make it easier and cleaner to make changes in rendering code Since this will be relatively large operation, it will have to be done in few stages: - [x] Move sequencer code from BKE. - 18d7aeacf2 - [x] Split sequencer code into smaller chunks - 3baf65975c - 0f43fe7fa6 - 01db805a82 - a5a302bd18 - e5df87b1b9 - [x] Rename / refactor sequencer code - ea1c5a6c15 - 2a6a26bbd7 - 046ca0749a - [ ] Move as much as possible lower-level code from operators and RNA. - 8ec6b34b8e - 9e4a4c2e99 - TODO: - sequencer_add.c ([D9760: VSE: Refactor VSE strip loading code](https://archive.blender.org/developer/D9760)) - sequencer_edit.c (got multiple smaller patches for this file as well) - sequencer_select.c - rna_sequencer.c - [ ] Refactor operator and RNA code moving remaining low-level logic. Aim for limiting logic to iteration and calling sets of functions from "libraries". - 13ca11ac52 - TODO: - Add RNA API functions to sequencer API

Richard Antalik self-assigned this 2020-06-07 20:59:49 +02:00

Richard Antalik commented

2020-06-07 20:59:49 +02:00

Added subscriber: @iss

Ankit Meel commented

2020-06-07 21:03:16 +02:00

Added subscriber: @ankitm

Stanislav Ovcharov commented

2020-06-08 06:27:24 +02:00

Added subscriber: @StanislavOvcharov

Răzvan Cosmin Rădulescu commented

2020-06-08 09:22:00 +02:00

Added subscriber: @razcore

Marcos Perez commented

2020-06-08 10:45:28 +02:00

Added subscriber: @pistolario

Manuel Albert commented

2020-06-09 07:56:18 +02:00

Added subscriber: @ManuelAlbert

Pipeliner commented

2020-06-10 16:31:25 +02:00

Added subscriber: @Pipeliner

Duarte Farrajota Ramos commented

2020-06-10 20:07:05 +02:00

Added subscriber: @DuarteRamos

Andy Cuccaro commented

2020-06-11 05:36:02 +02:00

Added subscriber: @AndyCuccaro

Sergey Sharybin commented

2020-06-12 13:24:25 +02:00

Added subscribers: @brecht, @Sergey

Sergey Sharybin commented

2020-06-12 13:24:25 +02:00

Generally is super nice to have sequencer better organized from the code point of view, and de-duplicate logic as much as possible.

As for the locations of the files I don't think they belong to editors. Editor API should not be used by render pipeline, for example. To me it seems that sequencer should become more top-level folder, like compositor is, and used by render pipeline, sequencer space and so on.

As for the approach of how to structure the changes:

Try to move files as-is to the neighbourhood of their destination
Split them up at their final destination

Moving files can happen as a bulk operation, since that'd be annoying to keep incrementally modify entire source code. Splitting up and other changes try to keep as isolated as possible, in a stream of commits.

For the planing, it seems like this is an involved change, and we have quite reasonable amount of reports which doesn't seem to be requiring such refactor. To me it seems for 2.90 is better to do stabilization pass, and aim this refactor for 2.91. Hence i'll place this to 2.91 column. Surely, some work can start early on, but finalized (with all the cleanup and de-duplication you've mentioned) is unlikely to happen for 2.90 due to time constraints.

This is my vision anyway. Since there is some source layout/etc involved, wouldn't mind if @brecht shares his strong opinion :)

Generally is super nice to have sequencer better organized from the code point of view, and de-duplicate logic as much as possible. As for the locations of the files I don't think they belong to editors. Editor API should not be used by render pipeline, for example. To me it seems that sequencer should become more top-level folder, like `compositor` is, and used by render pipeline, sequencer space and so on. As for the approach of how to structure the changes: * Try to move files as-is to the neighbourhood of their destination * Split them up at their final destination Moving files can happen as a bulk operation, since that'd be annoying to keep incrementally modify entire source code. Splitting up and other changes try to keep as isolated as possible, in a stream of commits. For the planing, it seems like this is an involved change, and we have quite reasonable amount of reports which doesn't seem to be requiring such refactor. To me it seems for 2.90 is better to do stabilization pass, and aim this refactor for 2.91. Hence i'll place this to 2.91 column. Surely, some work can start early on, but finalized (with all the cleanup and de-duplication you've mentioned) is unlikely to happen for 2.90 due to time constraints. This is my vision anyway. Since there is some source layout/etc involved, wouldn't mind if @brecht shares his strong opinion :)

Brecht Van Lommel commented

2020-06-15 18:27:38 +02:00

+1 to everything @Sergey said.

Richard Antalik commented

2020-06-15 20:04:22 +02:00

Thanks for input, I agree with all points here, so I will change description with more discrete steps that can be checked out. Will have to look at compositor code organization, but I suspect that I will probably want to keep alloc/free functions in BKE if I wanted to make strip a ID at some point.

Dalai Felinto commented

2020-06-24 17:14:06 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Vincent Girès commented

2020-09-03 15:57:35 +02:00

Added subscriber: @VincentGires

tintwotin commented

2020-09-10 08:37:03 +02:00

Added subscriber: @tintwotin

tintwotin commented

2020-09-10 08:37:03 +02:00

Just curious, as I see a lot of files being moved around now for this project, but has the advantages of moving the Sequencer data out of Scene data, so Scenes can be switched without switching Sequencer contents, and vice versa, been taken into consideration?
{F8860684, size = full}

Some of the benefits:

Having the sequencer scene strip preview in the 3d view((no switching)good for storyboarding, previz etc.
A source and a master edit preview in the same workspace(as the VSE 2.0 mock-up, but both previews are Sequencer previews(just different Sequences))
Pancake editing workspace(all footage in one sequence and all edited material in another) in the same workspace.
Accessing sequencer-data-block from everywhere like node editor, 3d view background(ex. grease pencil drawing on edited sequence) and texture material, which can be manipulated in node editor and returned to VSE as scene strip.

{F8860669, size=full}

More on the advantages here on RCS: https://blender.community/c/rightclickselect/kWbbbc/

Just curious, as I see a lot of files being moved around now for this project, but has the advantages of moving the Sequencer data out of Scene data, so Scenes can be switched without switching Sequencer contents, and vice versa, been taken into consideration? {[F8860684](https://archive.blender.org/developer/F8860684/image.png), size = full} Some of the benefits: - Having the sequencer scene strip preview in the 3d view((no switching)good for storyboarding, previz etc. - A source and a master edit preview in the same workspace(as the VSE 2.0 mock-up, but both previews are Sequencer previews(just different Sequences)) - Pancake editing workspace(all footage in one sequence and all edited material in another) in the same workspace. - Accessing sequencer-data-block from everywhere like node editor, 3d view background(ex. grease pencil drawing on edited sequence) and texture material, which can be manipulated in node editor and returned to VSE as scene strip. {[F8860669](https://archive.blender.org/developer/F8860669/comp2vse2.gif), size=full} More on the advantages here on RCS: https://blender.community/c/rightclickselect/kWbbbc/

Andy Cuccaro commented

2020-09-10 15:40:48 +02:00

I fully support Peter's proposal of moving the sequencer out of the scene. I've been thinking a lot lately about how the sequencer fits into the Blender file/scene hierarchy (and I'll make a mockup of some ideas I have on this and on the compositor as soon as I can).

In the end, the sequence should be the final step of the production, no? Thus, I think it's more logical to have the sequence be the "parent" of the many scenes within the blend file than the other way around. And, like Peter mentions in his proposal, it could open the gates for a more streamlined workflow for storyboarding, for example.

Please, please consider doing this change.

I fully support Peter's proposal of moving the sequencer out of the scene. I've been thinking a lot lately about how the sequencer fits into the Blender file/scene hierarchy (and I'll make a mockup of some ideas I have on this and on the compositor as soon as I can). In the end, the sequence should be the final step of the production, no? Thus, I think it's more logical to have the sequence be the "parent" of the many scenes within the blend file than the other way around. And, like Peter mentions in his proposal, it could open the gates for a more streamlined workflow for storyboarding, for example. Please, please consider doing this change.

Brecht Van Lommel commented

2020-09-10 17:44:15 +02:00

This task is purely about restructuring the code layout, please move discussion about changes to functionality elsewhere.

Brecht Van Lommel changed title from ~~Gigantic VSE refactor~~ to Sequencer: restructing the code layout

2020-09-10 17:45:01 +02:00

Brecht Van Lommel changed title from ~~Sequencer: restructing the code layout~~ to Sequencer: restructuring the code layout

2020-09-10 17:45:17 +02:00

tintwotin commented

2020-09-15 19:57:24 +02:00

In #77580#1013237, @brecht wrote:
This task is purely about restructuring the code layout, please move discussion about changes to functionality elsewhere.

It has been at RCS since aug. 2018, but no devs there to discuss it: https://blender.community/c/rightclickselect/kWbbbc/

> In #77580#1013237, @brecht wrote: > This task is purely about restructuring the code layout, please move discussion about changes to functionality elsewhere. It has been at RCS since aug. 2018, but no devs there to discuss it: https://blender.community/c/rightclickselect/kWbbbc/

Campbell Barton commented

2020-10-05 07:35:52 +02:00

Added subscriber: @ideasman42

Campbell Barton commented

2020-10-05 07:35:52 +02:00

As of 18d7aeacf2 there is:

./source/blender/sequencer/BKE_sequencer.h (outside of ./source/blender/blenkernel).

I think we shouldn't leave partially finished module re-arrangement in this state.

While multiple commits to move and rename can be OK, it's leaving a mess to keep a module split-up and spread across different directories - longer term.

As of 18d7aeacf2 there is: `./source/blender/sequencer/BKE_sequencer.h` (outside of `./source/blender/blenkernel`). I think we shouldn't leave partially finished module re-arrangement in this state. While multiple commits to move and rename can be OK, it's leaving a mess to keep a module split-up and spread across different directories - longer term.

Richard Antalik commented

2020-10-05 07:59:53 +02:00

I will clean up that file on one or two commits (split + rename).

There will be still bunch fo functions with BKE prefix, so unless this is done in one go, there will have to be undesirable state for some time.

I will try to resolve this state in first place and as quickly as I can. I already got one file split, but perhaps it would be better to start with BKE_sequencer.h actually - it would make subsequent splits even easier.

I will clean up that file on one or two commits (split + rename). There will be still bunch fo functions with BKE prefix, so unless this is done in one go, there will have to be undesirable state for some time. I will try to resolve this state in first place and as quickly as I can. I already got one file split, but perhaps it would be better to start with `BKE_sequencer.h` actually - it would make subsequent splits even easier.

ok what commented

2021-12-15 11:04:05 +01:00

Added subscriber: @ok_what

Richard Antalik added this to the Video Sequencer project 2023-02-09 20:57:27 +01:00

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Sequencer: restructuring the code layout #77580