@nick, I believe I have addressed most (if not all) of your points in this version
removed Terms & Definitions
All the terms are defined in context later in the document. The few remaining definitions weren’t serving much of a purpose. If you’re reading this document and don’t know what a LUT is, then it’s not the document for you. Descriptions of what the nodes are are included in the specific ProcessNode-substitute sections
Log Node
changed attributes of LogParams node back to match Doug’s terms (per @zachlewis)
Implementation Notes
moved the notes on XML white space and newline characters up to the XML section
made explicit bit-depth handling sections - consolidated a lot of the existing language - hopefully this reads ok
simplified the Interpolation Types section - Do we even need this? Can this be removed entirely?
Schema
other updates
added ASC CDL node (surprisingly this was not included even in the previous version of the spec)
I still need comment on the items below.
(If I don’t hear any opinions, I’ll have to make the decision.)
Are IndexMaps are still necessary at all?
It seems to me that a Range node does the same thing that an IndexMap with dim=2 would accomplish. What am I missing about why an IndexMap would still be useful if it’s limited to dim=2?
Should InputDescriptor and OutputDescriptor be required instead of optional? (suggestion from @nick)
Should ACESTransformID and ACESuserName be sub-elements in an ACESInfo element? (suggestion from @zachlewis)
Should ACEStransformID be applicable on a ProcessNode level rather than on a ProcessList?
Should CalibrationInfo be a part of the specification or not?
It was, then it wasn’t, then it was again, and currently it isn’t (although it’s still in the schema). If it’s not hurting anything and is optional metadata, it’s probably ok to leave it. Nobody actually has to use it. However, there was concern that it was unnecessary metadata that could easily be tracked using one or more Description nodes. So… is it in or out?
LogParams - Is it wrong that I have the linearOffset as a settable attribute? (rather than having it always computed automatically?)
To me, tt seems like it might be useful if, for example, I’m copying another formula exactly and they didn’t correctly make the transition between the log/linear segments smooth according to the derivative. (like the discrepancy between the sRGB written spec vs the intended formula)
This version of the document will imminently be “locked” and submitted to the Architecture TAC as the deliverable from this VWG. I am sure there are still typos and small things that the Implementation group will find as they try to implement from the updated specification, and those will be fixed. So the document is not completely “locked”. But we are not adding new features or changing things in a major way at this point in time.
The main difference between an IndexMap and a separate Range node, is that an IndexMap is (I believe) restricted so it always maps a given range to the range of the subsequent table. This means that a 1D or 3D LUT in another format with a defined input range could be converted to a one node CLF, and the resulting CLF could easily be converted back to the original format matching the original exactly. If the input range is broken out into a separate Range node, a LUT converter converting back to the original format would have to apply some intelligent analysis to the two node CLF. And not all Range + 3D CLFs could be accurately represented by a simpler LUT format with defined input range.
I suspect that in many use cases, a CLF may be used to encapsulate a single 3D LUT from another source. And easy cross conversion may be a benefit.
Copy-pasting verbatim, and for the public record, the CLF Review notes that I sent, as a TAC member, to the ACES Leadership:
Hi,
Congratulations for the hard work! It was helpful to have followed the process, made the review easier and shorter.
Here are my notes, you will find that a lot of them are stylistic and pedantic but they are striving for improving the consistency of the document. There are a few questions that I would like to hear answers for and at least an important correction that pushed me to reject the proposal in its current state. I have assumed that the maths are correct and did not spend too much time in the interpolation appendixes.
A Common File Format for Look-Up Tables - Review - CLF_3_0-2019-11-26.pdf
Summary
[ Rejection Source ] The correction I would like to be made is, actually, after just reading Graeme comments, also about the matrix dimensions, see 27.2.
[ Rejection Source ] There are a few hard-coded ranges in the specification in non ASC_CLD nodes, I want to hear why, see 34.1.
[ Rejection Source ] How is the compatibility with a particular version of CDL defined?, see 35.2.
I’m generally annoyed by the (mentioned during calls) casing inconsistency in many places, this is not great and nobody came with a single good reason as to why it should not be changed. The only ones I heard: There are already some implementations of CLF 2 and changing casing might break them. or It is fine, people will just copy-paste the names.. I’m sorry but this is not a good reason. Any major version update is the opportunity to flatten irregularities like that and it is a shame that we don’t do that here, see 19.6, 28.2. It also goes beyond casing, for example, basicFwdMirror should really be basicMirrorFwd, see 32.1.
It would be useful (and consistent) to have generic Log and AntiLog styles that are using the baseLogParams, see 29.1.
I still think that Range node is badly designed and tries to do too much, even though I’m pleased to see that it is in a better place than a few months ago. I hope this will be addressed by deprecating it and adding specific Remap and Clampnodes in CLF 4.0.
I would be keen to discuss specifically about 8.1, 15.2, 22.1, 23.1.
As some VFX studios existence spans multi-decades, it has become a super critical information to perform archeology on old shows and chase information. One can use the Copyright element but I reckon having dedicated elements would permit to establish good practise to track that down because Copyright @ The Moving Picture Company or Copyright @ Pixar is not really helping here.
Consistency needs to be improved, it is a matter of doing a few search-and-replaces, rewording a few things. This is a technical document and must be hyper-consistent. I’m happy to help with that on Overleaf.
Notes
Any Page:3by1D or 3x1D: need to pick a variant, I would favour the latter as the former does not look elegant.
Any Page:3 by 3, 3 by 4 or 3x3, 3x4: need to pick a variant, I would favour the latter for reasons previously stated.
Any Page: 3DLUT or 3D LUT: need to pick a variant, I would favour the latter for reasons previously stated.
Any Page:XML file or CLF file: need to choose one variant or the other, I would favour the latter after introducing it, e.g. A CLF file is a XML file [...].
Any Page: There are many variants around ProcessNode: Process Node, process node, ProcessNode, the document needs to settle on one.
Any Page: A lot of the ProcessNode(s) are introduced like so: The LUT1D element, this should be consistently changed because element has a particular meaning in XML and in CLF itself. What about something along the lines of The LUT1D derived process node [...].
Any Section Title: They need to be consistently cased: Input and output to a ProcessList should be Input andOutputto a ProcessList.
3 Specification (Page 7):
the sequence of transformations is described as a node-graph: graph feels quite pompous given we are talking about a sequence (list). It may also make people think, incorrectly, that CLF might be an Undirected Graph, a Multi Graph, etc…, while it is, at best, a super simple Directed Acyclic Graph (DAG), without any explicit edges. If it was effectively a graph while not having called ProcessList, ProcessGraph? I would be keen for that to be disambiguated or if the intent is to have vertices with multiple explicit edges in the future, for any reason, having it explicitly stated.
3 Specification (Page 7):
Each XML file shall be completely self-contained, requiringno external information or metadata.: requiring is more appropriate in the context of this technical document.
3 Specification (Page 8):
Arbitrary ordering of list elements is notsupportedin the format
3 Specification (Page 8):
For simplicity’s sake,: Is not sake being too mundane here, in a technical context?
3.1.2 Declaration (Page 8):
Remove the paragraph that says The file should contain the following [...], as it is redundant with 3.1.3.
3.1.3 XML version and encoding (Page 8):
XMLVersionandEncoding
a starting line that identifies the XML version number and encodingvalues.
3.1.4 Comments (Page 9):
brackets like so:
3.1.7 Newline Control Characters (Page 9):
andWindowssystems: A Unix system can be a personal computer (PC).
I would be keen to hear practical situations where the newline character has been an issue because this is a quite hard thing to enforce and people tinkering with text editor will surely mess with that.
4.1 Array (Page 9):
TheArrayelement contains: there should be an emphasis on the Array term here and generally, when the document references a specific construct, there should be an emphasis on it.
Figure 2 (Page 10):
The diagram should be moved to another page, it cuts the description of the dim attribute, making it hard to follow.
4.1 Array (Page 11):
The section should start with Description: like for Array .
4 entriesrepresentthe dimensions: It might be my english, but have sounds weird here, likewise for the others.
of a 3D cubeandthe number of components per entry
with 3color components
[ Rejection Source ] There is an important issue with the dim attribute for the matrices that I discuss in point 27-1.
4.2 ProcessList (Page 11):
This section starts with Description: but not Array ?
A LUT designer can allow floating-point values to be interpreted by applications and thus delay control of the final encoding through user selections. I don’t understand what this means.
NOTE 2: This note should be in 4.4.5, it is currently misplaced as we are talking about the node container and not any specific node.
NOTE 3: This note should redirect toward the ACEStransformID definition down or be moved as it is confusing here. It could be reworded as follows: This attribute may also store the ACEStransformID, cf. definition below, which is [...]
This is a question for the whole document: Should the Attributes ordered by requirement and alphabetically? Right now it is neither.
I mentioned it a few times but I don’t like the fact the casing is inconsistent.
4.2 ProcessList (Page 12):
At least one ProcessNode must be in the list. : What about The ProcessList must contain at least one ProcessNode?
4.3 ProcessNode (Page 12):
No line return after Description:
A ProcessNode element represents a primary color transformation: What does primary means in that context?
4.3 ProcessNode (Page 13):
“32f”: 32-bit floating point (single precision) No support for 64f double-precision? This contrasts with 5.3 recommending that all calculations be done in at least 32-bit floating point.
The Attributes should be ordered in a meaningful way as previously stated.
4.4 Substitutes for ProcessNode (Page 13):
Substitutes for ProcessNode: The first consumers for the specification will be software engineers, I would favour using terms they are used to and make sense in software development, e.g. Subclasses, Inheritors, Derived Nodes, etc… especially when it is stated that the attributes and elements are inherited.
NOTE: Additional substitutes for ProcessNode could be introduced in future versions of this specification. Is this note required here? Seems like stating the obvious.
4.4.2 LUT1D (Page 13):
Linear interpolation shall be used for LUT1D: Might need more explanations on as to why and put an emphasis that it is a recommendation, especially for non-native english readers.
dim: I almost wish this attribute was called shape
4.4.2 LUT1D (Page 14):
AnIndexMapis used to define [...] or Anindex mapis used to define [...], consistency oblige
an integer that indicates the number of items in the list. The only valid value is 2. Any specific reason to require dim here if it is always the same value?
This attribute is optional but is fixed to "linear" as the only allowed value: With respect to my comment in 24-1, is it shall or must?
halfDomain, rawHalfs Would be great to have better description consistency: those two attributes start by giving their default value first while interpolation gives it at the end of its description. More explanation and examples would also be welcomed here.
4.4.3 LUT3D (Page 15):
with 3 colorcomponentcolor
NOTE 2: Array is formatted differently when it is contained in a LUT1D or Matrix element. Please explain how, seems like it is coming out of thin air.
The Examples sub-section looks like it is part of the Index Map element but is not actually, it should be dedented accordingly.
4.4.4 Matrix (Page 16):
An offset matrix may be defined using a 3x4 Array
[ Rejection Source ]dim does not look correct here because it is not giving the shape of the Array but is a mixed bag between the effective shape and the intended number of component the Array is meant to act on. The elements of dim multiplied together should be equal to the number of elements in Array. This is extremely problematic. With all the previous cases of Array, and while using Matlab or Numpy, one can simply tokenize the data, convert to number representation and reshape but here it would break for no obvious reasons.
4.4.5 Range (Page 17):
I still think that this node tries to do too much and is badly designed. The very long specification speaks for itself and I don’t like it. I sincerely hope this will be addressed by deprecating it and adding specific Remap and Clampnodes in CLF 4.0. I had to take my breath after reading its description…
Why are the Elements title-cased here? Consistency, consistency!
4.4.6 Log (Page 19):
It would be useful (and consistent) to have generic Log and AntiLog styles that are using the baseLogParams.
specifies the form thelogarithmicfunction to be applied: Consistency again.
4.4.6 Log (Page 20):
logarithmic function with a gain factor, anoffset and a linear segment.
assume integer dataisnormalized
The equations are starting to be hard to read and overcrowded, might need to define some indirection variables and use where "Foo" is equal to:, especially for y -logSideOffset / logSideSlope.
4.4.6 Log (Page 21):
by using the derivative of thelogarithmicportion of the curve: More consistency.
4.4.7 Exponent (Page 22):
"basicFwdMirror": Direction should be a suffix, e.g. “basicMirrorFwd”, “basicMirrorRev”, “basicPassthruFwd” and “basicPassthruRev”.
4.4.7 Exponent (Page 23):
"moncurveFwd" : “monCurveFwd” Also, should there be a note that this one is intended to mimic a monitor curve hence the name?
4.4.7 Exponent (Page 24):
[ Rejection Source ]Must be in the range 0:01 to 100:0, inclusive. Default is 1.0: The specification, abstracting ASC CDL, is generally free of any restrictions such as that one, which is great. Are there any really really good reasons for having those? They are also different from ASC CDL ones. Likewise for offset.
4.4.8 ASC_CDL (Page 25):
they are always interpreted as if the bit depths were float.: This one is oddly worded, does this mean that 8-bit is effectively 8.0-bit? What about saying something along the line of they always act on floating-point data?
[ Rejection Source ] How is the compatibility with a particular version of CDL defined? Say ASC CDL 2.0 is released, how do we make sure it does not break CLF? It seems important that we define which version of ASC CDL is supported in a dedicated attribute.
5.1.1 Processing precision (Page 26):
ProcessingPrecision
shall not affect the quantization of color values., this is oddly written, what about an explicit: do not impose that the colour values should be quantized to to the particular bit-dpeth and converted to integer, only that they should be scaled accordingly. Upon further reading, it is actually repeated in 5.1.2 but this is super important, so might as well double-down on this one.
5.1.2 Input and output to a ProcessList (Page 27):
Input andOutputto a ProcessList
5.1.3 Input and output to a ProcessNode (Page 27):
Input andOutputto a ProcessNode
the formulas in the Log, Exponent, and ASC CDL elements assume that data is normalized 0-1., because there are so many ways to normalise data between 0-1, it seems like it would be important to expand 5.1.4, making it a dedicated section about everything normalisation and reference it across the document, for example in 4.4.6: the Log node assume integer data isnormalized to floating-point scaling,see Section 5.1.4.
5.3 Efficient Processing (Page 28):
all calculations be done in at least 32-bit floatprecision.`
5.5 Floating-point Output of the ProcessList (Page 28):
Floating-pointOutputof the ProcessList
do not contain infinities and NaNrepresentations.: code is not really appropriate here.
Appendix A (Page 30): I suppose that the example LUT (abstracting the abridged values) is valid against the Schema?
Sorry that it has taken me awhile to compile my feedback, but it is finally enclosed below.
First, I want to say thank you for all the work pulling together the unwieldy raw material from many hours of conversation and various forum posts into a coherent document – it was no small task. There is some work remaining, but it’s awesome you and JD have gotten the bulk of the writing accomplished for us.
Before getting into specific points, here is my take on a few of the issues discussed on the forum in recent posts:
ACEStransformId / ACESuserInfo – Yes, I agree with Zach that these should go under the Info element (in fact the v2 spec was contradictory on this subject but did indicate they should go there in one part). We also may want to mention and/or illustrate the tie-in to AMF since the IDs seem to play an extremely important role there.
LinearOffset should definitely be removed from Log. We don’t want to allow people to create discontinuous Log transforms.
I would support your suggestion to remove the IndexMap entirely since one of our goals was to try and simplify the job for implementors. Nick raises a potential use-case, but the utility IndexMap brings is not worth the significant complexity (IMHO).
Regarding the Array dim attribute for Matrix, this has been in the spec since v1. The Lut1D has [length, chanels], the Lut3D is [dim1,dim2,dim3,chanels], and Matrix is [rows,cols,channels]. The v1 and v2 specs (e.g. see pg 12) contemplated allowing 4 channels (RGBA) and this was discussed during some of the meetings. But we decided the channels element must always be 3 in the current version of CLF. Perhaps the syntax could have been done in a clearer way by the original ASC committee for v1 but it is far from unworkable. In fact from a parsing standpoint it is extremely simple, whereas other approaches (e.g. breaking it out into separate attributes) would be more complicated to implement.
And now here are some more detailed comments on the individual sections:
3
At the end of the fourth paragraph please add: “Implementations may process 1-component transforms by applying the same processing to R, G, and B.”
4.1
At first glance, I did agree with the earlier comment questioning whether the separate Array section is really needed, as the content is repeated elsewhere already. But perhaps in light of recent feedback it would be helpful to keep it since it illustrates the structure of the dim attribute for Lut1D, Lut3D, and Matrix all in one place.
4.2
Please add: “The compCLFversion corresponding to this version of the specification is “3.0”.”
4.3
I think it would be clearer to describe the bit-depths as “a string that may be used by a ProcessNode to help describe the scaling of parameter values. Note that this string is for formating purposes and does not control processing precision.” If you talk about it in terms of “expected input/output values” it leads people to go down the wrong path about how to think of these attributes.
4.4.1
Please add: “The inBitDepth of a ProcessNode must match the outBitDepth of the preceding ProcessNode (if any).”
4.4.2
Please add: “The scaling of the array values is based on the outBitDepth (the inBitDepth is not considered).”
As per the v2 spec, please add: “Implementations are not required to support LUT1Ds longer than 65536 entries.”
As stated above, I’m not opposed to keeping it if people really want it, but I still don’t feel IndexMap is worth the effort.
4.4.3
Please add: “The scaling of the array values is based on the outBitDepth (the inBitDepth is not considered).”
I think the last sentence of the first paragraph adds more confusion than help. It doesn’t really change the lookup, per se.
The high end of the index map is not 1, it is the grid dimension minus 1.
Please restore the following sentence from the v2 spec “LUT3Ds have the same dimension on all axes (i.e. Array dimensions are of the form ”n n n 3”). A LUT3D with axial dimensions greater than 128x128x128 should be avoided.”
4.4.4
Change LUT to Matrix (2nd paragraph).
4.4.5
The default cannot change from the v2 spec, it must remain a clamp.
It might be clearer to forbid the option to omit all min/max values. No one would ever need this – I think it was intended as a convenience to keep bit-depths matching, but it may cause more confusion than help.
4.4.6
LinearOffset should not be a free parameter, it should always be calculated.
Eq. 4.19 is not really the derivative, it’s that with linSideBreak substituted in.
4.4.7
I’m thinking we should add the mirror option for moncurve too.
The exponent attribute is always required.
The valid exponent range you stated is only for the basic styles, the valid moncurve range is [1,10] (allowing it to go below 1 breaks the formulas).
The Description element of the first example doesn’t match the params.
Please remove the statement about “similar but different to 61966-2” (They are the same to within rounding error. People will think you are referring to the gamma 2.2 debacle, which is not what we want.).
If you want to include values for all three common curves, the constants are – sRGB: (2.4, 0.055), L*: (3, 0.16), Rec.709: (1/0.45, 0.099).
4.4.8
The CDL spec only places non-negativity bounds. Do we really want to place upper bounds?
5.1.1
Remove paren at end.
5.1.2 and 5.1.3
During one of the calls I thought we had agreed to the bit-depth description I posted on May 1. The spec now says something quite different. The description as it stands now is not workable for implementors. Would you like me to propose new wording, or do you want to have a call to discuss, what is your preference on how to rework this?
5.3
In general, combining operators will result in substantially different results and should only be done as a last resort. I think the wording needs to discourage this more strongly. Also, it should clearly state that any such approximation should be compared against the original transform, which is the reference.
5.4, 5.5, and 5.6
I find these confusing and am not clear what value they add above the actual interpolation directions in the appendix and other material already provided elsewhere. I would vote to remove them.
5.8
This should only apply to elements outside the Info block. Unrecognized elements in the Info block are fine and may simply be ignored.
Also, I don’t think Description should necessarily be used for arbitrary metadata if it is not basically a description. Other types of stuff could be put in new elements inside the Info block.
6
We need better examples. If you want to include a complex example, you may want to consider an ACES2065-1 to ACEScct transform. Let me know if you’d like me to provide this or others.
App. A
I don’t think this should be normative. In fact, I would vote to move it to separate file.
I haven’t reviewed v1 or v2 nor read them in details but I would have flagged it likewise.
Let’s say I’m looking at that from a data scientist standpoint that uses Multi-Dimensional Arrays in for example Matlab, Fortran, Numpy, Tensorflow, Julia, name-your-MDA implementation etc…
The dim attribute provides the dimensionality of the indexes: No, it does not! The case for the Matrix does not comply with that definition. Either the attribute must be renamed to something else, e.g. almostDimButNotQuite or the definition changed and the way the Matrix case is specified updated accordingly.
The LUTxD are effectively describing the axes/dimensions of a Multi-Dimensional Array:
17 17 17 3 indicates a 17-cubed 3D lookup table with 3 component color
256 3 indicates a 256 element 1D LUT with 3 components (a 3by1DLUT)
256 1 indicates a 256 element 1D LUT with 1 component (1DLUT)
Multiplying the indexes together indeed gives the size of the array, which is exactly what one would expect. The channels are implicitly accounted in the tail of the array, i.e. the last axis of the array.
For the Matrix case:
3 3 3 is a 3 by 3 matrix acting on 3-component values
3 4 3 is a 3 by 4 matrix acting on 3-component values
dim gives BOTH the axes AND something else, it is not surprising that the text had to be formulated differently to accommodate for that.
Something I forgot to add in my notes but came apparent as I started to review AMF and will also post in my AMF notes: I would like to see native support for indicating the author(s) and contact(s) information.
As some VFX studios existence spans multi-decades, it has become a super critical information to perform archeology on old shows and chase information. In CLF, one can use the Copyright element but I reckon having dedicated elements would permit to establish good practise to track that down because Copyright @ The Moving Picture Company or Copyright @ Pixar is not really helping here.
@thomas@doug_walker
Thanks both for your detailed feedback. It’s nice to have people checking my work as its easy to mess things up with the reorganization of the document and working from the previous words. I’ll work today to incorporate your comments - they will certainly make the text stronger.
As I go, I will compile a list of remaining questions and/or contradictory feedback that the group will need to settle. (e.g. matrix dimensioning scheme)
I will also record any major changes to the substance of the specification - i.e. moving ACEStransformIDs into an Info element - so that everyone is clear on which refinements were accepted - and to give anyone vehemently opposed a chance to respond.
@zachlewis, removing the LinearOffset does not have a meaningful effect on what curves may be represented. This is simply an internal variable in the equations that should not be exposed to the end user.
All the ARRI LogC curves up to about an ISO of 1400 may be represented using the proposed interface. Above about 1400 they use a Hermite spline to roll off the highlights and those curves would still need to be baked into LUTs. Exposing LinearOffset would not have any effect on representing ARRI curves. The only thing it would allow you to do is make discontinuous curves where the pieces do not meet at a common point. We don’t want to enable people to make broken curves.
@Thomas_Mansencal@doug_walker@zachlewis@Graeme_Nattress I believe I have addressed most of your points. Thanks again for your thorough reviews. I have attached a new copy but I’m not sure how helpful that is since it doesn’t redline the changes. There may be a few lingering items that I plan to catch as I read through it again (trying to address all your points simultaneously was very tedious). This is a very sizeable response and I’m positive that I will miss some things - but I want to get keep the conversation going…
Matrix dimensioning
A few back-and-forths on this above. @Graeme_Nattress, @Thomas_Mansencal and I believe at least one other has expressed confusion over why the matrix dimensions have a 3 at the end. I can see the argument on both sides but the more I look at it, the more I don’t really see it as terribly inconsistent. I could go either way on this but I don’t see it as a deal-breaker if left as-is.
Bit-depth @nick and @doug_walker have both voiced concerns that these sections are still not clear enough. There were many places in the old spec where things were casually mentioned (and usually not that clear). I think this version is better for trying to say it all in one place but in the editing process I may have lost the key points that others think are most important. (Inline notes to @doug_walker below)
Hard coded ranges
There seems to be some debate over whether certain parameters should have reasonable limits imposed on them. Specifically the LogParams, ExponentParams, and ASC_CDL parameters. In some cases, limits are set so as to prevent errors (divide-by-zero errors, log of a negative number, etc.)
Should values be left unrestricted whenever possible? Or is declaring practical range limits preferred?
File extension
The AMF spec sets .amf as the file extension for those files. We do not specify a file extension for CLF files anywhere in the specification. Do we want to specify one? If so, is it .clf? Or should we leave the extension as .xml?
Log/Antilog @thomas has requested a generic Log/Antilog style that uses the basic LogParams structure. You can already use the LogToLin/LinToLog styles with all LogParams left at their nominal values (0 for offsets, 1 for slopes) and effectively get basic Log/Antilog behavior, so if I am understanding his request correctly, then this style would simply an alias to the LogToLin/LinToLog styles. We provide a base 10 and base 2 basic logarithm/antilogarithm. @thomas is this what you meant? Thoughts from others?
ASC_CDL versioning
Per @Thomas_Mansencal, how do we maintain compatibility with a particular version of CDL? Should there not be a “version” attribute or element to protect us in case a new version of CDL is ever released?
Casing consistency
The sub elements in the Range node are title cased, even though they are not anywhere else.
Examples
If you’ve got practical examples using CLF, please send them my way and I’ll add them as supplements to the document. @doug_walker proposed an ACES-to-ACEScct conversion and I’m sure theres a few other simple yet practical examples we could use to augment the document.
I’m sure there are more that I forgot to summarize, but this message needs to get posted, so I will follow up if I realize there are items I missed…
(Some) major changes made in this version:
IndexMap removed entirely from spec
Created ACESInfo block with ACEStransformID and ACESusername as child elements (per @zachlewis) (@doug_walker, can you reflect this change on the diagram?)
Restored CalibrationInfo block with its numerous, very specific child elements. I figured that since we’re going to allow ACES metadata, then there’s no harm in other optional metadata fields, as long as they’re contained in the Info element. I don’t think anyone will ever use these Calibration Info elements but they were already in v2, so it’s not like we’re adding anything…we’re just not taking it away either.
Exponent
Renamed “style” params such that Fwd/Rev was always at the end of the name. (Per suggestions from @Zach & @Thomas_Mansencal)
Re-added the “monCurveMirror” functions (per @doug_walker) . @nick had been the only one to reply to my query about them, so I removed them since no one objected. @doug_walker has now asked for them to be included, so I re-added them.
Re-cased “moncurve” as “monCurve” and “Passthru” as “PassThru” (casing consistency, per @thomas)
Set separate valid ranges for “exponent” depending on “style” (per @doug_walker). I wonder if my revised note about exponent=1 is sufficient protection (similar issue to allowing 0 for offset). Can I just set range to (min,max] to imply exclusive on bottom end and inclusive on top end? I do a similar thing for “Power” in ASC_CDL node… (related to decision on #2 above)
Implementation Notes
Removed sections formerly numbered 5.4, 5.5, 5.6 - Indexing Calculations, Floating-point Output of the ProcessList, and Half-float 1DLUTs (per @doug_walker - I agreed they weren’t adding anything)
Added to state that up to 3 LogParams elements can be used (in the case of defining separate operations for the three channels)
I re-added the “mirror” for moncurve style functions. @nick was the only one to have commented in favor or against their removal, so I removed them. New <Exponent> Process Node - #12 by sdyer
Changed exponent attribute to required
I have set separate valid ranges for “exponent” depending on the “style”. I added a note to warn about exponent=1 and wonder if this is sufficient protection against potential divide-by-zero issues (a similar issue to allowing 0 for offset). Could I just set the range to (min,max] to imply exclusive on bottom end and inclusive on top end? I already do a similar thing for “Power” in ASC_CDL node to avoid allowing that value to equal 0…
Added more examples and renamed sRGB (that was changed in response to @nick who had pointing out that the example does not exactly match the spec - but it is what was intended)
On the new examples, did I get the EOTF and OETF directions correct?
Yes, technically you’re right, but I picked what I thought would still be substantially large end points. If it’s not a problem to specify it as “infinity” then I will, but I also didn’t want people trying to use Inf or -Inf values from 16-bit half.
The CDL spec says (for slope) “in practice, systems should probably limit at a substantially lower value”, and (for offset) “offset, can in theory range from -∞ to +∞ although the range -1.0 to 1.0 will fit most traditional use”.
I thought (based on values used for Exponent and such) that [0, 100] for Slope, [-100, 100] for Offset, and (0, 10] for Power was more than sufficient. Should I instead express these limits as in the CDL spec at their theoretical limits? (Related to #2 above)
There were a LOT of different snippets about this, although I did originally start from copy/pasting your post into the document. I guess when trying to remove redundancies and conflicting statements, I perhaps lost the original meaning of your words.
Yes, It would be of great help if you could suggest new wording that adequately addresses your main concerns regarding bit-depth handling. In the meantime, I will also attempt a revision.
I am completely ok with removing these. I was trying not to cut too much out of the original spec. There were a lot of “floating statements” like these just thrown into the implementation notes. If you don’t think they add anything - then I’m all for cutting them. As of now, they’ve been removed.
Agree. I will welcome any examples - and some would be helpful as added to the spec. However, I also foresee many more helpful examples being generated to accompany the spec as a part of the implementation group. These are easy to drop in. If you have an ACES to ACEScct example ready to go, I’ll take it. Otherwise I’ll add one myself after the rest of the changes are decided.
You and others brought this up and I have fixed it. This is a new node so we can call it whatever we want. It does make more sense to have the direction consistently at the end of the style.
I’ve modified this.
64f has never been supported as input/output bit-depths. I’m not sure why (considering we seem to support everything else). Anybody?
I usually think in terms of classes, subclasses, inheritance, etc. but I thought that XML was the reason we used the terminology “substitution groups”. This wording is from the previous draft and I thought was the preferred terminology when speaking in the context of XML. I could be wrong. Though I’m not opposed to changing the terminology, I don’t think the meaning is currently lost by not using different words. But I’m open to suggestion if you think something could be said more clearly.
Thanks for carefully reviewing all the feedback, this draft is looking much better.
Here is my feedback on your points 1-8:
Matrix: Please leave it as is. It would cause huge interop problems with v2 files to make that change.
Bit-depth: I will propose some wording in a separate post.
Hard-coded ranges: The only thing I would change from your latest draft is to make ASC CDL ranges consistent with the actual ASC CDL spec. I would not want to deviate from that unless there is an extremely compelling reason. You also don’t need to specify all of these in the form [min,max], you could simply provide only the lower bound if that’s all we want to restrict (rather than using Inf for max).
File extension: Yes, excellent point! I agree is should be .clf.
Log base: As you say, the spec already allows an arbitrary base and I feel the Log2 and Log10 are a quick and easy short-hand for 99% of the pure-log use-cases. But I’m not opposed to adding it if Thomas really wants it.
ASC versioning: I had made a similar point to Thomas back during the v2 discussions and had proposed a style of “v1.2_Fwd” so that the style had the version. However, the group wanted to just call it “Fwd”. I don’t think any action is needed now but if a new version is introduced then a new style name will need to be added.
Casing: I don’t feel the need for it, but am not opposed if others want to change it.
Examples: Sure thing, I will provide some. But in the Exponent examples, swap the EOTF/EOTF usage you have currently (i.e., moncurveFwd is an EOTF).
Regarding the ACES elements, I think I interpreted what Zach was asking for differently than you. I thought he was asking that they be moved from the ProcessList level down into the Info block whereas you seem to think he was asking to create an ACESInfo sub-element under Info for them. I don’t see the purpose for the ACESInfo element but am not opposed to doing that if that’s what you and Zach prefer. Please confirm you want to keep it this way and I’ll update the diagram. However, I do think some more attention should be paid to them now given how important they will become to the success of AMF. Ideally, there would be an example with an AMF and a CLF linked in that way.
Regarding adding ‘64f’, this is not necessary. The key point is that the bit-depths only define the scaling of parameters in certain ProcessNodes, they do not affect the processing precision used by applications. In fact, ‘16f’ and ‘32f’ are already redundant since they imply the same scaling. We do not need to add ‘64f’ to allow applications to process at double precision. These are unrelated concepts and that is what the rewording of the bit-depth discussion should make clear.
Again, thanks for all the thorough work on this Scott.
I may start a separate post for for the bit-depth wording since this one is getting lengthy!
Your inner Matlab-fu knows it is not True This is my number one issue with the specification so far. As you know, I have sometimes (often?) a strong opinion on things and this is one of those cases
@doug_walker: Something quite bothers me in that statement and it is not the first time during the various conversations I took part in with respect to CLF 3 (or others with you, e.g. when we discussed about implementing the current ODTs with the new SSTS). I have felt a lot of resistance to change stuff from your end and I’m curious as to why. We are talking about CLF 3 here, not CLF 2.1 or are we? It is a major version of the specification thus backwards-incompatible changes should be perfectly valid and it is a unique opportunity, the only one, in fact if rigorous about development, to implement them. I don’t see where the interop problems would occur, do you have practical examples? Having the version of the specification in the file is to enable such changes and so that parsers can cope with them. My feeling, and correct me if I’m misinterpreting anything, is that you have been opposed to basically any change that breaks compatibility. To me, it defeats the entire purpose of a major version and ultimately, it prevents innovation. I surely would not expect Maya 7 to open a Maya 2019 file at all or at least without having a ton of stuff breaking.
Hard coded ranges
Which makes sense, but then why that particular number over another, and, why having them inconsistently specified between the various Process Nodes? Such limits should be delayed as much as possible in the software stack. Generally, I impose them at the UI level and even there, it is a soft limit. I would not mind having recommended ranges indicated in the specification though, but I don’t think it is the right place to have such normative hard constraints.
File extension
.clf sounds quite appropriate!
Log/Antilog
It is was more a nice to have than a strong request.
Unless I missed something, the base default value is 10 though? What I’m asking for is the natural logarithm ln here.
The explanation makes sense and adopted resolution sounds great, seems super important to make a good delineation here!
Cheers,
Thomas
PS: I haven’t re-read the updated specification yet, will be weekend duties
I empathise with the desire for completeness, but given that logarithms to different bases are convertible with a simple constant scale factor, there is an argument that the inclusion of base 2 and base 10 has a certain amount of redundancy for the sake of convenience. The linToLog and logToLin operators already include a base parameter, so a natural logarithm could be achieved by specifying e as the base (albeit to a fixed decimal precision) or by scaling logSideSlope.
Can you elaborate, @Thomas_Mansencal, on the reason for precise built-in ln() and exp() operators? As I have noted before, I don’t consider absolute precision to be a motivating factor, as LUTs are by their nature frequently imprecise.
Agreed with Thomas that the matrix dimensioning is my major concern too. It’s not about consistency but that the 3 a the end is redundant and doesn’t tell us anything useful. The definition merely needs to dimension the array, not combine dimensioning and “use” together. By the very dimension of the matrix it tells us it needs a vector3 input. Having the 3 at the end makes it look like the LUT definition of 3x 1D LUTs whereby each element of the matrix should be an RGB triple not a single number so yes, it’s inconsistent, but that’s not my primary reason for concern.
@Thomas_Mansencal, the priorities that inform my comments on any proposed changes are first, “how much value does it bring to end-users?”, and second, “how much complexity does it add to the implementations?”.
Looking at the proposal to change the way a Matrix is declared, if we start with the complexity issue, it is certainly making the code to support v3 and prior versions more complicated. Perhaps more significantly, it also adds new test cases that need to be added to the plug-fest/QA process and unit tests.
So making this change is definitely not ‘free’, but if there was a numerical issue that needed to be fixed or we wanted to add new capabilities, then it would very likely be worth doing.
And that bring us to my other consideration which is about how much value it brings to end-users. Hopefully we can agree that it actually brings no value to end-users of CLF.
You have argued it brings value to authors of CLF by making the syntax more like Matlab, etc. That is an aesthetic judgement and you may very well be correct, in any case I respect your opinion. On the other hand, it was not something I recall being mentioned during the v1 or v2 spec drafting meetings, so historically it has definitely not been a major concern.
Furthermore, CLFs will generally be authored via software rather than by hand, so the number of people actually encountering this syntax is very small. So to me, the cost/benefit trade-off is clearly not worth it.
Fundamentally however, I think changing the syntax from one version of the spec to another for reasons that are (in my opinion) hard to justify sets a bad precedent and could discourage vendors from wanting to implement these specs.
Thanks @doug_walker, I understand better where you are coming from and a lot of your points make sense. Please be reassured that I also respect your opinion. That being said I don’t agree with the cost/benefit comparison because following this logic you end up with something that is really inconsistent, and that will be full of warts in the coming years. I could for example go on for hours about Maya MEL API on this particular topic.
You say that CLFs will generally be generated by software rather by hand but I cannot really say. I’m still generating LUTs by hands quite often or part of them and as I’m both author and end-user, I would tend to think that I’m not the only one.
To the how much complexity it adds to the implementation question, the answer is contextual. If you have implemented support for CLF 1 & 2, sure it will be a bit more work. If you have not implemented support for any of those, well you are lucky because you will have less branching to do, no more special case about the Matrix or testing.
How much software has support for CLF currently and how much will support it? I’m ready to bet that there will be more software supporting CLF 3 in the future than the number of software currently supporting 1 & 2. If the spec is riddled with issues and inconsistencies like that, it might as well not entice people to implement it. I’m looking at the future here.
Scott, you are some kind of wonderful, I’ll have you know.
Hard Coded ranges
I don’t feel too strongly about whether or not constraints should be imposed in the spec; if not, I do think it’s worth drawing attention to potential problematic parameters, and perhaps providing ‘suggested’ or ‘appropriate’ range values in the literature – especially for attributes whose default values would elicit computation errors for certain styles.
Thanks for updating the language and rules pertaining to the quantity and quality of LogParam / ExponentParam elements permitted per node! That makes implementation much simpler. Very well articulated, as well. Nice work.
File extension
I’m down with CLF yeah you know me.
Log/Antilog
Interesting, I totally misread / misinterpreted what Thomas was suggesting.
As far as convenient defaults go, I definitely wouldn’t mind “e” as the default base value; appropriate precision could be determined by the implementation. Manually specifying 2s and 10s is comparatively painless.
So… just to put this out there… how would you guys feel about getting rid of the ‘style’ attribute entirely? Hear me out:
Most of the information conveyed by ‘style’ is implicit in the presence or absence of provided parameters.
a lone base attribute set to 2 or 10 implies log2/antiLog2 or log10/antiLog10, respectively (i.e., a linToLog / logToLin operation with default slope and offset values)
If a linSideBreak attribute is present, the node represents a piecewise camera style operation; if not… it’s one of the “affine” styles (linToLog or logToLin)
It seems to me that the only information uniquely conveyed by the style attribute is directionality
Which means style could be reduced to a choice of Log or antiLog… (this is what I thought Thomas was suggesting)
…or, preferably, replaced entirely with a “antiLog” boolean (antiLog = True means one of the “log-to-lin” algorithms).
On the other hand, removing styles means losing the convenience afforded by the log2/log10 styles – i.e., not having to provide LogParams. Personally speaking, as far as human-friendliness goes, I think the benefits of having a consistent place to look for parameter values outweighs the costs of having to explicitly provide LogParams in all cases. If you’re hand-writing CLF, you’ve already resigned yourself to a certain degree of inconvenience. Just my opinion. I won’t be heartbroken when you guys decide to veto my suggestion
ASC_CDL versioning
I’m still wrapping my head around XML, so forgive me if I’m missing something, but… wouldn’t it make sense to reference and inherit (extend?) types from the existing ASC CDL schema definition itself? If the version1.2 (or 1.1) CDL XSD is explicitly referenced as part of the CLF XSD, wouldn’t that provide de facto constraints on CDL version compatibility? I’m sure this has been discussed elsewhere; feel free to ignore if this isn’t a productive comment.
I didn’t realize this Array/Matrix issue would be the biggest sticking point. I sense both sides are pretty firmly rooted in their opinions
Personally I’ve never been that “bothered” by this, since the spec states what it means and what to expect. However, I’ve also not tried to implement CLF so never realized that it didn’t “make sense”. My comments here aren’t necessarily my opinion on what should happen to the spec, I’m just “thinking out loud” to explore this more.
So…playing as an advocate for @Thomas_Mansencal for a second…
For implementations… I’m no expert about how parsing (and testing) for XML usually works in implementations, so, I’m curious - do implementations really look at the values in the dim string to learn that it’s a Matrix? How is that 3rd value currently used? And why would it’s removal be so difficult for implementors to protect against?
It seems to me (if I were coding a CLF parser myself) that a robust implementation would examine the value assigned to a dim attribute string contained in an Array contained in a Matrix element to “validate” it. Currently, I would probably:
Is the Array in a Matrix element? If so, then my expectations are that it will have three values separated by white space. So…
Parse the string assigned to the dim attribute to see if there are 3 (and only 3) values separated by white space.
Check that those values are convertible to integers, and convert them.
Is the third value equal to 3?
Does the first value X second value equal the number of values that are separated by white space and contained within the Array element in the Matrix element?
Did I miss anything? If we agree these are reasonable steps to validate the node, then my question is, how much more complicated does it get if we remove step #4? If an Array is in a Matrix ProcessNode, we just do #5 (look at the first two digits). Any extra digits can be ignored. Or, if we want backward compatibility, we could add some complex if/then statements to also check the ‘compCLFversion’, etc. but even then, would simply ignoring the third dim value make a difference?
Then of course in the spec… we’d have to state that previous versions had a third value as part of the dim, but this styling is deprecated and implementations need only check the first two values in a dim string.
But then advocating for @doug_walker:
We could just leave it as is! Then:
It’s never been a major concern in v1 and v2.
It doesn’t really break anything.
Current implementations wouldn’t need to change parsing logic.
We’re just left with a syntax that is perhaps objectionable aesthetically and consistency (e.g. it’s just a quirk that for an Array contained in a Matrix, multiplying all the dimensions will not equal the number of elements contained in the Array).
Like Scott, I am also surprised that (after not coming up at all during the v1 and v2 spec writing) that this syntax issue has emerged as the main sticking point for v3.
I must say that I still believe that rewriting the syntax at every version sets a dangerous precedent and is unfriendly towards those that have invested time to help develop and implement the previous versions.
But I also believe that it is very important to somehow find consensus in these projects and often that requires making concessions.
If this would allow us to get Thomas and Graeme on board and move forward, I’m willing to withdraw my objection, as a gesture of good will.
or the definition changed and the way the Matrix case is specified updated accordingly.
This is my number one issue with the specification so far. As you know, I have sometimes (often?) a strong opinion on things and this is one of those cases 