search

Psyop / Cryptomatte

Cryptomatte Nuke plugin, Fusion plugin, sample images, and specification
BSD 3-Clause "New" or "Revised" License
632 stars 151 forks source link

manif_file relative path vs absolute - update to spec to support current gizmo behaviour #144

Open tanant opened 3 years ago

tanant commented 3 years ago

I'm doing some testing with regards to Nuke 13/crypto and came across what I thought was an issue with the implementation, and while checking, it's actually it's spec-compliant but I feel the spec might need updating?

In 1.2.0 of the spec, it was changed so that that the manif_file is always relative to the EXR directory and should not start with "../"or "./", but provides the following python as sample processing which I believe is the same code in the reference gizmo 

def resolve_manifest_paths(exr_path, sidecar_path):
    import os
    return os.path.join(os.path.dirname(exr_path), sidecar_path)

and 

resolve_manifest_paths('/exr/abs/path', 'relative/path')  --> /exr/abs/path'/relative/path
resolve_manifest_paths('/exr/abs/path', '/relative/path') --> /relative/path

Obviously, this is a python implementation detail and it's not ported to the C++ binary, but the issue here is that we have files that don't always have the JSON sidecar deeper than the exr in the path hierarchy (so not 1.2.0 compliant), but the gizmo implementation never raised a concern so this has become part of the implementation detail that we're using (the sidecars are in their own separate directory at the same level, so you'd need to go ../sidecar or in our case, we write our files with absolute pathing)

Normally I'd update the source files rather than seek a spec update but this one feels like it might be the other way around.

tanant commented 2 years ago

For anyone reading this, to be clear (cause the issue isn't clear now rereading) - absolute pathing is how things were done as an implementation detail that would just happen to work with the reference python gizmo.

The core issue is that the JSON sidecar was not stored alongside the EXR, it was stored one level up in the hierarchy. To deal with this there are two possibilities (both not in spec currently)

of the two, either would work but it's likely that absolute pathing is a Very Bad Idea for file portability, so I'd suggest the .. traversal if you asked me.