From 961caf0edca8f21e7c326c0f160fed9bbf8d64f4 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Fri, 28 Jan 2022 00:13:37 -0500 Subject: [PATCH 1/4] explore bug, try to repro --- write.go | 7 +++++++ write_test.go | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/write.go b/write.go index 4b148ff5..7085b5e9 100644 --- a/write.go +++ b/write.go @@ -210,6 +210,7 @@ func writeFileHeader(w dicomio.Writer, ds *Dataset, metaElems []*Element, opts w } func writeElement(w dicomio.Writer, elem *Element, opts writeOptSet) error { + fmt.Println("WRITE ELEMENT ", elem.Tag) vr := elem.RawValueRepresentation var err error vr, err = verifyVROrDefault(elem.Tag, elem.RawValueRepresentation, opts) @@ -220,10 +221,13 @@ func writeElement(w dicomio.Writer, elem *Element, opts writeOptSet) error { if !opts.skipValueTypeVerification && elem.Value != nil { err := verifyValueType(elem.Tag, elem.Value, vr) if err != nil { + fmt.Println("return") return err } } + fmt.Println("after verify") + length := elem.ValueLength var valueData = &bytes.Buffer{} if elem.Value != nil { @@ -323,6 +327,7 @@ func verifyValueType(t tag.Tag, value Value, vr string) error { ok = valueType == Floats default: ok = valueType == Strings + fmt.Println(valueType) } if !ok { @@ -457,6 +462,7 @@ func writeValue(w dicomio.Writer, t tag.Tag, value Value, valueType ValueType, v } func writeStrings(w dicomio.Writer, values []string, vr string) error { + fmt.Println("WRITE STR ", values) s := "" for i, substr := range values { if i > 0 { @@ -510,6 +516,7 @@ func writeInts(w dicomio.Writer, values []int, vr string) error { return err } case vrraw.UnsignedLong, vrraw.SignedLong: + fmt.Println("WRITE: ", value) if err := w.WriteUInt32(uint32(value)); err != nil { return err } diff --git a/write_test.go b/write_test.go index ae87fade..9f62abe9 100644 --- a/write_test.go +++ b/write_test.go @@ -3,6 +3,7 @@ package dicom import ( "bytes" "encoding/binary" + "fmt" "io/ioutil" "os" "testing" @@ -47,6 +48,16 @@ func TestWrite(t *testing.T) { mustNewElement(tag.FloatingPointValue, []float64{128.10}), mustNewElement(tag.DimensionIndexPointer, []int{32, 36950}), mustNewElement(tag.RedPaletteColorLookupTableData, []byte{0x1, 0x2, 0x3, 0x4}), + mustNewElement(tag.SelectorSLValue, []int{-20}), + { + Tag: tag.Tag{0x0019, 0x1026}, + ValueRepresentation: tag.VRStringList, + RawValueRepresentation: "UN", + ValueLength: 4, + Value: &stringsValue{ + value: []string{"1234"}, + }, + }, }}, expectedError: nil, }, @@ -468,6 +479,8 @@ func TestWrite(t *testing.T) { } cmpOpts = append(cmpOpts, tc.cmpOpts...) + fmt.Println(readDS.Elements) + if diff := cmp.Diff( wantElems, readDS.Elements, From ad70b3f6723017656e143a340544d8676f933440 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Fri, 28 Jan 2022 00:33:05 -0500 Subject: [PATCH 2/4] updates --- write_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/write_test.go b/write_test.go index 9f62abe9..3e86a82a 100644 --- a/write_test.go +++ b/write_test.go @@ -58,8 +58,18 @@ func TestWrite(t *testing.T) { value: []string{"1234"}, }, }, + { + Tag: tag.Tag{0x0019, 0x1027}, + ValueRepresentation: tag.VRInt32List, + RawValueRepresentation: "SL", + ValueLength: 4, + Value: &intsValue{ + value: []int{100}, + }, + }, }}, expectedError: nil, + opts: []WriteOption{SkipValueTypeVerification(), SkipVRVerification()}, }, { name: "private tag", From ec40a27e618c54896b07e892130da86e4faaaf4f Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Fri, 28 Jan 2022 21:31:22 -0500 Subject: [PATCH 3/4] UN with defined length should be read as OW --- pkg/tag/tag.go | 2 +- read.go | 3 +-- write.go | 11 ++--------- write_test.go | 22 +++++----------------- 4 files changed, 9 insertions(+), 29 deletions(-) diff --git a/pkg/tag/tag.go b/pkg/tag/tag.go index 36f1db57..5c250e54 100644 --- a/pkg/tag/tag.go +++ b/pkg/tag/tag.go @@ -128,7 +128,7 @@ func GetVRKind(tag Tag, vr string) VRKind { return VRDate case "AT": return VRTagList - case "OW", "OB": + case "OW", "OB", "UN": return VRBytes case "LT", "UT": return VRString diff --git a/read.go b/read.go index 78567994..fe4b829b 100644 --- a/read.go +++ b/read.go @@ -422,7 +422,7 @@ func readSequenceItem(r dicomio.Reader, t tag.Tag, vr string, vl uint32) (Value, func readBytes(r dicomio.Reader, t tag.Tag, vr string, vl uint32) (Value, error) { // TODO: add special handling of PixelData - if vr == vrraw.OtherByte { + if vr == vrraw.OtherByte || vr == vrraw.Unknown { data := make([]byte, vl) _, err := io.ReadFull(r, data) return &bytesValue{value: data}, err @@ -466,7 +466,6 @@ func readString(r dicomio.Reader, t tag.Tag, vr string, vl uint32) (Value, error // Split multiple strings strs := strings.Split(str, "\\") - return &stringsValue{value: strs}, err } diff --git a/write.go b/write.go index 7085b5e9..7d1f6409 100644 --- a/write.go +++ b/write.go @@ -210,7 +210,6 @@ func writeFileHeader(w dicomio.Writer, ds *Dataset, metaElems []*Element, opts w } func writeElement(w dicomio.Writer, elem *Element, opts writeOptSet) error { - fmt.Println("WRITE ELEMENT ", elem.Tag) vr := elem.RawValueRepresentation var err error vr, err = verifyVROrDefault(elem.Tag, elem.RawValueRepresentation, opts) @@ -221,13 +220,10 @@ func writeElement(w dicomio.Writer, elem *Element, opts writeOptSet) error { if !opts.skipValueTypeVerification && elem.Value != nil { err := verifyValueType(elem.Tag, elem.Value, vr) if err != nil { - fmt.Println("return") return err } } - fmt.Println("after verify") - length := elem.ValueLength var valueData = &bytes.Buffer{} if elem.Value != nil { @@ -317,7 +313,7 @@ func verifyValueType(t tag.Tag, value Value, vr string) error { ok = valueType == Sequences case "NA": ok = valueType == SequenceItem - case vrraw.OtherWord, vrraw.OtherByte: + case vrraw.OtherWord, vrraw.OtherByte, vrraw.Unknown: if t == tag.PixelData { ok = valueType == PixelData } else { @@ -327,7 +323,6 @@ func verifyValueType(t tag.Tag, value Value, vr string) error { ok = valueType == Floats default: ok = valueType == Strings - fmt.Println(valueType) } if !ok { @@ -462,7 +457,6 @@ func writeValue(w dicomio.Writer, t tag.Tag, value Value, valueType ValueType, v } func writeStrings(w dicomio.Writer, values []string, vr string) error { - fmt.Println("WRITE STR ", values) s := "" for i, substr := range values { if i > 0 { @@ -494,7 +488,7 @@ func writeStrings(w dicomio.Writer, values []string, vr string) error { func writeBytes(w dicomio.Writer, values []byte, vr string) error { var err error switch vr { - case vrraw.OtherWord: + case vrraw.OtherWord, vrraw.Unknown: err = writeOtherWordString(w, values) case vrraw.OtherByte: err = writeOtherByteString(w, values) @@ -516,7 +510,6 @@ func writeInts(w dicomio.Writer, values []int, vr string) error { return err } case vrraw.UnsignedLong, vrraw.SignedLong: - fmt.Println("WRITE: ", value) if err := w.WriteUInt32(uint32(value)); err != nil { return err } diff --git a/write_test.go b/write_test.go index 3e86a82a..3b682002 100644 --- a/write_test.go +++ b/write_test.go @@ -3,7 +3,6 @@ package dicom import ( "bytes" "encoding/binary" - "fmt" "io/ioutil" "os" "testing" @@ -49,27 +48,18 @@ func TestWrite(t *testing.T) { mustNewElement(tag.DimensionIndexPointer, []int{32, 36950}), mustNewElement(tag.RedPaletteColorLookupTableData, []byte{0x1, 0x2, 0x3, 0x4}), mustNewElement(tag.SelectorSLValue, []int{-20}), - { - Tag: tag.Tag{0x0019, 0x1026}, - ValueRepresentation: tag.VRStringList, - RawValueRepresentation: "UN", - ValueLength: 4, - Value: &stringsValue{ - value: []string{"1234"}, - }, - }, + // Some tag with an unknown VR. { Tag: tag.Tag{0x0019, 0x1027}, - ValueRepresentation: tag.VRInt32List, - RawValueRepresentation: "SL", + ValueRepresentation: tag.VRBytes, + RawValueRepresentation: "UN", ValueLength: 4, - Value: &intsValue{ - value: []int{100}, + Value: &bytesValue{ + value: []byte{0x1, 0x2, 0x3, 0x4}, }, }, }}, expectedError: nil, - opts: []WriteOption{SkipValueTypeVerification(), SkipVRVerification()}, }, { name: "private tag", @@ -489,8 +479,6 @@ func TestWrite(t *testing.T) { } cmpOpts = append(cmpOpts, tc.cmpOpts...) - fmt.Println(readDS.Elements) - if diff := cmp.Diff( wantElems, readDS.Elements, From 7a68236bf98cd2fddb1475aff526d60371b5b98b Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Fri, 28 Jan 2022 21:37:53 -0500 Subject: [PATCH 4/4] add test --- read_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/read_test.go b/read_test.go index 715ccf17..0d9820d4 100644 --- a/read_test.go +++ b/read_test.go @@ -168,12 +168,19 @@ func TestReadOWBytes(t *testing.T) { expectedErr error }{ { - name: "even-number bytes", + name: "OW VR with even-number bytes", bytes: []byte{0x1, 0x2, 0x3, 0x4}, VR: vrraw.OtherWord, want: &bytesValue{value: []byte{0x1, 0x2, 0x3, 0x4}}, expectedErr: nil, }, + { + name: "UN VR even-number bytes", + bytes: []byte{0x1, 0x2, 0x3, 0x4}, + VR: vrraw.Unknown, + want: &bytesValue{value: []byte{0x1, 0x2, 0x3, 0x4}}, + expectedErr: nil, + }, { name: "error on odd-number bytes", bytes: []byte{0x1, 0x2, 0x3},