CLF Spec Review Period

[sdyer (Scott Dyer)]

@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…

Access live doc: https://www.overleaf.com/read/jdwpzdrdrnwv
Current (as-is) draft: CLF_3_0-2019-12-10.pdf (632.7 KB)

Some top issues that remain:

1. 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.

2. 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)

3. 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?

4. 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?

5. 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?

6. 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?

7. Casing consistency
   The sub elements in the Range node are title cased, even though they are not anywhere else.

8. 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)

@doug_walker

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).

• 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?

@doug_walker

4.4.8

• The CDL spec only places non-negativity bounds. Do we really want to place upper bounds?

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)

@doug_walker

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?

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.

@doug_walker

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.

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.

@doug_walker

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.

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.

@Thomas_Mansencal

It also goes beyond casing, for example, basicFwdMirror should really be basicMirrorFwd , see 32.1 .

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.

@Thomas_Mansencal

3 Specification (Page 7):

1. the sequence of transformations is described as a node-graph

I’ve modified this.

@Thomas_Mansencal

4.3 ProcessNode (Page 13):

1. ““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.”

64f has never been supported as input/output bit-depths. I’m not sure why (considering we seem to support everything else). Anybody?

@Thomas_Mansencal

4.4 Substitutes for ProcessNode (Page 13):

1. 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 .

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.