Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Burnins: Refactored burnins script #5094

Merged
merged 13 commits into from
Jun 22, 2023
Merged

Conversation

iLLiCiTiT
Copy link
Member

Changelog Description

Refactored list value for burnins and fixed command length limit by using temp file for filters string.

Additional info

Burnin script does not print irrelevant warning about not supported subdictionary formattings (it is supported). List values can have multiple list keys in single template and can expect other keys to fill (except timecode). FFmpeg filters are not passed to cli but through temp file and using -filter_script command to the file which should fix possible limit of command length.

Testing notes:

  1. Burnins should work with templates using dictionary subkeys (e.g. {project[name]})
  2. Burnins should work as expected with list values (currently supported only in focal length in maya)
    • make sure one of the burnin templates has {focalLength}
    • set keys for camera focal lenght in scene
    • publish
    • validate the burnin has right focal lenght values

@ynbot ynbot added size/S Denotes a PR changes 100-499 lines, ignoring general files type: bug Something isn't working labels Jun 2, 2023
Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works with dictionary subkeys {project[name]} as seen in the bottom center burnin "Libor_Tests"
but When adding those burnins for focalLength it gets too many decimals/digits. and when trying to limit those via OP Burnins settings focalLength:2f it gets N/A mm in the burnins.

Not sure if somehow connected to this PR but I guess yes when in the testing steps...

image

image

image

@iLLiCiTiT iLLiCiTiT added the sponsored Client endorsed or requested label Jun 9, 2023
@iLLiCiTiT iLLiCiTiT requested a review from LiborBatek June 9, 2023 09:34
@iLLiCiTiT
Copy link
Member Author

When adding those burnins for focalLength it gets too many decimals/digits. and when trying to limit those via OP Burnins settings focalLength:2f it gets N/A mm in the burnins.

Should be resolved

openpype/scripts/otio_burnin.py Show resolved Hide resolved
openpype/scripts/otio_burnin.py Outdated Show resolved Hide resolved
openpype/scripts/otio_burnin.py Outdated Show resolved Hide resolved
openpype/scripts/otio_burnin.py Outdated Show resolved Hide resolved
@iLLiCiTiT iLLiCiTiT requested a review from 64qam June 14, 2023 08:30
Copy link
Member

@64qam 64qam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in Nuke

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The focalLength values differ from maya scene.

For example on frame #1046 instead being 120.11mm it is 122.64mm

The same is valid for startframe and endframe. Values being off.

image

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Jun 20, 2023

The focalLength values differ from maya scene.

Not an issue of this PR. Only if it does work in develop...

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 20, 2023

That does sound like an issue of this PR since it's likely due to a time/frame mismatch of the data. It works fine in develop as far as I know.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When tested in develop focal length is correct.

So its an issue within this PR and need to be addressed.

@LiborBatek
Copy link
Member

start frame being correct

focalLength being 50

but

endframe does not match (should be 125mm)
image

and the same endframe in the review mov
image

@iLLiCiTiT
Copy link
Member Author

So it was found out that there are some differences in fps, which cause that seconds are calculated wrong (24 vs 25 fps). Tried to fix it.

Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now all works as expected and
startframe and endframe values of focalLength matches with maya

50mm and 125mm both set correctly in burnins.
image

image

@iLLiCiTiT iLLiCiTiT merged commit f76e329 into develop Jun 22, 2023
@iLLiCiTiT iLLiCiTiT deleted the bugfix/burnin_formatting_fix branch June 22, 2023 15:39
@ynbot ynbot added this to the next-patch milestone Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size/S Denotes a PR changes 100-499 lines, ignoring general files sponsored Client endorsed or requested type: bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants