Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add #OFRAME# (outframe) to DispatchSupportService to support chunks #597

Merged
merged 2 commits into from
Jan 16, 2020

Conversation

lithorus
Copy link
Contributor

@lithorus lithorus commented Jan 5, 2020

Link the Issue(s) this Pull Request is related to.
#83
#104

Summarize your change.
This is to better support frame chunks in the layers.
Frame chunks are really valuable for software packages like Nuke and Houdini

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 5, 2020

CLA Check
The committers are authorized under a signed CLA.

@@ -389,6 +389,7 @@ public RunFrame prepareRqdRunFrame(VirtualProc proc, DispatchFrame frame) {
frame.command
.replaceAll("#ZFRAME#", zFrameNumber)
.replaceAll("#IFRAME#", String.valueOf(frameNumber))
.replaceAll("#OFRAME#", String.valueOf(frameNumber+frame.chunkSize-1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks good to me. The only thing I'm unsure of is the name -- was OFRAME used elsewhere in the codebase or is this some kind of standard I'm unaware of?

I think it would be much more clear if we used START_FRAME and END_FRAME as our tokens. But of course we need to maintain IFRAME as it's what everyone is using already (and it makes sense for single-frame jobs). So I guess my suggestion is:

.replaceAll("#IFRAME#",  String.valueOf(frameNumber))
.replaceAll("#START_FRAME#",  String.valueOf(frameNumber))
.replaceAll("#END_FRAME#",  String.valueOf(frameNumber+frame.chunkSize-1))

Any thoughts from anyone else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds much better, wasn't happy about the OFRAME anyway. :)

We could also add CHUNK_FRAME and STEP_FRAME. Or perhaps reverse them :
FRAME_CHUNK, FRAME_STEP, FRAME_START, FRAME_END

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer reversing them to all be prefixed with FRAME_, and agree with adding FRAME_CHUNK and FRAME_STEP.

@lithorus
Copy link
Contributor Author

lithorus commented Jan 15, 2020

That was supposed to be "Changed to FRAME_END and added FRAME_START and FRAME_CHUNK"

Copy link
Contributor

@smith1511 smith1511 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.

Copy link
Collaborator

@bcipriano bcipriano 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, thanks!

@TanJeremy
Copy link

Hello all, sorry to reopen this subject two year later, but we found an issue related to this subject.
We have an issue with the #FRAME_END# token when the frame range is not divisible by the chunk size.

For exemple, if we have 72 frames with a chunk size of 10,
the last job will have the correct #FRAMESPEC# "71,72", but the #FRAME_END# will be 80,
rendering 8 more frame than necessary.

unfortunatly, i don't have knowledge in java to offer you a ready to use "pull request", and only suggest you some idea like
.replaceAll("#END_FRAME#", String.valueOf(Math.min(frameNumber+frame.chunkSize-1), fs.getAll()[ fs.size() - 1]))

Many thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants