Page MenuHome

VSE: Added filter mode to strip transform
ClosedPublic

Authored by Dimitry Kaplin (DimKa) on Oct 9 2021, 6:37 PM.

Details

Summary

Problem

  • Strip A is transformed
  • Strip A is blended with underlying Strip B
  • Underlying Strip B is very contrast

    In that case you will see aliased sharp border of Strip A in Preview

    But in Render - you will see dark "smooth" border of Strip A

    View:
    Setup:

Reason

This happens because the filtration mode of transformed strips is hardcoded to:

  • Bilinear if render
  • Nearest otherwise

Solution
Let user decide what type of filtration it wants to use.

Result

  • Nearest
  • Bilinear

Diff Detail

Repository
rB Blender

Event Timeline

Dimitry Kaplin (DimKa) requested review of this revision.Oct 9 2021, 6:37 PM
Dimitry Kaplin (DimKa) created this revision.
Dimitry Kaplin (DimKa) edited the summary of this revision. (Show Details)Oct 9 2021, 6:45 PM
Dimitry Kaplin (DimKa) updated this revision to Diff 43092.EditedOct 9 2021, 10:29 PM

Backward compatibility

  • Bilinear set as default filter (as it was for render)
  • Bilinear set as StripTransform filter if open an old file where wasn't specified

The current behavior seems like a good default, with this patch the default behavior for the sequencer will be slower.

Why not have an AUTO option that keeps the current behavior?

The current behavior seems like a good default, with this patch the default behavior for the sequencer will be slower.

Why not have an AUTO option that keeps the current behavior?

Current behavior is quite frustrating. You see one image in preview and another in render. This breaks concept of WYSIWYG editor.

My opinion is - getting correct output image is more desirable rather than saving few CPU cycles.

AUTO option as default will hide the problem from user (as it works now).
Nearest filter as default option will cause render result differs from previous version (test were failing because of this BTW).
Bilinear - preserves the same render result, and shows it to user before render.

I don't think performance degradation in editor will be noticeable, even if all 32 channels will be in use.

However, if it's critical - such option might be added but to Preview settings.
So, if user will see a significant performance drop - it may force enable nearest filtering in the preview.

When this new system was added, bilinear interpolation was significantly slower than nearest. This has been optimized in 28617bb16798 significantly, but nearest interpolation is still 2x faster. Interpolation method can be also artistic choice, but this is preserved in transform strip still so it is not strictly necessary for transform property, and that is only reason why it isn't included.

Having setting for quality vs performance was suggested by @Sergey Sharybin (sergey) in T80278 though in different context, but still if we do this kind of optimizations, it could cover this case as well perhaps.

Being fast but inaccurate in preview is not ideal. With an optimized interpolation it think it totally worth having Bilinear by default.

Not sure it should be an option, or just always use bi-linear.
If we go with option, then the default should be bi-linear with do-version, so that this patch does not affect final render of existing files.

Also, have a look at interpolation_items in rna_def_transform. It does support bi-cubic as well, and this matches Transform node in the Compositor.

And last but not least, with the Transform effect there is Interpolation and Filter exposed to artists. One issue with this is this is that it exposes same concept under different names. Another issue is that it is unclear which exact filtering will be used.

As long as users don't find this a regression and report the sequencer being noticeably slower in 3.0 - I don't have an issue with changing behavior.
In this case though, it could always use high quality interpolation (without adding options).

Note that we could use nearest only while transforming if the difference is noticeable.

but this is preserved in transform strip still so it is not strictly necessary for transform property

Yes, transform strip has such option, but it doesn't affect on render of the applied strip:

Here is my setup (blender 2.93.5 Steam):

  • Background white strip
  • Foreground green strip with 45 degrees rotation (I know that it could be rotated with transform strip, but this is imitation of 2d stabilization, which was the origin of this patch)
  • Transform strip without any transformations but applied filter

None
Work as expected - dark unexpected outline

Bilinear
Nothing changed

Bicubic
My suggestion: first the strip was rendered with bilinear filter applied, then transform strip applied its bicubic filter.
But again, preview image doesn't match final render.

Also, have a look at interpolation_items in rna_def_transform. It does support bi-cubic as well, and this matches Transform node in the Compositor.

And last but not least, with the Transform effect there is Interpolation and Filter exposed to artists. One issue with this is this is that it exposes same concept under different names. Another issue is that it is unclear which exact filtering will be used.

I absolutely agree that it could be frustrating to have "filter" and "interpolation" properties in one strip. However the Transform strip has doubled, position, rotation and scale properties, which might be already confusing.
I think it would be great if suggested StripTranform filter property also had a 'bicubic' option, but IMB_transform function does not support it.

And I think it might help if I explain the origin of this patch:

I did some video editing of my shoots from GoPro. Due to the specific of the video it was very shaky and I need to stabilize it.
The camera did a lot of rotations around its "forward" axis, so my desire was to compensate this rotations.
As you could expect when your 2.7k 4:3 view has a lot of rotation even around fixed point, the best you can get - is highly scaled image where a lot of details near borders are lost.
But since the camera rotations are not constant (happens time to time) you can keep relatively small scale, and when 2D stabilization rotates the video small artifacts on corners.
Like this:


So the solution is simple - duplicate stabilized movie strip, apply additional scale x1.2, and gaussian blur (similar effect you could see on youtube, when someone inserts vertical video, you can see blurred and scaled copy of vertical video on the sides)
So you do this and see everything is great in preview, but when you watch the final video - you see artifacts:

Yes, I understand that full hide of such artifacts is possible when the colors are pretty same, however, it does better results with nearest filtering in my opinion:

BilinearNearest

@Dimitry Kaplin (DimKa) If I understand your last comment correctly, you want to render with nearest interpolation, not bilinear, right?

Being fast but inaccurate in preview is not ideal. With an optimized interpolation it think it totally worth having Bilinear by default.

Not sure it should be an option, or just always use bi-linear.

I think this really should be an option (see below).

And last but not least, with the Transform effect there is Interpolation and Filter exposed to artists. One issue with this is this is that it exposes same concept under different names. Another issue is that it is unclear which exact filtering will be used.

The one reason why I want this to be an option is, because I want to remove transform effect completely - it would duplicate functionality and existing transform tools can't work with it. Though I haven't discussed this much, so it may not happen too. it's just mentioned as possibility in design task. Another reasons I stated earlier - it can be artistic choice and also it impacts performance.

Also, have a look at interpolation_items in rna_def_transform. It does support bi-cubic as well, and this matches Transform node in the Compositor.

It uses different API now, so bicubic interpolation have to be added first to this API.

As long as users don't find this a regression and report the sequencer being noticeably slower in 3.0 - I don't have an issue with changing behavior.
In this case though, it could always use high quality interpolation (without adding options).

Note that we could use nearest only while transforming if the difference is noticeable.

There was post on devtalk, that using integer values may not be best idea either, so I think that should be changed too. See https://devtalk.blender.org/t/vse-transform-position-attribute-issue/20774/3
As far as performance goes, I mentioned earlier, that playback speed is 2x worse with bilinear interpolation, so I am not really sure if that is best option.

but this is preserved in transform strip still so it is not strictly necessary for transform property

Yes, transform strip has such option, but it doesn't affect on render of the applied strip:

It does, but in your setup the transform effect strip does not do any transformation - values are at defaults. As was mentioned, this is quite confusing when we have 2 nearly identical ways to transform images.


To summarize my position, I agree with approach in this patch - user should decide what interpolation will be used.

To summarize my position, I agree with approach in this patch - user should decide what interpolation will be used.

We should be making it easier and more clear how to use sequencer. In the current state of the patch the Transform strip becomes very confusing: there are two interpolations options, named differently, one of which allows bicubic and other does not.
You've mentioned deprecation.removal of the Transform strip. Is there task for it? Is it high in the priority list?

Also, files saved in the current master branch will have interpolation set to Nearest, which is an unnecessary backwards compatibility breakage.

To summarize my position, I agree with approach in this patch - user should decide what interpolation will be used.

We should be making it easier and more clear how to use sequencer. In the current state of the patch the Transform strip becomes very confusing: there are two interpolations options, named differently, one of which allows bicubic and other does not.
You've mentioned deprecation.removal of the Transform strip. Is there task for it? Is it high in the priority list?

It's mentioned in T90156 in last paragraph, but I haven't discussed this point so far, so priority is not assesed. So I already assume there will be only one set of these properties. Here I commented only on what this patch proposes without considering wider context.

Sorry for delay here, I wanted to commit this with other related changes, but missed the opeortunity during 3.1. Will accept now.

This revision is now accepted and ready to land.Feb 7 2022, 9:51 AM
This revision was automatically updated to reflect the committed changes.