In order to get this patch to compiled with strict flags I had to apply P1916.

Assuming those warnings are resolved, the new behavior seems to make more sense from user perspective than silently creating overlapping strips. Ont thing I've noticed is that if you have image and effect, and try to cut image without effect selected the effect strip is not cut and its length is only evaluated once you move strip. This is something to look into.

That being said, this change is not really a fix for the crash, but rather masquerades it. I will leave a comment in T84847 about what's going on and suggestion of how to move forward.

In D10209#255743, @Sergey Sharybin (sergey) wrote:

In order to get this patch to compiled with strict flags I had to apply P1916.

Assuming those warnings are resolved, the new behavior seems to make more sense from user perspective than silently creating overlapping strips. Ont thing I've noticed is that if you have image and effect, and try to cut image without effect selected the effect strip is not cut and its length is only evaluated once you move strip. This is something to look into.

---

That being said, this change is not really a fix for the crash, but rather masquerades it. I will leave a comment in T84847 about what's going on and suggestion of how to move forward.

Thanks I haven't checked the cause of crash before making this patch.

P1917 seems more reasonable as fix for 2.92 I would say. So I will change this patch to general development instead of bugfix.

Alternative version based on new iterator:

Do you have a simple .blend file where one can easily check behavior before and after? If so, please attach it in the description. Will make testing so much easier and more efficient :)

Oops, I must have screwed up diff in P2120. It looks like unfinished merge?

In any case my point was that I would rather follow this up in separate patch, because there will be number of unrelated changes.

Thing is, you have the following calls:

• SEQ_recursive_apply(seq, SEQ_apply_unique_name_fn, scene);
• SEQ_recursive_apply(iseq, SEQ_apply_unique_name_fn, scene);

And the change makes some low-level callback function SEQ_apply_unique_name_fn public.
So question is, why to expose a low-level callback instead of something like this:

/* Make sure sequence's name is unique. (And state explicitly what happens with the animation) */
SEQ_recursive_apply_unique_name(Scene *scene, Sequence *seq)
{
  SEQ_recursive_apply(iseq, seq_apply_unique_name_fn, scene);
}

In D10209#288721, @Sergey Sharybin (sergey) wrote:

Thing is, you have the following calls:

• SEQ_recursive_apply(seq, SEQ_apply_unique_name_fn, scene);
• SEQ_recursive_apply(iseq, SEQ_apply_unique_name_fn, scene);

And the change makes some low-level callback function SEQ_apply_unique_name_fn public.
So question is, why to expose a low-level callback instead of something like this:

/* Make sure sequence's name is unique. (And state explicitly what happens with the animation) */
SEQ_recursive_apply_unique_name(Scene *scene, Sequence *seq)
{
  SEQ_recursive_apply(iseq, seq_apply_unique_name_fn, scene);
}

I thought that this change would fit much better into broader cleanup in this area. But overall it doesn't matter that much.

The remaining bit is about the sorting. One confusing part is why SEQ_sort is capitalized and seq_sort_ex is not?
Maybe call it SEQ_sort_list/SEQ_sort_scene() ? And add a comment what exactly they are doing.

In D10209#288910, @Sergey Sharybin (sergey) wrote:

The remaining bit is about the sorting. One confusing part is why SEQ_sort is capitalized and seq_sort_ex is not?

Because SEQ_ is extern API (for operators, other modules), seq_ is intern.
I have been trying to make all code aware of seqbase that is being operated on so I should have changed SEQ_sort. This is because RNA functions don't always use active seqbase which is implicitly used by SEQ_sort

I will check code and how this function was used. I haven't thought about this aspect when I split the function.

I see.
I would avoid name seq_sort_ex as it is not an extended version of the SEQ_sort. Usually we use _ex suffix for functions which receive an extended list of arguments (instead of deducting them from a context, i.e.). seq_sort_seqbase with a comment (saying that it sorts in the order of dependencies) is easy to do and clear enough for this patch. More thorough refactor can happen aftewards.