Page MenuHome
Differential D10362

VSE: Simplify proxy settings
ClosedPublic
Actions

Authored by Richard Antalik (ISS) on Feb 8 2021, 11:10 PM.

Details

Reviewers
Sergey Sharybin (sergey)
Francesco Siddi (fsiddi)
Maniphest Tasks
T85469: VSE 2.0: Proxy system
Commits
rB91561629cd0b: VSE: Simplify proxy settings
Summary
  • Remove Full Render size from VSE preview size. Use just 100% instead
  • Add Use Proxies checkbox to control whether proxies are used globally
  • Move preview size to top so it is most prominent
  • Set default to 100% preview size and use proxies (could be separate patch as well)

Design task: T85469

No change has been done to individual strip setting as users may need to turn proxies on/off individually.
I think it would be best if size selection will be managed when automatic proxy building is enabled. In that case proxy panel can be simplified a lot. This is probalby better to leave for separate patch

Diff Detail

Repository
rB Blender
Branch
improve-proxy-ui (branched from master)
Build Status
Buildable 12732
Build 12732: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) created this revision.Feb 8 2021, 11:10 PM
Harbormaster completed remote builds in B12732: Diff 33734.Feb 8 2021, 11:10 PM
Richard Antalik (ISS) requested review of this revision.Feb 8 2021, 11:10 PM
Richard Antalik (ISS) edited the summary of this revision. (Show Details)Feb 8 2021, 11:15 PM
Richard Antalik (ISS) updated this revision to Diff 33736.Feb 8 2021, 11:19 PM
  • Rename "Proxy Render Size" to "Preview Size"
Harbormaster completed remote builds in B12734: Diff 33736.Feb 8 2021, 11:19 PM
Richard Antalik (ISS) edited the summary of this revision. (Show Details)Feb 9 2021, 12:16 AM
Aaron Carlisle (Blendify) added a subscriber: Aaron Carlisle (Blendify).Feb 9 2021, 1:35 AM

Here is an alternative UI where both proxy settings are on one line

Note, I chose "Proxy Preview" to not confuse the idea that proxies might affect the final render.

Richard Antalik (ISS) added subscribers: Hans Goudey (HooglyBoogly), Julian Eisel (Severin).Feb 9 2021, 1:41 AM

@Hans Goudey (HooglyBoogly), @Julian Eisel (Severin) Notifying UI if you want to review this patch.

Peter Fog (tintwotin) added a subscriber: Peter Fog (tintwotin).Feb 9 2021, 11:01 AM

It is very nice with the separation of Proxy and Preview settings, with noticeable speed gains, when switching on proxy:

However, I wonder what the use is, for the Preview Setting(Proxy unchecked), since it is by large only degrading the quality without improvement in performance(75% and 50% is worse than 100%):

Btw. notice the color shift when toggling the proxy in the first gif - this seems like a bug unrelated to this patch. Test video: https://www.youtube.com/watch?v=xZaVniqCSyQ

I did a write up on the current state of the vse proxy-system here: https://devtalk.blender.org/t/a-complete-redesign-of-the-vse-proxy-system-is-needed/17491

Richard Antalik (ISS) added a task: T85469: VSE 2.0: Proxy system.Feb 22 2021, 9:42 PM
Peter Fog (tintwotin) added a comment.Mar 15 2021, 7:56 PM

I wonder what the use is, for the Preview Setting(Proxy unchecked), since it is by large only degrading the quality without improvement in performance(75% and 50%) is worse than 100%?

Are you guys sure the unchecked 'Use Proxy' actually is useful for anything? If it isn't it might as well be removed.

Richard Antalik (ISS) added a comment.Mar 15 2021, 8:22 PM
In D10362#271854, @Peter Fog (tintwotin) wrote:

I wonder what the use is, for the Preview Setting(Proxy unchecked), since it is by large only degrading the quality without improvement in performance(75% and 50%) is worse than 100%?

Are you guys sure the unchecked 'Use Proxy' actually is useful for anything? If it isn't it might as well be removed.

It's useful when you don't want proxies to be used. I sometimes uncheck it just to see if I want to be sure that original image looks as I want it to look.

Peter Fog (tintwotin) added a comment.EditedMar 15 2021, 8:48 PM
In D10362#271855, @Richard Antalik (ISS) wrote:
In D10362#271854, @Peter Fog (tintwotin) wrote:

I wonder what the use is, for the Preview Setting(Proxy unchecked), since it is by large only degrading the quality without improvement in performance(75% and 50%) is worse than 100%?

Are you guys sure the unchecked 'Use Proxy' actually is useful for anything? If it isn't it might as well be removed.

It's useful when you don't want proxies to be used. I sometimes uncheck it just to see if I want to be sure that original image looks as I want it to look.

But it that case, you do not want to see a 25% downscaled bad quality version of your original footage, you want to see it at full resolution, I mean there is no use for the downscaled, un-proxied, bad performance, bad quality preview option. When switching off proxies, you should also switch off all downscaling. The button toggle proxies+scaling on and off.

Sergey Sharybin (sergey) added a comment.Mar 16 2021, 10:43 AM

Unfortunately, the patch doesn't seem to apply cleanly on the latest master, and with older one I run into issues with libraries.

From just looking at the code is a bit hard to say for sure: are the settings moved from strip to preview? Ultimately, we don't want 2 places which control behavior, so the control should only be done in the preview.

For moving forward:

  • Update the patch against latest master
  • Get Francesco to give it a pass of review

Outside of review. Did you think of adding something like "Auto" option, which will choose proxy size which suits best the preview size? Think this is what we ultimately want for an optimal performance/

Richard Antalik (ISS) added a comment.Mar 16 2021, 11:37 AM
In D10362#272077, @Sergey Sharybin (sergey) wrote:

Unfortunately, the patch doesn't seem to apply cleanly on the latest master, and with older one I run into issues with libraries.

Got no problems here but will update rebased patch.

are the settings moved from strip to preview? Ultimately, we don't want 2 places which control behavior, so the control should only be done in the preview.

There are no functional changes here technically, nothing is moved it's just more clearer representation of current possibilities.
These 3 patches should make it, such that user would not need to go to proxy settings panel. All should work out of the box. But he still can go there and do manual changes. Again we can hide this panel as was done with cache panel.

Outside of review. Did you think of adding something like "Auto" option, which will choose proxy size which suits best the preview size? Think this is what we ultimately want for an optimal performance/

That patch is worked on still (not much lately though), with using proxies by default this may require some introspection of built sizes and prioritizing closest one, so I say that yes, this is what we want.

Richard Antalik (ISS) updated this revision to Diff 35241.Mar 16 2021, 11:38 AM

Rebase

Harbormaster completed remote builds in B13534: Diff 35241.Mar 16 2021, 11:38 AM
Richard Antalik (ISS) updated this revision to Diff 35242.Mar 16 2021, 11:52 AM
  • Oops my head wasn't in sync with origin, rebasing again
Harbormaster completed remote builds in B13535: Diff 35242.Mar 16 2021, 11:53 AM
Peter Fog (tintwotin) removed a subscriber: Peter Fog (tintwotin).Mar 16 2021, 11:58 AM
Sergey Sharybin (sergey) accepted this revision.Mar 16 2021, 12:21 PM
Sergey Sharybin (sergey) added 2 blocking reviewer(s): Sergey Sharybin (sergey), Francesco Siddi (fsiddi).

There are no functional changes here technically, nothing is moved it's just more clearer representation of current possibilities.

Ah ok. Thanks for the clarification.

Accepting as the code seems fine. The interface we will review with Francesco and let you know soon.

Francesco Siddi (fsiddi) added a comment.Mar 16 2021, 3:37 PM

Reviewed together with Sergey. Original proposal is good, with the following feedback:

  • Move the "Use Proxies" option at the beginning of the list
  • Move the "Render Size" below and have it grayed-out when "Use Proxies" is not enabled
Francesco Siddi (fsiddi) accepted this revision.Mar 16 2021, 4:47 PM

Accepted, agreeing that there will be further iteration on how the UI will display proxy resolution.

This revision is now accepted and ready to land.Mar 16 2021, 4:47 PM
Richard Antalik (ISS) updated this revision to Diff 35265.Mar 16 2021, 5:50 PM
  • Bump subversion
Harbormaster completed remote builds in B13547: Diff 35265.Mar 16 2021, 5:50 PM
Aaron Carlisle (Blendify) added a comment.Mar 16 2021, 6:31 PM
In D10362#272200, @Francesco Siddi (fsiddi) wrote:

Reviewed together with Sergey. Original proposal is good, with the following feedback:

  • Move the "Use Proxies" option at the beginning of the list
  • Move the "Render Size" below and have it grayed-out when "Use Proxies" is not enabled

Alternatively, these can be placed on one row:

Closed by commit rB91561629cd0b: VSE: Simplify proxy settings (authored by Richard Antalik (ISS)). · Explain WhyMar 16 2021, 6:59 PM
This revision was automatically updated to reflect the committed changes.
Richard Antalik (ISS) added a commit: rB91561629cd0b: VSE: Simplify proxy settings.

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
16 lines
source/
blender/
blenloader/
intern/
35 lines
3 lines
editors/
space_sequencer/
1 line
makesdna/
3 lines
makesrna/
intern/
17 lines
sequencer/
3 lines
1 line
intern/
7 lines
5 lines

Diff 33734

View Options

release/scripts/startup/bl_ui/space_sequencer.py

Loading...
View Options

source/blender/blenloader/intern/versioning_290.c

Loading...
View Options

source/blender/blenloader/intern/versioning_defaults.c

Loading...
View Options

source/blender/editors/space_sequencer/sequencer_draw.c

Loading...
View Options

source/blender/makesdna/DNA_space_types.h

Loading...
View Options

source/blender/makesrna/intern/rna_space.c

Loading...
View Options

source/blender/sequencer/SEQ_proxy.h

Loading...
View Options

source/blender/sequencer/SEQ_render.h

Loading...
View Options

source/blender/sequencer/intern/proxy.c

Loading...
View Options

source/blender/sequencer/intern/render.c

Loading...