rescan frames and collections after slate render

[maxpareschi]

Brief description

Representation file list and collection will be recomputed after slate rendering

Description

We found mismatches between a representation collected data and actual files on disk when rendering from existing frames on nuke. This happened only if the artist was rendering manually without including the slate (ie. from frame 1001 instead of 1000). Since data gets collected before the extraction step all frameranges after slate rendering were one frame less at the head, and the subsequent Review Mov and integration were all missing a frame. This does not happen when selecting on local, since the script will always launch rendering from first_frame-1 By adding an extra "rescan" step just after slate rendering all subsequent extractions and integrations behave as expected now.

Additional info

This pr supersedes completely my old one #2962 , which can now be removed as this pr does not affect integrate new. Should be possible to merge without waiting for #2898 as the code does not modify it anymore.

Documentation (add "type: documentation" label)

no docs

Testing notes:

To reproduce the bug with current develop:

1. set up a shot with slate workflow, framerange 1001-1050, slate setup at 1000
2. render a comp from first frame to last frame (1001-1050)
3. select "use existing frames" in the render group and be sure to also check "publish" and "review"
4. start publish
5. the integrated render and the review mov should be both one frame less at the head.

The same behaviour should yeld the correct framerange now.

[maxpareschi]

This will probably need to be reworked again for develop, it's still based on 3.11.1, on which it resolves the bug completely. I've seen you switched place between extract_review_data_mov and extract_slate_frame. The new fixes you did are still bugged under the "use Existing Frames" option, but now they behave more constantly and render one frame less at the head even if the slate is already rendered. Unfortunately the new logic is not really compatible with this pr, because it relied on the slate before mov logic from before. I'm unsure how o continue? Are you going to change logic in the nuke plugin again in the future or is this kinda how it will be?

[jrsndl]

The error you describe "eats" first frame from review files. It took me some time to realize, that in your example 1001-1050 the review QuickTime produces slate, 1002, 1003-1050, skipping 1001. Good catch!

The ExporterReview.get_file_info adds slate to first_frame (L185) in nuke/api/plugin.py. I believe that compensating for ExporterReviewMov after L383 by substracting slate frame might fix the issue with skipped frame in review.

The trouble is that the main render (Exr) gets renumbered in Integrate Asset New, frame 1001 becomes 1000, (997 ->996) with no slate.

@jakubjezek001 What would be the cleanest way to fix this?

[maxpareschi]

The way we were thinking was to export the slate first, then rescan the collection on disk to see if the extra slate frame was rendered and was not in the collection, and if there is a mismatch fix the collection and the representation file list. adding an extra step afterwards and tagging it to the slate family makes the check only available if slate node is present. Afterwards integrate and exportReviewMov behave as expected. The only problem is that you have switched the extract review mov and the extract slate, so the extra step after the slate does not work. Logically it would be render the shot > render the slate > recheck the collection and files > render the mov > integrate ?

[jrsndl]

Logically it would be render the shot > render the slate > recheck the collection and files > render the mov > integrate ?

Sounds good to me. Care must be taken to read the shot and render the mov without the slate, maybe forcing Nuke read node frame mode to expression + frame to "" would do?

[iLLiCiTiT]

Could that be caused by this change? https://github.com/pypeclub/OpenPype/pull/3407

[jrsndl]

Could that be caused by this change? #3407 Quite possible, yes. That would explain why it doesn't count with slate now.

[maxpareschi]

The bug is there from 3.9.2, possibly before it as well. #3407 made it consistent, so it’s easier to get it. We’ve been running a modified 3.9.2 ever since just because of this.

[iLLiCiTiT]

https://github.com/pypeclub/OpenPype/pull/3407 made it consistent, so it’s easier to get it.

