WIP: VSE use new frequency equalizer template. #117700

Nate Rupsis · 2024-01-31T16:45:09+01:00

Nate Rupsis commented

2024-01-31 16:45:09 +01:00

The existing template_curve_mapping UI template is a bit overloaded, and doesn't fit all that well for the VSE Sound equalizer.

This dev talk thread highlights some fundamental issues that require a new template.

Old	New

The existing `template_curve_mapping` UI template is a bit overloaded, and doesn't fit all that well for the VSE Sound equalizer. This [dev talk thread](https://devtalk.blender.org/t/vse-and-sound-state-future/21904/95) highlights some fundamental issues that require a new template. | Old| New | | -------- | -------- | | ![image](/attachments/c0882f87-d397-48ab-9e32-834350b397ae) | ![image](/attachments/7325bce0-745f-44df-ac31-35720eca9901) |

image.png

216 KiB

image.png

61 KiB

Nate Rupsis added 2 commits 2024-01-31 16:45:19 +01:00

f8c91c4cdc duplicate template curve mapping, and make new frequency mapping template

035f28ef91 Merge branch 'main' into VSE-equalizer-clipping

Nate Rupsis added 1 commit 2024-01-31 16:52:13 +01:00

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

15888a84a5 switching back to X/Y for curve

Brecht Van Lommel reviewed 2024-01-31 19:41:22 +01:00

Brecht Van Lommel left a comment

This is WIP so not sure if it's planned to be addressed.

But this doesn't need to duplicate so much code. The few different labels and parameters can be put into e.g. a temporary struct TemplateCurveMapping that is passed to a common function, filled by uiTemplateFrequencyMapping and uiTemplateCurveMapping with appropriate defaults.

Also not sure it makes sense to give this a generic name like "frequency mapping" rather than "sound equalizer". Do we expect this to make sense for other types of frequencies?

This is WIP so not sure if it's planned to be addressed. But this doesn't need to duplicate so much code. The few different labels and parameters can be put into e.g. a temporary `struct TemplateCurveMapping` that is passed to a common function, filled by `uiTemplateFrequencyMapping` and `uiTemplateCurveMapping` with appropriate defaults. Also not sure it makes sense to give this a generic name like "frequency mapping" rather than "sound equalizer". Do we expect this to make sense for other types of frequencies?

Nate Rupsis commented

2024-01-31 19:45:22 +01:00

@blender-bot package

Blender Bot commented

2024-01-31 19:45:24 +01:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR117700) when ready.

Nate Rupsis commented

2024-01-31 20:06:22 +01:00

@brecht

This is WIP so not sure if it's planned to be addressed.

But this doesn't need to duplicate so much code. The few different labels and parameters can be put into e.g. a temporary struct TemplateCurveMapping that is passed to a common function, filled by uiTemplateFrequencyMapping and uiTemplateCurveMapping with appropriate defaults.

Also not sure it makes sense to give this a generic name like "frequency mapping" rather than "sound equalizer". Do we expect this to make sense for other types of frequencies?

Initial WIP was to get the proof of concept out, and see what people think of it. I 100% agree that there's too much code duplication going on. I didn't want to spend that time until I had community buy-in.

IMO, "sound equalizer" makes the most sense. I chose "frequency mapping" to keep things open ended for the moment. Unsure if it would be useful for other frequencies or not.

@brecht > This is WIP so not sure if it's planned to be addressed. > > But this doesn't need to duplicate so much code. The few different labels and parameters can be put into e.g. a temporary `struct TemplateCurveMapping` that is passed to a common function, filled by `uiTemplateFrequencyMapping` and `uiTemplateCurveMapping` with appropriate defaults. > > Also not sure it makes sense to give this a generic name like "frequency mapping" rather than "sound equalizer". Do we expect this to make sense for other types of frequencies? Initial WIP was to get the proof of concept out, and see what people think of it. I 100% agree that there's too much code duplication going on. I didn't want to spend that time until I had community buy-in. IMO, "sound equalizer" makes the most sense. I chose "frequency mapping" to keep things open ended for the moment. Unsure if it would be useful for other frequencies or not.

Gangneron commented

2024-01-31 22:01:24 +01:00

First-time contributor

I think it would be more judicious to put min dB and max dB in a row, this would make the flow simpler and more coherent
But else it's good idea and more easy

I think it would be more judicious to put min dB and max dB in a row, this would make the flow simpler and more coherent But else it's good idea and more easy

Marcos Perez commented

2024-02-01 10:56:22 +01:00

I think that it is a good idea to separate this template from the original template.

As a first case, change the literals of the clipping, but then, the buttons layout or other gui elements could be more sound oriented

I think that it is a good idea to separate this template from the original template. As a first case, change the literals of the clipping, but then, the buttons layout or other gui elements could be more sound oriented

tintwotin commented

2024-02-02 09:11:06 +01:00

First-time contributor

The key values could also have units exposed.

Maybe losing the x and y labels will add some confusion? If the units(dB & Hz) were implemented as actual units, they could be added after the numbers, and the Min X/Y etc. could be kept. This way it may be more obvious to users that x range is in Hz and the y range is in dB then?

But if that is too much trouble, the current suggestion is enough to go with that, over the current implementation.

The key values could also have units exposed. Maybe losing the x and y labels will add some confusion? If the units(dB & Hz) were implemented as actual units, they could be added after the numbers, and the Min X/Y etc. could be kept. This way it may be more obvious to users that x range is in Hz and the y range is in dB then? But if that is too much trouble, the current suggestion is enough to go with that, over the current implementation.

Nate Rupsis added 2 commits 2024-02-13 16:13:15 +01:00

03aa6b12ff Merge branch 'main' into VSE-equalizer-clipping

784e7984d5 update template name to sound equalizer; use dB/Hz for curve position

Nate Rupsis added 1 commit 2024-02-13 16:16:27 +01:00

b6bcab5258 missed some files

This pull request has changes conflicting with the target branch.

scripts/startup/bl_ui/space_sequencer.py
source/blender/editors/interface/interface_templates.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u VSE-equalizer-clipping:nrupsis-VSE-equalizer-clipping

git checkout nrupsis-VSE-equalizer-clipping

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

WIP: VSE use new frequency equalizer template. #117700

Checkout