hughbe / ICCReader

Swift definitions for structures, enumerations and functions defined in the [ICC Profile Format Specification]
9 stars 3 forks source link

minorVersion and refactoring tag #3

Open gmcusaro opened 3 years ago

gmcusaro commented 3 years ago

Hello Hugh, (I know, I’m a troublemaker😅)

Working on last version I’ve noticed some things:

func getVersionColorProfile(of profile: ICCColorProfile) -> String {
      let majorVersion = profile.header.version.majorVersion
      let minorVersion = profile.header.version.minorVersion
      let reserved1 = profile.header.version.reserved1
      return "\(majorVersion).\(minorVersion).\(reserved1)"
 }

For example sRGB_v4_ICC_preference_displayclass.icc (v. 4.2.0) appears 4.32.0, AdobeRGB1998.icc (v. 2.1.0) appears 2.16.0.

ICCTag(signature: ICCReader.ICCSignature(value: "desc"), type: ICCReader.ICCSignature(value: "desc"), data: ICCReader.ICCTagData.textDescription(ICCReader.textDescriptionType(sig: ICCReader.ICCSignature(value: "desc"), reserved: 0, count: 17, desc: "Adobe RGB (1998)", ucLangCode: 0, ucCount: 0, ucDesc: "", scCode: 0, scCount: 0, scDesc: “”)))

Are changes needed or am I probably doing something wrong to get for example only Adobe RGB (1998)?

hughbe commented 3 years ago

Thanks for all this!

minorVersion I tried several times with many profiles download from color.org, but the number doesn’t appear correct. This is my simple test function:

Well noticed! I have fixed this in a new commit, please update your package! (1.1.1)

Tag acsp This tag (pag. 16 of icc32.pdf) for profile file signature (byte offset 36-39) has not ecoding. I can think that it is only necessary to check its presence as rawValue in that byte offset as acsp. Using profile.getTag(signature: "acsp”) it returns always nil. Am I doing something wrong?

I don't think this tag actually exists as a value. It is simply present as a file version identifier. We already validate its existence

it is only present in the header:

        /// 36 to 39 4 ‘acsp’ (61637370h) profile file Signature 4-byte signature; see 7.2.11
        /// 7.2.11 Profile file Signature field (bytes 36 to 39)
        /// The profile file Signature field shall contain the value ‘acsp’ (61637379h) as a profile file Signature.
        self.magic = try ICCSignature(dataStream: &dataStream)
        guard self.magic == "acsp" else {
            throw ICCReadError.corrupted
        }

Tag desc The previous refactor facilitated access to header tags like profile.getTag(signature: ICCTagSignature.copyright)!.data. Getting the profile name only desc tag is no longer possible. Using profile.getTag(signature: ICCTagSignature.profileDescription.value) the console log is:

Sorry I don't fully understand - what are you trying to do?

Have you tried doing something like

let data = try getData(name: "AdobeRGB1998", fileExtension: "icc")
let file = try ICCColorProfile(data: data)
let description = file.profileDescription!
guard case let .textDescription(desc) = description else {
    fatalError("No desc tag")
}    
print(desc.desc) // Adobe RGB (1998)
hughbe commented 3 years ago

(https://github.com/hughbe/ICCReader/commit/db0ac2190004505ee35c9ea9e2282a6262e0484e)

gmcusaro commented 3 years ago

Thanks for the care you give to the project!

About acsp you are right, I can use catch for the corrupt profile with enum ICCReadError.corrupted and localize the string.

About name profile, your code is good, I was referring to a solution more like this profile.header.version.majorVersion in header tags. Something like profile.description.name.rawValue

Finally, I presume that subClassMajorVersion replace the old reserved1 and subClassMinorVersion replace reserved2. bugFixVersion maybe is for testing?

// Could this be correct?
func getVersionColorProfile(of profile: ICCColorProfile) -> String? {
        let majorVersion = profile.header.version.majorVersion
        let minorVersion = profile.header.version.minorVersion
        let subClassMajorVersion = profile.header.version.subClassMajorVersion
        return "\(majorVersion).\(minorVersion).\(subClassMajorVersion)"
    }
hughbe commented 3 years ago

I will also create a dedicated error such as invalidSignature because this may be common

e.g. people may try to read a jpeg etc

so you can just catch ICCReadError.invalidSignature

I think you should use bugFixVersion... subClassVersion is deprecated

hughbe commented 3 years ago

E.g. in iccMAX Specification (http://www.color.org/specification/ICC1v43_2010-12.pdf)

7.2.4 Profile version field (bytes 8 to 11)
The profile version with which the profile is compliant shall be encoded as binary-coded decimal in the profile
version field. The first byte (byte 8) shall identify the major version and byte 9 shall identify the minor version
and bug fix version in each 4-bit half of the byte. Bytes 10 and 11 are reserved and shall be set to zero. The
major and minor versions are set by the International Color Consortium. The profile version number consistent
with this ICC specification is “4.3.0.0” (encoded as 04300000h).
NOTE A major version number change occurs only when changes made to the specification require that both CMMs
and profile generating software be upgraded in order to correctly use or produce profiles conforming to the revised
specification. A minor version number change will occur when profiles conforming to the revised specification can be
processed by existing CMMs. For example, adding a required tag would require a major revision to the specification,
whereas adding an optional tag would only require a minor revision. 
gmcusaro commented 3 years ago

Yes, people may try to read other file type.

Sorry, I noticed that deviceSubClass before deviceClass now return ICCSignature(value: "\0\0\0\0") and not values like scnr mntr prtr etc. You too?