Skip to content

Commit

Permalink
Read elements with VR=UN and undefined VL as SQ (#331)
Browse files Browse the repository at this point in the history
This addresses #220 (and one of the dicoms in #327) by treating elements with a VR=UN and undefined VL as Sequences.

This pulls from work that I and @jabillings did previously (#235).

Some follow on changes may be needed to support this part of [the spec CP](https://dicom.nema.org/dicom/cp/cp246_01.pdf):
> If at some point an application knows the actual VR for an Attribute of VR UN (e.g. has its own applicable data dictionary), it can assume that the Value Field of the Attribute is encoded in Little Endian byte ordering with implicit VR encoding, irrespective of the current TransferSyntax.

---------

Co-authored-by: jabillings <[email protected]>
  • Loading branch information
suyashkumar and jabillings authored Jun 9, 2024
1 parent 65259e5 commit fe2aa28
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 3 deletions.
17 changes: 17 additions & 0 deletions dataset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,23 @@ func makeSequenceElement(tg tag.Tag, items [][]*Element) *Element {
}
}

func makeUNSequenceElement(tg tag.Tag, items [][]*Element) *Element {
sequenceItems := make([]*SequenceItemValue, 0, len(items))
for _, item := range items {
sequenceItems = append(sequenceItems, &SequenceItemValue{elements: item})
}

return &Element{
Tag: tg,
ValueRepresentation: tag.VRUnknown,
RawValueRepresentation: "UN",
Value: &sequencesValue{
value: sequenceItems,
},
ValueLength: tag.VLUndefinedLength,
}
}

func TestDataset_FindElementByTag(t *testing.T) {
data := Dataset{
Elements: []*Element{
Expand Down
7 changes: 6 additions & 1 deletion pkg/tag/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ const (
VRDate
// VRPixelData means the element stores a PixelDataInfo
VRPixelData
// VRUnknown means the VR of the element is unknown (possibly a private
// element seen while reading DICOMs in implicit transfer syntax).
VRUnknown
)

// GetVRKind returns the golang value encoding of an element with <tag, vr>.
Expand All @@ -144,7 +147,7 @@ func GetVRKind(tag Tag, vr string) VRKind {
return VRDate
case "AT":
return VRTagList
case "OW", "OB", "UN":
case "OW", "OB":
return VRBytes
case "LT", "UT":
return VRString
Expand All @@ -162,6 +165,8 @@ func GetVRKind(tag Tag, vr string) VRKind {
return VRFloat64List
case "SQ":
return VRSequence
case "UN":
return VRUnknown
default:
return VRStringList
}
Expand Down
12 changes: 12 additions & 0 deletions read.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,18 @@ func (r *reader) readValue(t tag.Tag, vr string, vl uint32, isImplicit bool, d *
return r.readPixelData(vl, d, fc)
case tag.VRFloat32List, tag.VRFloat64List:
return r.readFloat(t, vr, vl)
// More details on how we treat Unknown VRs can be found at
// https://github.com/suyashkumar/dicom/issues/220
// and
// https://github.com/suyashkumar/dicom/issues/231.
// TODO(suyashkumar): consider replacing UN VRs with SQ earlier on if they
// meet this criteria, so users of the Dataset can interact with it
// correctly.
case tag.VRUnknown:
if vl == tag.VLUndefinedLength {
return r.readSequence(t, vr, vl, d)
}
return r.readBytes(t, vr, vl)
default:
return r.readString(t, vr, vl)
}
Expand Down
4 changes: 3 additions & 1 deletion write.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,14 @@ func verifyValueType(t tag.Tag, value Value, vr string) error {
ok = valueType == Sequences
case "NA":
ok = valueType == SequenceItem
case vrraw.OtherWord, vrraw.OtherByte, vrraw.Unknown:
case vrraw.OtherWord, vrraw.OtherByte:
if t == tag.PixelData {
ok = valueType == PixelData
} else {
ok = valueType == Bytes
}
case vrraw.Unknown:
ok = valueType == Bytes || valueType == Sequences
case vrraw.FloatingPointSingle, vrraw.FloatingPointDouble:
ok = valueType == Floats
default:
Expand Down
37 changes: 36 additions & 1 deletion write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestWrite(t *testing.T) {
// Some tag with an unknown VR.
{
Tag: tag.Tag{0x0019, 0x1027},
ValueRepresentation: tag.VRBytes,
ValueRepresentation: tag.VRUnknown,
RawValueRepresentation: "UN",
ValueLength: 4,
Value: &bytesValue{
Expand Down Expand Up @@ -577,6 +577,41 @@ func TestWrite(t *testing.T) {
}},
parseOpts: []ParseOption{SkipPixelData()}, wantError: errorDeflatedTransferSyntaxUnsupported,
},
{
name: "nested unknown sequences",
dataset: Dataset{Elements: []*Element{
mustNewElement(tag.MediaStorageSOPClassUID, []string{"1.2.840.10008.5.1.4.1.1.1.13"}),
mustNewElement(tag.MediaStorageSOPInstanceUID, []string{"1.2.3.4.5.6.7"}),
mustNewElement(tag.TransferSyntaxUID, []string{uid.ImplicitVRLittleEndian}),
mustNewElement(tag.PatientName, []string{"Bob", "Jones"}),
makeUNSequenceElement(tag.Tag{0x0019, 0x1027}, [][]*Element{
// Item 1.
{
{
Tag: tag.Tag{0x0019, 0x1028},
ValueRepresentation: tag.VRUnknown,
RawValueRepresentation: "UN",
Value: &bytesValue{
value: []byte{0x1, 0x2, 0x3, 0x4},
},
},
// Nested Sequence.
makeUNSequenceElement(tag.Tag{0x0019, 0x1029}, [][]*Element{
{
{
Tag: tag.PatientName,
ValueRepresentation: tag.VRStringList,
RawValueRepresentation: "PN",
Value: &stringsValue{
value: []string{"Bob", "Jones"},
},
},
},
}),
},
}),
}},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down

0 comments on commit fe2aa28

Please sign in to comment.