From a87c2fa02b11ead85b944fa599fc7853b10ddfce Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 22:45:02 -0500 Subject: [PATCH 01/10] Return an error if Pixel Representation is signed and a signed native pixel data value is found. --- read.go | 52 ++++++++++++++++++++++++++++++++----- read_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++------- write_test.go | 4 +++ 3 files changed, 112 insertions(+), 15 deletions(-) diff --git a/read.go b/read.go index 337735f9..d7c730a4 100644 --- a/read.go +++ b/read.go @@ -28,9 +28,10 @@ var ( // ErrorUnsupportedBitsAllocated indicates that the BitsAllocated in the // NativeFrame PixelData is unsupported. In this situation, the rest of the // dataset returned is still valid. - ErrorUnsupportedBitsAllocated = errors.New("unsupported BitsAllocated") - errorUnableToParseFloat = errors.New("unable to parse float type") - ErrorExpectedEvenLength = errors.New("field length is not even, in violation of DICOM spec") + ErrorUnsupportedBitsAllocated = errors.New("unsupported BitsAllocated") + errorUnableToParseFloat = errors.New("unable to parse float type") + ErrorExpectedEvenLength = errors.New("field length is not even, in violation of DICOM spec") + ErrorSignedNativePixelDataUnsupported = errors.New("the Pixel Representation tag indicates signed native pixel data is present, but this is not yet supported.") ) // reader is responsible for mid-level dicom parsing capabilities, like @@ -297,6 +298,19 @@ func (r *reader) readPixelData(vl uint32, d *Dataset, fc chan<- *frame.Frame) (V } +// isNegativeIn2sComplement returns true if the most significant bit is 1. +func isNegativeIn2sComplement(input any) bool { + switch v := input.(type) { + case uint8: + return (1 << 7 & v) > 0 + case uint16: + return (1 << 15 & v) > 0 + case uint32: + return (1 << 31 & v) > 0 + } + return false +} + func getNthBit(data byte, n int) int { debug.Logf("mask: %0b", 1< 0 { @@ -368,6 +382,20 @@ func makeErrorPixelData(reader io.Reader, vl uint32, fc chan<- *frame.Frame, par func (r *reader) readNativeFrames(parsedData *Dataset, fc chan<- *frame.Frame, vl uint32) (pixelData *PixelDataInfo, bytesToRead int, err error) { // Parse information from previously parsed attributes that are needed to parse NativeData Frames: + + // TODO(https://github.com/suyashkumar/dicom/issues/294): Add support for + // signed Native PixelData. + pixelDataIsSigned := false + pxRep, err := parsedData.FindElementByTag(tag.PixelRepresentation) + if err == nil { + // If found successfully, ensure it is 0, which indicates unsigned + // pixel values, which is the only thing we currently support. + pxRepValue := MustGetInts(pxRep.Value) + if len(pxRepValue) > 0 && pxRepValue[0] != 0 { + pixelDataIsSigned = true + } + } + rows, err := parsedData.FindElementByTag(tag.Rows) if err != nil { return nil, 0, err @@ -469,12 +497,24 @@ func (r *reader) readNativeFrames(parsedData *Dataset, fc chan<- *frame.Frame, v return nil, bytesToRead, fmt.Errorf("could not read uint%d from input: %w", bitsAllocated, err) } + insertIdx := (pixel * samplesPerPixel) + value if bitsAllocated == 8 { - buf[(pixel*samplesPerPixel)+value] = int(pixelBuf[0]) + buf[insertIdx] = int(pixelBuf[0]) + if pixelDataIsSigned && isNegativeIn2sComplement(pixelBuf[0]) { + return nil, bytesToRead, ErrorSignedNativePixelDataUnsupported + } } else if bitsAllocated == 16 { - buf[(pixel*samplesPerPixel)+value] = int(bo.Uint16(pixelBuf)) + val := bo.Uint16(pixelBuf) + buf[insertIdx] = int(val) + if pixelDataIsSigned && isNegativeIn2sComplement(val) { + return nil, bytesToRead, ErrorSignedNativePixelDataUnsupported + } } else if bitsAllocated == 32 { - buf[(pixel*samplesPerPixel)+value] = int(bo.Uint32(pixelBuf)) + val := bo.Uint32(pixelBuf) + buf[insertIdx] = int(val) + if pixelDataIsSigned && isNegativeIn2sComplement(val) { + return nil, bytesToRead, ErrorSignedNativePixelDataUnsupported + } } else { return nil, bytesToRead, fmt.Errorf("bitsAllocated=%d : %w", bitsAllocated, ErrorUnsupportedBitsAllocated) } diff --git a/read_test.go b/read_test.go index b8fe8002..2e2b31d8 100644 --- a/read_test.go +++ b/read_test.go @@ -210,7 +210,7 @@ func TestReadNativeFrames(t *testing.T) { cases := []struct { Name string existingData Dataset - data []uint16 + data []int16 dataBytes []byte expectedPixelData *PixelDataInfo expectedError error @@ -225,8 +225,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -251,8 +252,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"3"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 0}, + data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 0}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -295,8 +297,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"2"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{2}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 5}, + data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 5}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -330,8 +333,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"2"}), mustNewElement(tag.BitsAllocated, []int{32}), mustNewElement(tag.SamplesPerPixel, []int{2}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3}, + data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3}, expectedPixelData: nil, expectedError: ErrorMismatchPixelDataLength, }, @@ -343,8 +347,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{32}), mustNewElement(tag.SamplesPerPixel, []int{2}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, + data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, expectedPixelData: nil, expectedError: ErrorMismatchPixelDataLength, }, @@ -356,8 +361,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{32}), mustNewElement(tag.SamplesPerPixel, []int{2}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, + data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, expectedPixelData: &PixelDataInfo{ ParseErr: ErrorMismatchPixelDataLength, Frames: []*frame.Frame{ @@ -378,8 +384,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, expectedPixelData: nil, expectedError: ErrorElementNotFound, }, @@ -391,8 +398,9 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{24}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []uint16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, expectedPixelData: nil, expectedError: ErrorUnsupportedBitsAllocated, }, @@ -404,6 +412,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"3"}), mustNewElement(tag.BitsAllocated, []int{8}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, dataBytes: []byte{11, 12, 13, 21, 22, 23, 31, 32, 33, 11, 12, 13, 21, 22, 23, 31, 32, 33, 11, 12, 13, 21, 22, 23, 31, 32, 33, 0}, // there is a 28th byte to make total value length even, as required by DICOM spec expectedPixelData: &PixelDataInfo{ @@ -449,6 +458,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"3"}), mustNewElement(tag.BitsAllocated, []int{8}), mustNewElement(tag.SamplesPerPixel, []int{3}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, dataBytes: []byte{1, 2, 3, 1, 2, 3, 1, 2, 3, 0}, // 10th byte to make total value length even expectedPixelData: &PixelDataInfo{ @@ -493,12 +503,53 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"2"}), mustNewElement(tag.BitsAllocated, []int{8}), mustNewElement(tag.SamplesPerPixel, []int{3}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, dataBytes: []byte{1, 2, 3, 1, 2, 3}, expectedPixelData: nil, pixelVLOverride: 7, expectedError: ErrorExpectedEvenLength, }, + { + Name: "Signed Pixel Representation with No Negative Values: No Error", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{5}), + mustNewElement(tag.Columns, []int{5}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{1}), + }}, + data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + expectedPixelData: &PixelDataInfo{ + IsEncapsulated: false, + Frames: []*frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 5, + Cols: 5, + Data: [][]int{{1}, {2}, {3}, {4}, {5}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}}, + }, + }, + }, + }, + }, + { + Name: "Signed Pixel Representation with Negative Values: Returns Error", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{5}), + mustNewElement(tag.Columns, []int{5}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{1}), + }}, + data: []int16{-1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + expectedPixelData: nil, + expectedError: ErrorSignedNativePixelDataUnsupported, + }, } for _, tc := range cases { @@ -791,6 +842,7 @@ func TestReadNativeFrames_OneBitAllocated(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{1}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, data: []byte{0b00010111, 0b10010111}, expectedPixelData: &PixelDataInfo{ @@ -818,6 +870,7 @@ func TestReadNativeFrames_OneBitAllocated(t *testing.T) { mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{1}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), }}, data: []byte{0b00010111, 0b10010111}, expectedPixelData: &PixelDataInfo{ diff --git a/write_test.go b/write_test.go index 5c67fd59..4385d5da 100644 --- a/write_test.go +++ b/write_test.go @@ -284,6 +284,7 @@ func TestWrite(t *testing.T) { mustNewElement(tag.BitsAllocated, []int{8}), mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), mustNewElement(tag.PixelData, PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -314,6 +315,7 @@ func TestWrite(t *testing.T) { mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), mustNewElement(tag.PixelData, PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -342,6 +344,7 @@ func TestWrite(t *testing.T) { mustNewElement(tag.BitsAllocated, []int{32}), mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), mustNewElement(tag.PixelData, PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -370,6 +373,7 @@ func TestWrite(t *testing.T) { mustNewElement(tag.BitsAllocated, []int{32}), mustNewElement(tag.NumberOfFrames, []string{"2"}), mustNewElement(tag.SamplesPerPixel, []int{2}), + mustNewElement(tag.PixelRepresentation, []int{0}), mustNewElement(tag.PixelData, PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ From af0789d209f867d09f517f99b4606d7cb4504f2c Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 22:54:44 -0500 Subject: [PATCH 02/10] minor updates --- read.go | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/read.go b/read.go index d7c730a4..81d9e7d0 100644 --- a/read.go +++ b/read.go @@ -31,7 +31,7 @@ var ( ErrorUnsupportedBitsAllocated = errors.New("unsupported BitsAllocated") errorUnableToParseFloat = errors.New("unable to parse float type") ErrorExpectedEvenLength = errors.New("field length is not even, in violation of DICOM spec") - ErrorSignedNativePixelDataUnsupported = errors.New("the Pixel Representation tag indicates signed native pixel data is present, but this is not yet supported.") + ErrorSignedNativePixelDataUnsupported = errors.New("the Pixel Representation tag indicates signed native pixel data is present, _and_ negative Pixel Data values were found, but this is not yet supported") ) // reader is responsible for mid-level dicom parsing capabilities, like @@ -298,19 +298,6 @@ func (r *reader) readPixelData(vl uint32, d *Dataset, fc chan<- *frame.Frame) (V } -// isNegativeIn2sComplement returns true if the most significant bit is 1. -func isNegativeIn2sComplement(input any) bool { - switch v := input.(type) { - case uint8: - return (1 << 7 & v) > 0 - case uint16: - return (1 << 15 & v) > 0 - case uint32: - return (1 << 31 & v) > 0 - } - return false -} - func getNthBit(data byte, n int) int { debug.Logf("mask: %0b", 1< 0 { @@ -388,8 +375,6 @@ func (r *reader) readNativeFrames(parsedData *Dataset, fc chan<- *frame.Frame, v pixelDataIsSigned := false pxRep, err := parsedData.FindElementByTag(tag.PixelRepresentation) if err == nil { - // If found successfully, ensure it is 0, which indicates unsigned - // pixel values, which is the only thing we currently support. pxRepValue := MustGetInts(pxRep.Value) if len(pxRepValue) > 0 && pxRepValue[0] != 0 { pixelDataIsSigned = true @@ -869,3 +854,16 @@ func (r *reader) readRawItem(shouldSkip bool) ([]byte, bool, error) { func (r *reader) moreToRead() bool { return !r.rawReader.IsLimitExhausted() } + +// isNegativeIn2sComplement returns true if the most significant bit is 1. +func isNegativeIn2sComplement(input any) bool { + switch v := input.(type) { + case uint8: + return (1 << 7 & v) > 0 + case uint16: + return (1 << 15 & v) > 0 + case uint32: + return (1 << 31 & v) > 0 + } + return false +} From 431a05951727005bdf2a3b45e2e0316748cd8555 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 23:02:03 -0500 Subject: [PATCH 03/10] additional updates --- read.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/read.go b/read.go index 81d9e7d0..aac7427b 100644 --- a/read.go +++ b/read.go @@ -855,7 +855,8 @@ func (r *reader) moreToRead() bool { return !r.rawReader.IsLimitExhausted() } -// isNegativeIn2sComplement returns true if the most significant bit is 1. +// isNegativeIn2sComplement returns true if the most significant bit is 1 of the +// input unsigned integer. Panics if the input is not a Go unsigned integer. func isNegativeIn2sComplement(input any) bool { switch v := input.(type) { case uint8: @@ -864,6 +865,8 @@ func isNegativeIn2sComplement(input any) bool { return (1 << 15 & v) > 0 case uint32: return (1 << 31 & v) > 0 + case uint64: + return (1 << 63 & v) > 0 } - return false + panic("isNegativeIn2sComplement expects some form of Go unsigned integer") } From 8161503dcf63c3096b81db1167d6e11a368ecf60 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 23:04:26 -0500 Subject: [PATCH 04/10] additional updates --- read.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/read.go b/read.go index aac7427b..54b4e30d 100644 --- a/read.go +++ b/read.go @@ -373,8 +373,7 @@ func (r *reader) readNativeFrames(parsedData *Dataset, fc chan<- *frame.Frame, v // TODO(https://github.com/suyashkumar/dicom/issues/294): Add support for // signed Native PixelData. pixelDataIsSigned := false - pxRep, err := parsedData.FindElementByTag(tag.PixelRepresentation) - if err == nil { + if pxRep, err := parsedData.FindElementByTag(tag.PixelRepresentation); err == nil { pxRepValue := MustGetInts(pxRep.Value) if len(pxRepValue) > 0 && pxRepValue[0] != 0 { pixelDataIsSigned = true From 75727e47a119986547104267ad4311d4e90deafd Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 23:19:53 -0500 Subject: [PATCH 05/10] additional updates --- read_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/read_test.go b/read_test.go index 2e2b31d8..ce92ed40 100644 --- a/read_test.go +++ b/read_test.go @@ -5,6 +5,7 @@ import ( "bytes" "encoding/binary" "errors" + "math" "math/rand" "strconv" "testing" @@ -603,6 +604,54 @@ func TestReadNativeFrames(t *testing.T) { } } +func TestReadNativeFrames_MaxUInt16PixelValue(t *testing.T) { + // This tests that reading the maximum uint16 pixel value still works, when + // PixelRepresentation is 0, mostly as a regression test. + existingDS := Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{5}), + mustNewElement(tag.Columns, []int{5}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), + }} + pixelData := []uint16{math.MaxUint16, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} + want := &PixelDataInfo{ + IsEncapsulated: false, + Frames: []*frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 5, + Cols: 5, + Data: [][]int{{math.MaxUint16}, {2}, {3}, {4}, {5}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}}, + }, + }, + }, + } + + dcmdata := bytes.Buffer{} + for _, item := range pixelData { + if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { + t.Errorf("TestReadNativeFrames: Unable to setup test buffer") + } + } + + r := &reader{ + rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), + } + + parsedPixelData, _, err := r.readNativeFrames(&existingDS, nil, uint32(dcmdata.Len())) + if err != nil { + t.Errorf("TestReadNativeFrames(): did not get expected error. got: %v, want: %v", err, nil) + } + + if diff := cmp.Diff(want, parsedPixelData, cmpopts.EquateErrors()); diff != "" { + t.Errorf("TestReadNativeFrames(): unexpected diff: %v", diff) + } +} + func TestReadPixelData_SkipPixelData(t *testing.T) { cases := []struct { name string From f9d6b5f89d0f850e434870216959e908b1a7feb8 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 23:27:26 -0500 Subject: [PATCH 06/10] additional quick updates --- read_test.go | 108 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 102 insertions(+), 6 deletions(-) diff --git a/read_test.go b/read_test.go index ce92ed40..d4bcd9ee 100644 --- a/read_test.go +++ b/read_test.go @@ -604,18 +604,66 @@ func TestReadNativeFrames(t *testing.T) { } } +func TestReadNativeFrames_MaxUInt8PixelValue(t *testing.T) { + // This tests that reading the maximum uint8 pixel value still works, when + // PixelRepresentation is 0, mostly as a regression test. + existingDS := Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{1}), + mustNewElement(tag.Columns, []int{1}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{8}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), + }} + pixelData := []uint8{math.MaxUint8} + want := &PixelDataInfo{ + IsEncapsulated: false, + Frames: []*frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 8, + Rows: 1, + Cols: 1, + Data: [][]int{{math.MaxUint8}}, + }, + }, + }, + } + + dcmdata := bytes.Buffer{} + for _, item := range pixelData { + if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { + t.Errorf("TestReadNativeFrames: Unable to setup test buffer") + } + } + + r := &reader{ + rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), + } + + parsedPixelData, _, err := r.readNativeFrames(&existingDS, nil, uint32(dcmdata.Len())) + if err != nil { + t.Errorf("TestReadNativeFrames(): did not get expected error. got: %v, want: %v", err, nil) + } + + if diff := cmp.Diff(want, parsedPixelData, cmpopts.EquateErrors()); diff != "" { + t.Errorf("TestReadNativeFrames(): unexpected diff: %v", diff) + } +} + func TestReadNativeFrames_MaxUInt16PixelValue(t *testing.T) { // This tests that reading the maximum uint16 pixel value still works, when // PixelRepresentation is 0, mostly as a regression test. existingDS := Dataset{Elements: []*Element{ - mustNewElement(tag.Rows, []int{5}), - mustNewElement(tag.Columns, []int{5}), + mustNewElement(tag.Rows, []int{1}), + mustNewElement(tag.Columns, []int{1}), mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{0}), }} - pixelData := []uint16{math.MaxUint16, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} + pixelData := []uint16{math.MaxUint16} want := &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -623,9 +671,57 @@ func TestReadNativeFrames_MaxUInt16PixelValue(t *testing.T) { Encapsulated: false, NativeData: frame.NativeFrame{ BitsPerSample: 16, - Rows: 5, - Cols: 5, - Data: [][]int{{math.MaxUint16}, {2}, {3}, {4}, {5}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}}, + Rows: 1, + Cols: 1, + Data: [][]int{{math.MaxUint16}}, + }, + }, + }, + } + + dcmdata := bytes.Buffer{} + for _, item := range pixelData { + if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { + t.Errorf("TestReadNativeFrames: Unable to setup test buffer") + } + } + + r := &reader{ + rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), + } + + parsedPixelData, _, err := r.readNativeFrames(&existingDS, nil, uint32(dcmdata.Len())) + if err != nil { + t.Errorf("TestReadNativeFrames(): did not get expected error. got: %v, want: %v", err, nil) + } + + if diff := cmp.Diff(want, parsedPixelData, cmpopts.EquateErrors()); diff != "" { + t.Errorf("TestReadNativeFrames(): unexpected diff: %v", diff) + } +} + +func TestReadNativeFrames_MaxUInt32PixelValue(t *testing.T) { + // This tests that reading the maximum uint32 pixel value still works, when + // PixelRepresentation is 0, mostly as a regression test. + existingDS := Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{1}), + mustNewElement(tag.Columns, []int{1}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{32}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{0}), + }} + pixelData := []uint32{math.MaxUint32} + want := &PixelDataInfo{ + IsEncapsulated: false, + Frames: []*frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 32, + Rows: 1, + Cols: 1, + Data: [][]int{{math.MaxUint32}}, }, }, }, From 5263e25193d1e69f7cf855ec9a1183a144200f27 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 23:51:22 -0500 Subject: [PATCH 07/10] some updates --- read_test.go | 88 ++++++++++++++++------------------------------------ 1 file changed, 26 insertions(+), 62 deletions(-) diff --git a/read_test.go b/read_test.go index d4bcd9ee..f3a84291 100644 --- a/read_test.go +++ b/read_test.go @@ -556,25 +556,17 @@ func TestReadNativeFrames(t *testing.T) { for _, tc := range cases { tc := tc t.Run(tc.Name, func(t *testing.T) { - dcmdata := bytes.Buffer{} + var dcmdata *bytes.Buffer var expectedBytes int if len(tc.data) == 0 { // writing byte-by-byte expectedBytes = len(tc.dataBytes) - for _, item := range tc.dataBytes { - if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { - t.Errorf("TestReadNativeFrames: Unable to setup test buffer") - } - } + dcmdata = writeIntsToBuffer(t, tc.dataBytes) } else { // writing 2 bytes (uint16) at a time expectedBytes = len(tc.data) * 2 - for _, item := range tc.data { - if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { - t.Errorf("TestReadNativeFrames: Unable to setup test buffer") - } - } + dcmdata = writeIntsToBuffer(t, tc.data) } var vl uint32 @@ -585,7 +577,7 @@ func TestReadNativeFrames(t *testing.T) { } r := &reader{ - rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), + rawReader: dicomio.NewReader(bufio.NewReader(dcmdata), binary.LittleEndian, int64(dcmdata.Len())), opts: tc.parseOptSet, } @@ -631,25 +623,8 @@ func TestReadNativeFrames_MaxUInt8PixelValue(t *testing.T) { }, } - dcmdata := bytes.Buffer{} - for _, item := range pixelData { - if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { - t.Errorf("TestReadNativeFrames: Unable to setup test buffer") - } - } - - r := &reader{ - rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), - } - - parsedPixelData, _, err := r.readNativeFrames(&existingDS, nil, uint32(dcmdata.Len())) - if err != nil { - t.Errorf("TestReadNativeFrames(): did not get expected error. got: %v, want: %v", err, nil) - } - - if diff := cmp.Diff(want, parsedPixelData, cmpopts.EquateErrors()); diff != "" { - t.Errorf("TestReadNativeFrames(): unexpected diff: %v", diff) - } + dcmdata := writeIntsToBuffer(t, pixelData) + testReadNativeFramesBody(t, dcmdata, &existingDS, want) } func TestReadNativeFrames_MaxUInt16PixelValue(t *testing.T) { @@ -678,26 +653,8 @@ func TestReadNativeFrames_MaxUInt16PixelValue(t *testing.T) { }, }, } - - dcmdata := bytes.Buffer{} - for _, item := range pixelData { - if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { - t.Errorf("TestReadNativeFrames: Unable to setup test buffer") - } - } - - r := &reader{ - rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), - } - - parsedPixelData, _, err := r.readNativeFrames(&existingDS, nil, uint32(dcmdata.Len())) - if err != nil { - t.Errorf("TestReadNativeFrames(): did not get expected error. got: %v, want: %v", err, nil) - } - - if diff := cmp.Diff(want, parsedPixelData, cmpopts.EquateErrors()); diff != "" { - t.Errorf("TestReadNativeFrames(): unexpected diff: %v", diff) - } + dcmdata := writeIntsToBuffer(t, pixelData) + testReadNativeFramesBody(t, dcmdata, &existingDS, want) } func TestReadNativeFrames_MaxUInt32PixelValue(t *testing.T) { @@ -727,27 +684,34 @@ func TestReadNativeFrames_MaxUInt32PixelValue(t *testing.T) { }, } - dcmdata := bytes.Buffer{} - for _, item := range pixelData { - if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil { - t.Errorf("TestReadNativeFrames: Unable to setup test buffer") - } - } + dcmdata := writeIntsToBuffer(t, pixelData) + testReadNativeFramesBody(t, dcmdata, &existingDS, want) +} - r := &reader{ - rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())), - } +func testReadNativeFramesBody(t *testing.T, dcmdata *bytes.Buffer, existingDS *Dataset, wantPixelData *PixelDataInfo) { + r := &reader{rawReader: dicomio.NewReader(bufio.NewReader(dcmdata), binary.LittleEndian, int64(dcmdata.Len()))} - parsedPixelData, _, err := r.readNativeFrames(&existingDS, nil, uint32(dcmdata.Len())) + parsedPixelData, _, err := r.readNativeFrames(existingDS, nil, uint32(dcmdata.Len())) if err != nil { t.Errorf("TestReadNativeFrames(): did not get expected error. got: %v, want: %v", err, nil) } - if diff := cmp.Diff(want, parsedPixelData, cmpopts.EquateErrors()); diff != "" { + if diff := cmp.Diff(wantPixelData, parsedPixelData, cmpopts.EquateErrors()); diff != "" { t.Errorf("TestReadNativeFrames(): unexpected diff: %v", diff) } } +func writeIntsToBuffer[T any](t *testing.T, ints []T) *bytes.Buffer { + t.Helper() + dcmdata := &bytes.Buffer{} + for _, item := range ints { + if err := binary.Write(dcmdata, binary.LittleEndian, item); err != nil { + t.Errorf("unable to setup test buffer") + } + } + return dcmdata +} + func TestReadPixelData_SkipPixelData(t *testing.T) { cases := []struct { name string From 22805dab3d8ac9aebb96a0ad47876476653c2b89 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 5 Nov 2023 23:58:36 -0500 Subject: [PATCH 08/10] some more tests --- read_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/read_test.go b/read_test.go index f3a84291..e859ae01 100644 --- a/read_test.go +++ b/read_test.go @@ -512,7 +512,47 @@ func TestReadNativeFrames(t *testing.T) { expectedError: ErrorExpectedEvenLength, }, { - Name: "Signed Pixel Representation with No Negative Values: No Error", + Name: "Signed Pixel Representation with No Negative Values, uint8: No Error", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{8}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{1}), + }}, + dataBytes: []byte{1, 2, 3, 0}, + expectedPixelData: &PixelDataInfo{ + IsEncapsulated: false, + Frames: []*frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 8, + Rows: 2, + Cols: 2, + Data: [][]int{{1}, {2}, {3}, {0}}, + }, + }, + }, + }, + }, + { + Name: "Signed Pixel Representation with Negative Values, uint8: Returns Error", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{8}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + mustNewElement(tag.PixelRepresentation, []int{1}), + }}, + dataBytes: []byte{0b10000001, 2, 3, 0}, + expectedPixelData: nil, + expectedError: ErrorSignedNativePixelDataUnsupported, + }, + { + Name: "Signed Pixel Representation with No Negative Values, uint16: No Error", existingData: Dataset{Elements: []*Element{ mustNewElement(tag.Rows, []int{5}), mustNewElement(tag.Columns, []int{5}), @@ -538,7 +578,7 @@ func TestReadNativeFrames(t *testing.T) { }, }, { - Name: "Signed Pixel Representation with Negative Values: Returns Error", + Name: "Signed Pixel Representation with Negative Values, uint16: Returns Error", existingData: Dataset{Elements: []*Element{ mustNewElement(tag.Rows, []int{5}), mustNewElement(tag.Columns, []int{5}), From e41e1d037ed02df63d7ea3d08246eeaaa5888da6 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Mon, 6 Nov 2023 00:09:25 -0500 Subject: [PATCH 09/10] updates --- read_test.go | 81 ++++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/read_test.go b/read_test.go index e859ae01..d44f5477 100644 --- a/read_test.go +++ b/read_test.go @@ -211,8 +211,8 @@ func TestReadNativeFrames(t *testing.T) { cases := []struct { Name string existingData Dataset - data []int16 - dataBytes []byte + int16Data []int16 + uint8Data []byte expectedPixelData *PixelDataInfo expectedError error pixelVLOverride uint32 @@ -228,7 +228,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + int16Data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -255,7 +255,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 0}, + int16Data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 0}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -300,7 +300,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{2}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 5}, + int16Data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 5}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -336,7 +336,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{2}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3}, + int16Data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3}, expectedPixelData: nil, expectedError: ErrorMismatchPixelDataLength, }, @@ -350,7 +350,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{2}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, + int16Data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, expectedPixelData: nil, expectedError: ErrorMismatchPixelDataLength, }, @@ -364,7 +364,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{2}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, + int16Data: []int16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 2}, expectedPixelData: &PixelDataInfo{ ParseErr: ErrorMismatchPixelDataLength, Frames: []*frame.Frame{ @@ -387,7 +387,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + int16Data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, expectedPixelData: nil, expectedError: ErrorElementNotFound, }, @@ -401,12 +401,12 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + int16Data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, expectedPixelData: nil, expectedError: ErrorUnsupportedBitsAllocated, }, { - Name: "3x3, 3 frames, 1 samples/pixel, data bytes with padded 0", + Name: "3x3, 3 frames, 1 samples/pixel, int16Data bytes with padded 0", existingData: Dataset{Elements: []*Element{ mustNewElement(tag.Rows, []int{3}), mustNewElement(tag.Columns, []int{3}), @@ -415,7 +415,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - dataBytes: []byte{11, 12, 13, 21, 22, 23, 31, 32, 33, 11, 12, 13, 21, 22, 23, 31, 32, 33, 11, 12, 13, 21, 22, 23, 31, 32, 33, 0}, // there is a 28th byte to make total value length even, as required by DICOM spec + uint8Data: []byte{11, 12, 13, 21, 22, 23, 31, 32, 33, 11, 12, 13, 21, 22, 23, 31, 32, 33, 11, 12, 13, 21, 22, 23, 31, 32, 33, 0}, // there is a 28th byte to make total value length even, as required by DICOM spec expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -452,7 +452,7 @@ func TestReadNativeFrames(t *testing.T) { expectedError: nil, }, { - Name: "1x1, 3 frames, 3 samples/pixel, data bytes with padded 0", + Name: "1x1, 3 frames, 3 samples/pixel, int16Data bytes with padded 0", existingData: Dataset{Elements: []*Element{ mustNewElement(tag.Rows, []int{1}), mustNewElement(tag.Columns, []int{1}), @@ -461,7 +461,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{3}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - dataBytes: []byte{1, 2, 3, 1, 2, 3, 1, 2, 3, 0}, // 10th byte to make total value length even + uint8Data: []byte{1, 2, 3, 1, 2, 3, 1, 2, 3, 0}, // 10th byte to make total value length even expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -506,7 +506,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{3}), mustNewElement(tag.PixelRepresentation, []int{0}), }}, - dataBytes: []byte{1, 2, 3, 1, 2, 3}, + uint8Data: []byte{1, 2, 3, 1, 2, 3}, expectedPixelData: nil, pixelVLOverride: 7, expectedError: ErrorExpectedEvenLength, @@ -521,7 +521,7 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{1}), }}, - dataBytes: []byte{1, 2, 3, 0}, + uint8Data: []byte{1, 2, 3, 0}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -547,21 +547,21 @@ func TestReadNativeFrames(t *testing.T) { mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{1}), }}, - dataBytes: []byte{0b10000001, 2, 3, 0}, + uint8Data: []byte{0b10000001, 2, 3, 0}, expectedPixelData: nil, expectedError: ErrorSignedNativePixelDataUnsupported, }, { Name: "Signed Pixel Representation with No Negative Values, uint16: No Error", existingData: Dataset{Elements: []*Element{ - mustNewElement(tag.Rows, []int{5}), - mustNewElement(tag.Columns, []int{5}), + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{1}), }}, - data: []int16{1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + int16Data: []int16{1, 2, 3, 4}, expectedPixelData: &PixelDataInfo{ IsEncapsulated: false, Frames: []*frame.Frame{ @@ -569,9 +569,9 @@ func TestReadNativeFrames(t *testing.T) { Encapsulated: false, NativeData: frame.NativeFrame{ BitsPerSample: 16, - Rows: 5, - Cols: 5, - Data: [][]int{{1}, {2}, {3}, {4}, {5}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}}, + Rows: 2, + Cols: 2, + Data: [][]int{{1}, {2}, {3}, {4}}, }, }, }, @@ -580,14 +580,14 @@ func TestReadNativeFrames(t *testing.T) { { Name: "Signed Pixel Representation with Negative Values, uint16: Returns Error", existingData: Dataset{Elements: []*Element{ - mustNewElement(tag.Rows, []int{5}), - mustNewElement(tag.Columns, []int{5}), + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), mustNewElement(tag.NumberOfFrames, []string{"1"}), mustNewElement(tag.BitsAllocated, []int{16}), mustNewElement(tag.SamplesPerPixel, []int{1}), mustNewElement(tag.PixelRepresentation, []int{1}), }}, - data: []int16{-1, 2, 3, 4, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + int16Data: []int16{-1, 2, 3, 4}, expectedPixelData: nil, expectedError: ErrorSignedNativePixelDataUnsupported, }, @@ -599,14 +599,15 @@ func TestReadNativeFrames(t *testing.T) { var dcmdata *bytes.Buffer var expectedBytes int - if len(tc.data) == 0 { + if len(tc.uint8Data) > 0 { // writing byte-by-byte - expectedBytes = len(tc.dataBytes) - dcmdata = writeIntsToBuffer(t, tc.dataBytes) - } else { + expectedBytes = len(tc.uint8Data) + dcmdata = writeIntsToBuffer(t, tc.uint8Data) + } + if len(tc.int16Data) > 0 { // writing 2 bytes (uint16) at a time - expectedBytes = len(tc.data) * 2 - dcmdata = writeIntsToBuffer(t, tc.data) + expectedBytes = len(tc.int16Data) * 2 + dcmdata = writeIntsToBuffer(t, tc.int16Data) } var vl uint32 @@ -623,14 +624,14 @@ func TestReadNativeFrames(t *testing.T) { pixelData, bytesRead, err := r.readNativeFrames(&tc.existingData, nil, vl) if !errors.Is(err, tc.expectedError) { - t.Errorf("TestReadNativeFrames(%v): did not get expected error. got: %v, want: %v", tc.data, err, tc.expectedError) + t.Errorf("TestReadNativeFrames(%v): did not get expected error. got: %v, want: %v", tc.int16Data, err, tc.expectedError) } if err == nil && bytesRead != expectedBytes { - t.Errorf("TestReadNativeFrames(%v): did not read expected number of bytes. got: %d, want: %d", tc.data, bytesRead, expectedBytes) + t.Errorf("TestReadNativeFrames(%v): did not read expected number of bytes. got: %d, want: %d", tc.int16Data, bytesRead, expectedBytes) } if diff := cmp.Diff(tc.expectedPixelData, pixelData, cmpopts.EquateErrors()); diff != "" { - t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v", tc.data, diff) + t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v", tc.int16Data, diff) } }) } @@ -795,9 +796,9 @@ func TestReadPixelData_SkipPixelData(t *testing.T) { } } -// Used to encode the data from the generated headers. +// Used to encode the int16Data from the generated headers. type headerData struct { - // The byte encoded header data. + // The byte encoded header int16Data. HeaderBytes *bytes.Buffer // The decoded elements conforming the header. Elements []*Element @@ -890,7 +891,7 @@ func TestReadHeader_TryAllowErrorMetaElementGroupLength(t *testing.T) { t.Run("NoFileMetaInformationGroupLength", func(t *testing.T) { dcmheaderNoInfoGrpLen, err := headerWithNoFileMetaInformationGroupLength() if err != nil { - t.Fatalf("unsuccesful generation of fake header data") + t.Fatalf("unsuccesful generation of fake header int16Data") } else { r := &reader{ rawReader: dicomio.NewReader(bufio.NewReader(dcmheaderNoInfoGrpLen.HeaderBytes), binary.LittleEndian, int64(dcmheaderNoInfoGrpLen.HeaderBytes.Len())), @@ -911,7 +912,7 @@ func TestReadHeader_TryAllowErrorMetaElementGroupLength(t *testing.T) { t.Run("WithFileMetaInformationGroupLength", func(t *testing.T) { dcmHeaderInfoGrpLen, err := headerWithFileMetaInformationGroupLength() if err != nil { - t.Fatalf("unsuccesful generation of fake header data with FileMetaInformationGroupLength") + t.Fatalf("unsuccesful generation of fake header int16Data with FileMetaInformationGroupLength") } else { r := &reader{ rawReader: dicomio.NewReader(bufio.NewReader(dcmHeaderInfoGrpLen.HeaderBytes), binary.LittleEndian, int64(dcmHeaderInfoGrpLen.HeaderBytes.Len())), @@ -1062,7 +1063,7 @@ func TestReadNativeFrames_OneBitAllocated(t *testing.T) { } if diff := cmp.Diff(tc.expectedPixelData, pixelData); diff != "" { - t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v\ndata:%v", tc.data, diff, pixelData) + t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v\nint16Data:%v", tc.data, diff, pixelData) } }) } From 0f53fb47c2c27a2790cff8721af76698c33da548 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Mon, 6 Nov 2023 00:13:05 -0500 Subject: [PATCH 10/10] add todo --- read_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/read_test.go b/read_test.go index d44f5477..9cb91685 100644 --- a/read_test.go +++ b/read_test.go @@ -637,6 +637,8 @@ func TestReadNativeFrames(t *testing.T) { } } +// TODO(suyashkumar): revisit these tests and better structure/consolidate with +// above tests. func TestReadNativeFrames_MaxUInt8PixelValue(t *testing.T) { // This tests that reading the maximum uint8 pixel value still works, when // PixelRepresentation is 0, mostly as a regression test.