Page MenuHome

Fix T51828 VSE Proxy smaller than 100% produces wrong results when using Crop or Offset for movie strip
Needs ReviewPublic

Authored by Eitan (EitanSomething) on Nov 27 2019, 7:37 AM.

Details

Summary

This patch accounts for the different size when proxy size of less that 100% is used

Diff Detail

Repository
rB Blender
Branch
Fix-T51828 (branched from master)
Build Status
Buildable 6203
Build 6203: arc lint + arc unit

Event Timeline

  • Update sequencer.c
  • Update sequencer.c
  • Update sequencer.c
  • Update sequencer.c
  • Update sequencer.c

I wonder if this one is related? https://developer.blender.org/T70415

I will look into it

Eitan (EitanSomething) planned changes to this revision.Nov 27 2019, 5:53 PM

Doesn't work

  • Working Solution

I guess something like this could work, will have look into this in debug.

This diff doesn't revert rB450685b15664 actually
And there may be issue, that 50% proxy of clip that is 1000 x 1000 will be 500 x 500 regardless of what your project resolution is set to.

Eitan (EitanSomething) edited the summary of this revision. (Show Details)Nov 28 2019, 2:46 AM

I guess something like this could work, will have look into this in debug.

This diff doesn't revert rB450685b15664 actually
And there may be issue, that 50% proxy of clip that is 1000 x 1000 will be 500 x 500 regardless of what your project resolution is set to.

I haven’t tested it but
The clip should scale to fit when the offset button is turned off,but when it I selected it will stay at 500 x 500

Eitan (EitanSomething) edited the summary of this revision. (Show Details)Nov 28 2019, 3:33 AM
Eitan (EitanSomething) updated this revision to Diff 19919.EditedNov 28 2019, 4:51 AM
  • Fix Cropping
Eitan (EitanSomething) edited the summary of this revision. (Show Details)Nov 28 2019, 4:54 AM
Eitan (EitanSomething) retitled this revision from Fix T51828 to Fix T51828 VSE Proxy smaller than 100% produces wrong results when using Crop or Offset for movie strip.Dec 13 2019, 3:06 AM
Eitan (EitanSomething) edited the summary of this revision. (Show Details)Dec 31 2019, 6:29 AM
Eitan (EitanSomething) edited the summary of this revision. (Show Details)Jan 1 2020, 5:14 AM
Richard Antalik (ISS) requested changes to this revision.Jan 5 2020, 10:34 PM
Richard Antalik (ISS) added inline comments.
source/blender/blenkernel/intern/sequencer.c
1845–1852

I would be much happier, if we could use get_proxy_filename() from indexer.c in this case we are creating new code to "emulate" it's
function. Not good practice.

Also there was this switch when you use proxy per project or per strip
What I want to say in general is that we could refactor this code to make it a bit more human readable. I haven't seen this code for a while, and I barely know what I am looking at.

1959

This line will have impact on performance - it will have to decode (most likely) several frames.

I guess this function could be quite streamlined.

3872

I guess, that you have re-used this function to determine, if the proxy file exists.

In that case, you should rename it.

This revision now requires changes to proceed.Jan 5 2020, 10:34 PM
source/blender/blenkernel/intern/sequencer.c
1845–1852

we should use this code and then refactor the get proxy system in another patch.

3872

I think we can replace seq_proxy_fetch in this case with seq_proxy_get_fname and BLI_exist

Eitan (EitanSomething) edited the summary of this revision. (Show Details)
  • Rename variables