-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SetAttributeValue and dictionary VRs of XS, OX #38
Comments
I should add that this problem is not unique to VR. The same is true for SpecificCharacterSet. If it is not already part of the dataset, then it must be explicitly given when adding encoded strings. Of course, the difference is that if SpecificCharacterSet is not given then the default is ASCII (quite reasonable) while a default VR of UN for XS is perhaps unreasonable. Perhaps XS should be an allowed VR in memory, but resolved to SS or US upon writing the file? This can also lead to complications because the value.AsInt() method needs to know the signed-ness of the stored value. So once again, it seems that the best option is to inform the user of the need to explicitly specify the VR. |
Commit b54a3a1 (2005/3/2, v0.6.4) changes this issue: it ensures that calling the vtkDICOMValue constructor with vr=XS will create a value with vr=SS (unless explicitly created from "unsigned short *"), and it ensures that calling the constructor with vr=OX will create a value with vr=OW. So, the current situation is as follows: When adding an attribute with dictionary type XS or OW to vtkDICOMMetaData, it will first try to resolve the VR by using e.g. PixelRepresentation or BitsAllocated, but if this is not possible, it will print a warning and the VR will default to SS or OW respectively. Adding a value of dictionary type XS or OW to vtkDICOMItem will simply cause the value to not be added. This seems very bad, I think defaulting to SS or OW is better. Eventually a validator could check it and correct it, if just the sign of the VR is wrong, but if the attribute is missing outright then information is lost. The list of (non-retired) elements with VR=XS or VR=OX is short:
|
XS should default to US (not SS), because these tags come before PixelRepresentation (0028,0103) and will always be unsigned due to their use: ZeroVelocityPixelValue is for ultrasound, MappedPixelValue is for Opthalmic. Both IODs require PixelRepresentation to be 0, and both of these tags require the VR to be US if PixelRepresentation is 0.
|
Commit 2cc9e0f does a reasonable job of fixing this. Now one vtkDICOMItem constructor can take a character set and the VR for XS as arguments, and another vtkDICOMItem constructor can take a vtkDICOMMetaData object from which it will deduce them. Now the only time there should be a problem is when someone uses the default vtkDICOMItem constructor, or when there is otherwise not enough information to deduce the character set or VR for XS. In that case, the character set is assumed to be ISO_IR 6, and the VR for XS is assumed to be US. |
The SetAttributeValue(vtkDICOMTag tag, double value) method will check the dictionary to find the correct VR to use with the tag. However, the dictionary can return a VR of XS or OX, in which case the real VR must be determined with a more complicated set of rules. For example, XS is usually US of the pixel data is unsigned (if PixelRepresentation is 0), otherwise it is SS.
If there is insufficient information to determine the sign of XS (or the size of OX) then there is trouble. The vtkDICOMMetaData::FindDictVR() method will raise an error, but vtkDICOMItem::FindDictVR() will return UN (and in debug mode, it will assert).
Currently, it is necessary for the user to explicitly add a vtkDICOMValue(VR::US, ..) or vtkDICOMValue(VR::SS, ...). Is there a better way of at least informing the user that this needs to be done?
The text was updated successfully, but these errors were encountered: