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

Hiero: publish with retiming #1545

Merged
merged 34 commits into from
Jun 2, 2021
Merged

Conversation

create-issue-branch[bot]
Copy link

@create-issue-branch create-issue-branch bot commented May 20, 2021

closes #1377

  • Hiero: precolector for retiming effects (adding to versio data)
  • Hiero: fix: precolector effect should not be collecting TimeWarp
  • Global: fix: version data at global precolector instance data should be updating already created intance.data versionData key
  • Nuke: Loaders needed to be refactored for retime
  • Hiero: removing old plugins
  • moving retime op to global lib editorial so retiming is done in otio trough effects

Next steps

  • reversing speed loading to Nuke
  • timewarpe nodes with reversed speed in Nuke loading
  • timewarps can be creating offsets on start and end so loaders should count with those
  • loaders are not counting with less handles (version data) then they are comming form asset data
  • review is not baking retiming into review video

!! attantion !!

This PR is follower of #1511 and should not be merged before!

@jakubjezek001 jakubjezek001 requested a review from a team May 20, 2021 13:57
@jakubjezek001 jakubjezek001 self-assigned this May 20, 2021
@jakubjezek001 jakubjezek001 added type: enhancement Enhancements to existing functionality host: Hiero labels May 20, 2021
@jakubjezek001 jakubjezek001 force-pushed the feature/1377-hiero-publish-with-retiming branch 2 times, most recently from 45c7fe6 to fb45010 Compare May 20, 2021 14:33
@jakubjezek001 jakubjezek001 marked this pull request as ready for review May 20, 2021 22:01
Copy link
Member

@mkolar mkolar left a comment

Choose a reason for hiding this comment

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

No success for me with this one. I have source clip that is 44 frames long. I use timewarp in hiero to make it 2x speed and result in 22 frame long timeline clip.

When I publish the retime seems to be published correctly and it loads into Nuke as well, however the actual frames that get published are only 1-22, so I'm missing half the required footage in the publish.

Also a question. Shouldn't we support also a simple editorial retime in here? Just setting a clip to 200% for example without applying timewarp effect. In most simple scenarios that is more likely to happen, than a timewarp.

@jakubjezek001
Copy link
Member

Also a question. Shouldn't we support also a simple editorial retime in here? Just setting a clip to 200% for example without applying timewarp effect. In most simple scenarios that is more likely to happen, than a timewarp.

This is implemented and should be working. It is taking the editorial retime percentage and converts it into expected framerange. Then in nuke it is creating retime node under read node automatically with expected framerange. I was testing it and was working for me.

@jakubjezek001
Copy link
Member

@mkolar try it now. I found a bug where speed of track item was not projected within duration. Also different speed then 1 was not switching on retime family. So I guess it should cover both of your issues

@mkolar mkolar self-requested a review May 26, 2021 11:50
@mkolar
Copy link
Member

mkolar commented May 26, 2021

Retime is working now, but I keep getting the same result where I'm missing half of the frames that should be published. I'm trying with exr sequence and only half of the frames are copied to the publish folder.

Secondly we have some conflicts that need to be resolved.

Aand lastly, precollector alone can't work because of undefined variable, I fixed it locally for me for testing, but needs to be committed properly.
image

Copy link
Member

@mkolar mkolar left a comment

Choose a reason for hiding this comment

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

Viz comment above

@mkolar mkolar added this to the 3.0.0 milestone May 31, 2021
# calculate speed
speed_in = (node["lookup"].getValueAt(timeline_in) / (
float(timeline_in) * .01)) * .01
speed_out = (node["lookup"].getValueAt(timeline_out) / (
Copy link

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)


# calculate available material before retime
available_in = int(track_item.handleInLength() * speed)
available_out = int(track_item.handleOutLength() * speed)
Copy link

Choose a reason for hiding this comment

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

local variable 'available_out' is assigned to but never used

speed = track_item.playbackSpeed()

# calculate available material before retime
available_in = int(track_item.handleInLength() * speed)
Copy link

Choose a reason for hiding this comment

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

local variable 'available_in' is assigned to but never used

import math
from openpype.hosts.hiero.otio.hiero_export import create_otio_time_range

class PrecollectRetime(api.InstancePlugin):
Copy link

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@mkolar
Copy link
Member

mkolar commented Jun 2, 2021

Merging with the limitations mentioned in the PR text. those need to be made into separate issue. Especially the reviewable with retime applied

@mkolar mkolar self-requested a review June 2, 2021 16:36
@mkolar mkolar merged commit f0d03eb into develop Jun 2, 2021
@mkolar mkolar deleted the feature/1377-hiero-publish-with-retiming branch June 2, 2021 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: Hiero type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hiero: publish with retiming
2 participants