Paths

Table of Contentst

Differential D10026

Fix T82698: Speed effect not working on generator strips
ClosedPublic
Actions

Authored by Richard Antalik (ISS) on Jan 7 2021, 6:05 AM.

Details

Reviewers

Sergey Sharybin (sergey)

Maniphest Tasks

T82698: The Video Sequencer Bugs

Commits

rB93c10797dc35: Fix T82698: Speed effect not working on generator strips

Summary

Generator strips with zero inputs have their length set to 1 pernamently.

In some cases it is useful to use speed effect on these strips because they
can be animated. This can be done by using their length as is on timeline as
content length.

This is very simplified and temporary solution, as cutting these strips won't
give expected results. Lot of code relies on length of these strips being fixed
to 1, resolving this properly should be done by T59540.

Diff Detail

Repository: rB Blender

Event Timeline

Richard Antalik (ISS) requested review of this revision.Jan 7 2021, 6:05 AM

Richard Antalik (ISS) created this revision.

Harbormaster completed remote builds in B12027: Diff 32502.Jan 7 2021, 6:06 AM

Think it is a bit more permanent than temporary, because it will take a bit until T59540 is handled.

source/blender/sequencer/intern/effects.c
3154	Add explanation why this is needed (mostly copy-paste info from the patch description). Reading comment in the code is easier than to track down commit and read its description.

Add more verbose comment

Harbormaster completed remote builds in B12245: Diff 32865.Jan 18 2021, 3:03 PM

In D10026#251748, @Sergey Sharybin (sergey) wrote:

Think it is a bit more permanent than temporary, because it will take a bit until T59540 is handled.

Yes, what I meant is that this is not supposed to be pernament solution - or in other words it's not designed to work this way.

in other words it's not designed to work this way

Code should always be following design.

Is the proposed change to the code violates design we should not accept the change. If the code improves something within the current design we should accept the change, regardless of whether design is known to be reworked in the coming time.

source/blender/sequencer/intern/effects.c
3199	`const`

use const

Harbormaster completed remote builds in B12284: Diff 32943.Jan 19 2021, 9:10 PM

In D10026#253445, @Sergey Sharybin (sergey) wrote:

in other words it's not designed to work this way

Code should always be following design.

Is the proposed change to the code violates design we should not accept the change. If the code improves something within the current design we should accept the change, regardless of whether design is known to be reworked in the coming time.

I was talking about very low level design, close to actual implementation, if one can call that a design. On high level, when I apply speed control on strip, I expect playback speed to change, which this patch does implement for this case. So it is improvement in usability I would say.

Lets move discussion between design vs. code vs. implementation to the chat :)

So it is improvement in usability I would say.

Then lets go ahead with the fix!

This revision is now accepted and ready to land.Jan 20 2021, 9:59 AM

Closed by commit rB93c10797dc35: Fix T82698: Speed effect not working on generator strips (authored by Richard Antalik (ISS)). · Explain WhyJan 25 2021, 5:08 AM

This revision was automatically updated to reflect the committed changes.

Richard Antalik (ISS) added a commit: rB93c10797dc35: Fix T82698: Speed effect not working on generator strips.

Revision Contents
Changeset List

Path

Size

source/

blender/

sequencer/

intern/

effects.c

28 lines

Diff 33088

View Options

source/blender/sequencer/intern/effects.c

Fix T82698: Speed effect not working on generator stripsClosedPublicActions