So it could be (in theory) fixed in more "consistent" way instead of reading all collections from directory (which is against what https://github.com/pypeclub/OpenPype/pull/3407 changed).

EDITED: And maybe do it in the extractors where the slate is actually created? But this is just guessing don't know the workflow.

[maxpareschi]

Well the most consistent way would be to actually do away with all the checks about slate in every plugin apart from extract slate frame. I've noticed there's quite a lot of crosschecking in each plugin about that, and it tends to not be consistent because you know, subtract here, add there and then you lose track fo frames easily.

#3407 is a nice way to look at it and it's practical enough, but it still expects the user to actually render the slate in the range to make it available for integration (if integrate still relies on collection and the frame list). Artists tend to just push the button and be done with it unfortunately. There's also no way to "remember" each framerange if the user changes it often, aside from relying to the integrated framerange seen in the loader.

If there is the need to actually render whatever framerange instead of the actual instance framerange #3407 is probably the best way to go. But I don't really like the fact that frameranges are not validated against instance data, most of the times this would create problems downstream (let's say, an imageplane with alpha delivered with 10 more frames and suddenly the cg guys complain that the tracking is not working anymore).

If there was an enforced (but optional) validation the user can be notified and he can clear the stagingDir beforehand, ensuring that the files are only what are needed, but if you don't refer to the instance framerange how do you know if the user did render the slate already? integrate would probably work but the review mov will most probably have an extra frame if you don't add a first_frame += 1, but then we're back at square one.

Now it's kinda like:

1. render (whatever range)
2. extract mov (write start to write end)
3. 2. extract slate (write start -1 if slate)
4. extract burnins (same as mov)
5. extract review plus slate (slate + mov if slate)
6. integrate (write start -1 if slate)

There are 3 checks about slate, two of which in plugins that would be best to work without thinking about the slate, and it will still fail if the user rendered the slate at the beginning. Not to mention that it will not be valid by checking against instance data if the user rendered a different framerange. Ah, also timecode would be an issue.

My opinion is that the correct logic would be:

1. render (instance framestart to frameend)
2. extract mov (instance framestart to frameend)
3. extract slate (instance framestart -1, with timecode -1) 3.5 extract new framerange (update repre file list and collection to have the slate only if there is a slate - can be merged in extract slate if needed)
4. extract review plus slate (slate + mov, start timecode is mov -1)
5. integrate (new extracted range)

No checks, extra plugins happen only if there's the slate family (extract new framerange and extract review plus slate) if there's no slate there's no change to the collection, so everything works. if there's the slate then we compare the rendered files list length against the instance data length, if it's + 1 it means there's a new slate and it should be added to the collection and repre list so integrate works.

if you need esoteric frameranges just do a prerender with review and be sure to render it with the slate beforehand.

What do you think?

[iLLiCiTiT]

What do you think?

From my point of view it seems the most reasonable approach. But I think it would hit few issues related to current limited data about representation (wrong filled data). Each representation on instance can have different frame ranges then others but we don't know their frames.

https://github.com/pypeclub/OpenPype/pull/3407 is a nice way to look at it and it's practical enough, but it still expects the user to actually render the slate in the range to make it available for integration (if integrate still relies on collection and the frame list). Artists tend to just push the button and be done with it unfortunately.

That was my quickfix for specific bug that happened and I didn't know that it will actually affect slates so much.

Developer who is responsible for Nuke is on holiday, so I guess he won't be happy that I broke it.

[iLLiCiTiT]

It looks like that at this moment only required change to fix the issue (most of issues connected to that change) is to add the slate frame to collection on instance data in extract review slate and all should work, or?

[maxpareschi]

That's what my pr was about, so yes if it's 3.11.1, but i need to check it again on latest develop. Something will surely break ;)

[iLLiCiTiT]

That's what my pr was about, so yes if it's 3.11.1, but i need to check it again on latest develop. Something will surely break ;)

Well, yes but it again collect all frames from output dir and take all frames that are located there without checking which frames are related to current publish.

Wouldn't be this easier?

collection = instance.data.get("collection")
if collection:
    # Add one frame to indexes
    collection.indexes.add(list(collection.indexes)[0] - 1)

Wouldn't be possible to do it in ExtractSlateFrame?

[maxpareschi]

Wouldn't be this easier?

yes that can work, but do the representation files get updated autmomatically after updating the collection? if not we would also need to prepend that frame in there. And, we do need to check against framerange in instance data, since extract_render_local renders always with slate if it's present. That's another check i would do without btw.

Wouldn't be possible to do it in ExtractSlateFrame?

yes, a new plugin was just my preference since extract_slate is becoming a bit bloated, but a def in there called after everything should do the trick.

[iLLiCiTiT]

Created PR that solve it for local rendering, not sure if it's also issue for farm rendering?

EDITED: I just found out that I don't totaly misunderstood the workflow so all my changes are probably not relevant. I'll discuss tomorrow with the Nuke developer how to fix these issues.

[jakubjezek001]

Hey @maxpareschi , thank you very much for your input. Aslo accept my appology for the late response - the after-holiday crunch time was hitting me heavily ;)

The issue of missing frame was bugging us for a long time and every so often was reported from several clients.

I was doing the test from this PR on latest develop (3.12.1-nightly.3) and this is the outcome. It seems to be ok now. But I encuridge you to test it yourself and see if you find it solid now.

1. Loaded exr sequence from previouse nuke publishing
2. propertie bin of the read node is having frame range set to 1001-1050 and it is skipping the slate frame (that is by design. Just to avoid potential pixelation into project in case user would do some reasonable time offsets)
3. Once you look to the exr sequence with nuke file browser you can see the original range is 1000-1050
4. Loader is also showing only 1001-1050 as it is the original framerange without a slate frame. Slate frame is not project source material but more production data and again by design we are not poluting the prodution data with the slate frame.
5. actual workfile frame range 1001-1050

Let me know if this makes sense somehow please. Also In case it is working for you now in latest develop (and no objection regarding the workflow) you can close this PR yourself please. Thank you.

[jrsndl]

Looks like there is still issue with "double slate frame" in some apps. For example Resolve sometimes reads concatenated slate as two frames, but mrViewer always reads it correctly as one frame slate for mxf and H264 in mp4 container.

Maybe the way out is to actually reencode the slated output instead of relying on ffmpeg concat. Using concat in filter_complex produces consistently correct results for me (from command line). https://ffmpeg.org/ffmpeg-filters.html#concat

Assuming slated container format will likely not be used as final.

ffmpeg -i slate.mxf -i in.mxf -filter_complex "[0:v] [1:v] concat=n=2:v=1:a=0 [v]" -map "[v]" -f mxf_opatom -codec:v dnxhd -b:v 115M -metadata reel_name=sh010_v099 -y out.mxf

ffmpeg -i slate.mxf -i in.mxf -filter_complex "[0:v] [0:a] [1:v] [1:a] concat=n=2:v=1:a=1 [v] [a]" -map "[v]" -map "[a]" -f mxf_opatom -codec:v dnxhd -b:v 115M -metadata reel_name=sh010_v099 -y out.mxf

[jrsndl]

Here is a PR with reencoding slate: bugfix/OP-3520_Nuke-double-slate