VSE: add Bicubic filtering option, and optimize bicubic performance #117100

Aras Pranckevicius · 2024-01-13T19:29:48+01:00

2024-01-13 19:29:48 +01:00

Part of overall "improve filtering situation" (#116980) task:

Add Bicubic filtering option to strip Transform "Filter" setting. Previously this option only existed in Transform Effect "Interpolation" setting.
- With this addition, it feels like the transform effect could possibly be marked as legacy/deprecated, since the regular Transform that is on all strips can do everything that Transform Effect did? I think.
Speed up bicubic filtering (used now in VSE, but also in CPU Compositor, image paint, etc.) by slightly simplifying the code and using some SIMD. Upscaling 96x54 image to 3840x2160 resolution, using Bicubic filtering:
- Windows (VS2022, Ryzen 5950X): 35.5ms -> 15.1ms
- Mac (clang 15, M1 Max): 29.6ms -> 24.4ms
Add gtest coverage for bicubic functionality.

Filtering look comparison, Nearest, Bilinear and (new) Bicubic. As expected, Bicubic is most useful when scaling images up.

The actual Bicubic filter kind was not changed, i.e. it's the same math that already existed in the code (as well as Cycles and Eevee). Turns out it's the "slightly blurry" kind, i.e. a Mitchell-Netrevali filter with B=1 C=0 parameters which is in the "stuff's blurry!" area, instead of B=C=1/3 as recommended by literature normally. But that's a question/topic for some other day.

Part of overall "improve filtering situation" (#116980) task: - Add Bicubic filtering option to strip Transform "Filter" setting. Previously this option only existed in Transform Effect "Interpolation" setting. - With this addition, it feels like the transform effect could possibly be marked as legacy/deprecated, since the regular Transform that is on all strips can do everything that Transform Effect did? I think. - Speed up bicubic filtering (used now in VSE, but also in CPU Compositor, image paint, etc.) by slightly simplifying the code and using some SIMD. Upscaling 96x54 image to 3840x2160 resolution, using Bicubic filtering: - Windows (VS2022, Ryzen 5950X): 35.5ms -> 15.1ms - Mac (clang 15, M1 Max): 29.6ms -> 24.4ms - Add gtest coverage for bicubic functionality. Filtering look comparison, Nearest, Bilinear and (new) Bicubic. As expected, Bicubic is most useful when scaling images up. ![](/attachments/f0b546bc-71b9-4516-911d-c4a17366e9bc) The actual Bicubic filter kind was not changed, i.e. it's the same math that already existed in the code (as well as Cycles and Eevee). Turns out it's the "slightly blurry" kind, i.e. a Mitchell-Netrevali filter with B=1 C=0 parameters which is in the "stuff's blurry!" area, instead of B=C=1/3 as recommended by literature normally. But that's a question/topic for some other day.

filters.png

294 KiB

👍 2 ❤️ 5

Aras Pranckevicius commented

2024-01-13 19:59:26 +01:00

@blender-bot build

Aras Pranckevicius changed title from ~~WIP: VSE: add Bicubic filtering option, and optimize bicubic performance~~ to VSE: add Bicubic filtering option, and optimize bicubic performance

2024-01-13 22:17:37 +01:00

Aras Pranckevicius added this to the Video Sequencer project 2024-01-13 22:17:53 +01:00

Aras Pranckevicius requested review from Sergey Sharybin 2024-01-13 22:18:04 +01:00

Sergey Sharybin commented

2024-01-15 12:26:09 +01:00

Thanks for looking into this!

For the development processes purposes such things are typically split into several incremental steps, for the ease of review, presentation, and possible re-iteration if something fishy is found out later on. In this case it would be much better to have the following commits:

Conversion from C to C++ (for which you don't really need a review, just ensure no warnings are introduced on different platforms)
Optimization of the existing code
Addition of Bicubic filtering in VSE

Is such a split something you'd have time for, or would you rather us to take care of it?

Thanks for looking into this! For the development processes purposes such things are typically split into several incremental steps, for the ease of review, presentation, and possible re-iteration if something fishy is found out later on. In this case it would be much better to have the following commits: - Conversion from C to C++ (for which you don't really need a review, just ensure no warnings are introduced on different platforms) - Optimization of the existing code - Addition of Bicubic filtering in VSE Is such a split something you'd have time for, or would you rather us to take care of it?

Aras Pranckevicius commented

2024-01-15 12:47:22 +01:00

@Sergey by "commits", you mean submit these steps as separate PRs? (since this PR does have multiple commits already, quite similar to your steps)

Sergey Sharybin commented

2024-01-15 12:54:55 +01:00

@aras_p I'd be fine if those are different commits in the single PR. At the end of the day it's probably will be me revieweing them anyway.

The commits are indeed organized almost like this. It feels that:

The PR description contains more interesting details. For example, it references the design task for the VSE part
The gcc fix seems need to be squished into the optimization commit
The cleanup commits needs to be squished to the commit which added detailed description of what bicubic means exactly

So easy janitorial to make git history cleaner and more readable, and easier to bisect.

@aras_p I'd be fine if those are different commits in the single PR. At the end of the day it's probably will be me revieweing them anyway. The commits are indeed organized almost like this. It feels that: - The PR description contains more interesting details. For example, it references the design task for the VSE part - The gcc fix seems need to be squished into the optimization commit - The cleanup commits needs to be squished to the commit which added detailed description of what bicubic means exactly So easy janitorial to make git history cleaner and more readable, and easier to bisect.

👍 1

Aras Pranckevicius force-pushed vse_bicubic from e2c40fdc5f to 1aba34e24e

2024-01-15 14:17:40 +01:00

Compare

Aras Pranckevicius commented

2024-01-15 14:18:51 +01:00

@blender-bot build

Sergey Sharybin approved these changes 2024-01-15 15:30:11 +01:00

Sergey Sharybin left a comment

Thanks for taking care of squishing the commits!

Aras Pranckevicius merged commit 709b00179f into main

2024-01-15 16:38:49 +01:00