dcmjs icon indicating copy to clipboard operation
dcmjs copied to clipboard

VRInstances missing OL, OV, SV, and UV value representation (VR) types

Open GeoffCox opened this issue 4 months ago • 2 comments

The VRInstances used by createByTypeString cause logging of 'Invalid vr type - using UN' when missing.

static createByTypeString(type) {
        var vr = VRinstances[type];
        if (vr === undefined) {
            if (type == "ox") {
                // TODO: determine VR based on context (could be 1 byte pixel data)
                // https://github.com/dgobbi/vtk-dicom/issues/38
                validationLog.error("Invalid vr type", type, "- using OW");
                vr = VRinstances["OW"];
            } else if (type == "xs") {
                validationLog.error("Invalid vr type", type, "- using US");
                vr = VRinstances["US"];
            } else {
                validationLog.error("Invalid vr type", type, "- using UN");
                vr = VRinstances["UN"];
            }
        }
        return vr;
    }

During write this vr value is used to write bytes, but I think the VR value is preserved in writing and the writing of the raw bytes seems harmless for these types.

However, either the VRInstances should support writing OL, OV, SV, and UV with their own writers or if they map to using the UN write function then createByTypeString should not log a validation error for those VR strings.

GeoffCox avatar Sep 07 '25 17:09 GeoffCox

Nitpick: Please include deep links to the code instead of copying the code inline. E.g. here's the code in the repo: https://github.com/dcmjs-org/dcmjs/blob/167de51c745ed1f697783abc053ac5b3e33726f4/src/ValueRepresentation.js#L345-L362

pieper avatar Sep 15 '25 19:09 pieper

Definitely OX and XS handling should be fixed. These are VRs where the type is only known in the context of the rest of the file. Rather than throwing a validation error we should determine what they really are. (See this issue: https://github.com/dcmjs-org/dcmjs/issues/368).

Regarding OL, OV, SV, and UV, these should really just be added to the code explicitly. It should just follow the existing conventions so probably not a lot of work if someone wants to take it on (and add tests and testing data).

pieper avatar Sep 15 '25 19:09 pieper