Replies: 3 comments
-
|
Hello :) We appreciate your interest in the OpenPrintTag project. This indeed is a specification oversight. The clause was mostly intended for the region-root CBOR map to allow easier additions. Inside the regions, using definite containers would likely make more sense in general, because it saves the one byte, but it's really more of a recommendation. The specification revision is warranted. Do you want to prepare the PR, or should we? |
Beta Was this translation helpful? Give feedback.
-
|
I've raised #145 that limits the recommendation to the root map for the AUX region. This differs somewhat from my original comment above where I was discussing this for all root containers, however, on reflection I feel that limiting the recommendation to just the AUX container is likely optimal. |
Beta Was this translation helpful? Give feedback.
-
|
Proposed changes implemented (by the OP) and merged. Thank you for your contribution :) |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
For my golang implementation for openprinttag, I like to run integraiton tests comparing the binary tags generated by this python reference implementation with that generated by my own. This helps me test and validate my implementation.
I recently updated to include the latest field definitions, including primary_lab_color and its associated lab_color type.
When running integration tests including this new type/field, I hit a binary comparison error due to difference in CBOR encoding.
Specifically, this implementation generates primary_lab_color as a definite array. I'll note that the specification states:
Whilst the use of a definite array for primary_lab_color doesn't break specification since the specification uses "SHOULD" and not "MUST", it caught me off-guard since my implementation strictly follows the indefinite container recommendation.
Consider the following minimal YAML definition:
Python tools produce the following binary tag:
The primary_color_lab field is found within the CBOR as the following bytes:
This is definite form array. I'll note that other array instances i've seen (such as tags) do use indefinite form.
Technically, I question whether the specification stating that indefinite is the preferred form for all containers is maybe a little over broad. In this case, definite form is probably the best choice (it saves a byte and has no discernible downside).
My understanding was the indefinite form use was primarily to facilitate the block centric update of sub-regions (particularly AUX) within the tag. I see few use cases outside of the region maps where the recommendation to use indefinite has any likelihood of benefit, and in most cases probably just wastes a byte of space on the tag.
Accordingly, I wonder if further consideration might be due to this statement within the specification. It seems that allowing the underlying CBOR engine to choose its preferred container (excepting for the region containers) would be more appropriate than the currently recommended approach.
Beta Was this translation helpful? Give feedback.
All reactions