Parsing a .CDL with a ColorCorrection node without an id tag fails
Closed this issue · 6 comments
Is this how it should work or can we gracefully generate a unique ID for it instead? Or no ID at all even?
Below is an example export out of Arri's Amira Color Tool. Don't know if they are just exporting it incorrectly or what.
<?xml version="1.0" encoding="UTF-8"?>
<!-- ASC CDL Parameter written by ARRI Color Tool -->
<ColorDecisionList xmlns="urn:ASC:CDL:v1.01">
<InputDescription>Corresponding to Look File</InputDescription>
<ColorDecision>
<ColorCorrection>
<SOPNode>
<Description>ASC CDL Parameters to be applied to LogC data</Description>
<Slope> 1 1 1 </Slope>
<Offset> 0 0 0 </Offset>
<Power> 1 1 1 </Power>
</SOPNode>
<SatNode>
<Saturation> 1 </Saturation>
</SatNode>
</ColorCorrection>
</ColorDecision>
</ColorDecisionList>
It would need to change here to:
try:
cc_id = root.attrib['id']
except KeyError:
import uuid
cc_id = str(uuid.uuid4())
#raise ValueError('No id found on ColorCorrection')
It is actually off of the ASC CDL standard to have a CC without an ID, according to the standard all ASC CDL CCs must have an ID field.
We'll continue to raise an exception if the config.HALT_ON_ERROR value is set to True (False by default, True when using the --check
flag), otherwise we'll generate one with uuid as suggested. This generation will actually happen in the ColorCorrection class itself, allowing it to be generated with id=None, and the generation logic living on the class itself.
If only I could find any documents for the said CDL standard!
I figured it was off-spec but if Arri isn't getting it right you know the ASC didn't do a good job making the spec accessible!
Actually scratch the UUID stuff- we already have this in the ColorCorrect class:
elif not id:
if config.HALT_ON_ERROR:
raise ValueError('Blank id given to ColorCorrection.')
else:
id = str(len(ColorCorrection.members) + 1).rjust(3, '0')
Which does everything I was about to code in, except the UUID part, which now that I remember, I didn't use because I figured a numerically increasing number was probably easier to understand.
So I just changed the parse_cc to read:
try:
cc_id = root.attrib['id']
except KeyError:
if config.HALT_ON_ERROR:
raise ValueError('No id found on ColorCorrection')
else:
cc_id = None
The full ASC CDL spec can be obtained by emailing asc-cdl at theasc dot com
.
I'll email ARRI about their CDL generation.
Makes sense!
Actually wait. You could have a collision issue if another color correction node with a specified id is ID'd the same as your auto-generated ID (such as id="001")