VSE: Unify sequences collections API
ClosedPublic
Actions

Authored by Richard Antalik (ISS) on Nov 19 2020, 9:18 AM.

Details

Reviewers

Sergey Sharybin (sergey)
Bastien Montagne (mont29)

Commits

rB13ca11ac52a8: VSE: Unify sequences collections API

Summary

Use RNA_api_sequences() for SequenceEditor and MetaSequence
sequences member.

Defines pair of dispatch functions rna_Sequences_editing_* and
rna_Sequences_meta_* that pass pointer to seqbase to
rna_Sequences_* function.

Downside of this implementation is, that it defines 2 seemingly
different RNA collections - SequencesMeta and SequencesTopLevel

Diff Detail

Repository: rB Blender

Event Timeline

Richard Antalik (ISS) created this revision.Nov 19 2020, 9:18 AM

Harbormaster completed remote builds in B11337: Diff 31206.Nov 19 2020, 9:18 AM

Richard Antalik (ISS) requested review of this revision.Nov 19 2020, 9:18 AM

This is quite tricky code, in a sense that it doesn't bring immediate "yeah, go for it" thoughts. Can you give some more context over what is the problem you're solving with the change? That would help understanding how/if the proposed change can be accepted.

In D9601#239136, @Sergey Sharybin (sergey) wrote:

This is quite tricky code, in a sense that it doesn't bring immediate "yeah, go for it" thoughts. Can you give some more context over what is the problem you're solving with the change? That would help understanding how/if the proposed change can be accepted.

We have API functions defined for bpy.types.SequenceEditor.sequences. These are quite essential functions such as adding / removing strips.
These functions are not defined for bpy.types.MetaSequence.sequences and so you can not operate on nested strips.

It would be nice to have API for both types defined.
Some functions operated on ed->seqbasep which is pointer to nested seqbase that is currently in view. Concern about this approach was raised here https://developer.blender.org/D6892#inline-63465

I am pretty much out of ideas how to implement this in cleaner way.
Maybe defining flag for RNA_def_function_flag that would pass self as void * to API functions, but I am not sure if that would be useful for anything else than this case.

I see. Thanks for explanation.

RNA is not really my proficiency. @Bastien Montagne (mont29), can we use your brain to find bestest achievable solution here?

Don't see much issues with this patch tbh...

There may be slightly 'cleaner' way to define that API helper struct using macros and passing a few names to it instead of a blind boolean, but the end result API-wise would be the same.

source/blender/makesrna/intern/rna_sequencer.c
1970	No PascalCase in UI labels, if you really need to update those use spaces.
2342–2343	No PascalCase in UI labels, if you really need to update those use spaces. And probably add a short tooltip too, for sake of clarity?

address inlines

Harbormaster completed remote builds in B11447: Diff 31431.Nov 30 2020, 5:21 AM

I have renamed SequencesEditing to SequencesTopLevel because the former is non-descriptive and confusing. I have also generated sphinx docs just to check if this isn't too confusing, and I would say it's quite usable.

Richard Antalik (ISS) edited the summary of this revision. (Show Details)Nov 30 2020, 5:35 AM

Just a note, but at some point the use of Sequence and Sequences in the meanings Strip and Strips, needs to be cleaned up in the API. That is inconsistent and confusing naming.

Anything named Sequence should be related to the full contents of an instance of the Sequence Editor. And the word Strip should be used for the units inside the Sequence Editor.

@Bastien Montagne (mont29) Can i leave the final decision to you here?

Ok, patch LGTM then.

This revision is now accepted and ready to land.Nov 30 2020, 11:12 AM

Closed by commit rB13ca11ac52a8: VSE: Unify sequences collections API (authored by Richard Antalik (ISS)). · Explain WhyDec 2 2020, 5:53 AM

This revision was automatically updated to reflect the committed changes.

Richard Antalik (ISS) added a commit: rB13ca11ac52a8: VSE: Unify sequences collections API.