From 7883858321be79b1e7ae09756494df439bce38a6 Mon Sep 17 00:00:00 2001 From: "k.kashimov" Date: Sun, 20 Dec 2020 07:00:07 +0600 Subject: [PATCH 1/9] pre-allocate pixel buffer in readNativeFrames; add GetByteOrder() to the Reader --- pkg/dicomio/reader.go | 5 +++++ read.go | 32 +++++++++++++++++--------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/pkg/dicomio/reader.go b/pkg/dicomio/reader.go index 29af4ee0..5894a711 100644 --- a/pkg/dicomio/reader.go +++ b/pkg/dicomio/reader.go @@ -67,6 +67,7 @@ type Reader interface { // SetCodingSystem sets the charset.CodingSystem to be used when ReadString // is called. SetCodingSystem(cs charset.CodingSystem) + GetByteOrder() binary.ByteOrder } type reader struct { @@ -234,3 +235,7 @@ func (r *reader) SetCodingSystem(cs charset.CodingSystem) { func (r *reader) Peek(n int) ([]byte, error) { return r.in.Peek(n) } + +func (r *reader) GetByteOrder() binary.ByteOrder { + return r.bo +} diff --git a/read.go b/read.go index 14e7fbf0..8a9cfb6d 100644 --- a/read.go +++ b/read.go @@ -212,6 +212,8 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr // Parse the pixels: image.Frames = make([]frame.Frame, nFrames) + bo := d.GetByteOrder() + pixelBuf := make([]byte, bitsAllocated/8) for frameIdx := 0; frameIdx < nFrames; frameIdx++ { // Init current frame currentFrame := frame.Frame{ @@ -226,24 +228,24 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr buf := make([]int, int(pixelsPerFrame)*samplesPerPixel) for pixel := 0; pixel < int(pixelsPerFrame); pixel++ { for value := 0; value < samplesPerPixel; value++ { + + // read into pre-allocated pixel buffer + n, err := d.Read(pixelBuf) + if err != nil { + return nil, bytesRead, + fmt.Errorf("could not read uint%d from input: %w", bitsAllocated, err) + } + if n < bitsAllocated/8 { + return nil, bytesRead, + fmt.Errorf("not enough bytes in the input to read uint%d", bitsAllocated) + } + if bitsAllocated == 8 { - val, err := d.ReadUInt8() - if err != nil { - return nil, bytesRead, err - } - buf[(pixel*samplesPerPixel)+value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(pixelBuf[0]) } else if bitsAllocated == 16 { - val, err := d.ReadUInt16() - if err != nil { - return nil, bytesRead, err - } - buf[(pixel*samplesPerPixel)+value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(bo.Uint16(pixelBuf)) } else if bitsAllocated == 32 { - val, err := d.ReadUInt32() - if err != nil { - return nil, bytesRead, err - } - buf[(pixel*samplesPerPixel)+value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(bo.Uint32(pixelBuf)) } } currentFrame.NativeData.Data[pixel] = buf[pixel*samplesPerPixel : (pixel+1)*samplesPerPixel] From 1e175d17d0647fa76576339be77b39e626cf7079 Mon Sep 17 00:00:00 2001 From: "k.kashimov" Date: Sun, 20 Dec 2020 07:05:18 +0600 Subject: [PATCH 2/9] add GetByteOrder() to the MockReader --- mocks/pkg/dicomio/mock_reader.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/mocks/pkg/dicomio/mock_reader.go b/mocks/pkg/dicomio/mock_reader.go index e7b239ce..b89bc1b6 100644 --- a/mocks/pkg/dicomio/mock_reader.go +++ b/mocks/pkg/dicomio/mock_reader.go @@ -289,3 +289,17 @@ func (mr *MockReaderMockRecorder) SetCodingSystem(cs interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetCodingSystem", reflect.TypeOf((*MockReader)(nil).SetCodingSystem), cs) } + +// GetByteOrder indicates an expected call of GetByteOrder +func (mr *MockReaderMockRecorder) GetByteOrder() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetByteOrder", reflect.TypeOf((*MockReader)(nil).GetByteOrder)) +} + +// GetByteOrder mocks base method +func (m *MockReader) GetByteOrder() binary.ByteOrder { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetByteOrder") + ret0, _ := ret[0].(binary.ByteOrder) + return ret0 +} From e423725cfe5141758f110c702bb95ff43392d196 Mon Sep 17 00:00:00 2001 From: kaxap Date: Sun, 20 Dec 2020 16:03:49 +0600 Subject: [PATCH 3/9] Update pkg/dicomio/reader.go Co-authored-by: Suyash Kumar --- pkg/dicomio/reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dicomio/reader.go b/pkg/dicomio/reader.go index 5894a711..25994fa5 100644 --- a/pkg/dicomio/reader.go +++ b/pkg/dicomio/reader.go @@ -67,7 +67,7 @@ type Reader interface { // SetCodingSystem sets the charset.CodingSystem to be used when ReadString // is called. SetCodingSystem(cs charset.CodingSystem) - GetByteOrder() binary.ByteOrder + ByteOrder() binary.ByteOrder } type reader struct { From 7f107ef43d3cb5e5e2a64bd22f58fda4b918e5a7 Mon Sep 17 00:00:00 2001 From: kaxap Date: Sun, 20 Dec 2020 16:03:57 +0600 Subject: [PATCH 4/9] Update read.go Co-authored-by: Suyash Kumar --- read.go | 1 - 1 file changed, 1 deletion(-) diff --git a/read.go b/read.go index 8a9cfb6d..944a855e 100644 --- a/read.go +++ b/read.go @@ -229,7 +229,6 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr for pixel := 0; pixel < int(pixelsPerFrame); pixel++ { for value := 0; value < samplesPerPixel; value++ { - // read into pre-allocated pixel buffer n, err := d.Read(pixelBuf) if err != nil { return nil, bytesRead, From 3ef4870b9203206152288cc88b5e57d0037cb5c3 Mon Sep 17 00:00:00 2001 From: kaxap Date: Sun, 20 Dec 2020 16:04:04 +0600 Subject: [PATCH 5/9] Update pkg/dicomio/reader.go Co-authored-by: Suyash Kumar --- pkg/dicomio/reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dicomio/reader.go b/pkg/dicomio/reader.go index 25994fa5..4e6064a0 100644 --- a/pkg/dicomio/reader.go +++ b/pkg/dicomio/reader.go @@ -236,6 +236,6 @@ func (r *reader) Peek(n int) ([]byte, error) { return r.in.Peek(n) } -func (r *reader) GetByteOrder() binary.ByteOrder { +func (r *reader) ByteOrder() binary.ByteOrder { return r.bo } From fb1393cd9c0cd5533f1ddea041939967466167dc Mon Sep 17 00:00:00 2001 From: kaxap Date: Sun, 20 Dec 2020 16:04:13 +0600 Subject: [PATCH 6/9] Update mocks/pkg/dicomio/mock_reader.go Co-authored-by: Suyash Kumar --- mocks/pkg/dicomio/mock_reader.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mocks/pkg/dicomio/mock_reader.go b/mocks/pkg/dicomio/mock_reader.go index b89bc1b6..9f060a4b 100644 --- a/mocks/pkg/dicomio/mock_reader.go +++ b/mocks/pkg/dicomio/mock_reader.go @@ -290,16 +290,16 @@ func (mr *MockReaderMockRecorder) SetCodingSystem(cs interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetCodingSystem", reflect.TypeOf((*MockReader)(nil).SetCodingSystem), cs) } -// GetByteOrder indicates an expected call of GetByteOrder -func (mr *MockReaderMockRecorder) GetByteOrder() *gomock.Call { +// ByteOrder indicates an expected call of GetByteOrder +func (mr *MockReaderMockRecorder) ByteOrder() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetByteOrder", reflect.TypeOf((*MockReader)(nil).GetByteOrder)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ByteOrder", reflect.TypeOf((*MockReader)(nil).ByteOrder)) } -// GetByteOrder mocks base method -func (m *MockReader) GetByteOrder() binary.ByteOrder { +// ByteOrder mocks base method +func (m *MockReader) ByteOrder() binary.ByteOrder { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetByteOrder") + ret := m.ctrl.Call(m, "ByteOrder") ret0, _ := ret[0].(binary.ByteOrder) return ret0 } From 9e5de7df260ea4c5c259a5deaf0879c1be1a4a4d Mon Sep 17 00:00:00 2001 From: kaxap Date: Sun, 20 Dec 2020 16:09:24 +0600 Subject: [PATCH 7/9] Update read.go --- read.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read.go b/read.go index 944a855e..fee8a97f 100644 --- a/read.go +++ b/read.go @@ -212,7 +212,7 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr // Parse the pixels: image.Frames = make([]frame.Frame, nFrames) - bo := d.GetByteOrder() + bo := d.ByteOrder() pixelBuf := make([]byte, bitsAllocated/8) for frameIdx := 0; frameIdx < nFrames; frameIdx++ { // Init current frame From 25288ffd28e9c449df6cb6a10e5a9128df6cc8a9 Mon Sep 17 00:00:00 2001 From: "k.kashimov" Date: Mon, 21 Dec 2020 00:55:31 +0600 Subject: [PATCH 8/9] Add sentinel ErrorIncompleteRead for readNativeFrames Add test for ErrorIncompleteRead minor refactoring --- read.go | 15 ++++++++------- read_test.go | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/read.go b/read.go index fee8a97f..be8022ea 100644 --- a/read.go +++ b/read.go @@ -23,6 +23,7 @@ var ( // ErrorUnsupportedVR indicates that this VR is not supported. ErrorUnsupportedVR = errors.New("unsupported VR") errorUnableToParseFloat = errors.New("unable to parse float type") + ErrorIncompleteRead = errors.New("unable to read specified number of bytes") ) func readTag(r dicomio.Reader) (*tag.Tag, error) { @@ -213,7 +214,8 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr // Parse the pixels: image.Frames = make([]frame.Frame, nFrames) bo := d.ByteOrder() - pixelBuf := make([]byte, bitsAllocated/8) + bytesAllocated := bitsAllocated / 8 + pixelBuf := make([]byte, bytesAllocated) for frameIdx := 0; frameIdx < nFrames; frameIdx++ { // Init current frame currentFrame := frame.Frame{ @@ -230,14 +232,13 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr for value := 0; value < samplesPerPixel; value++ { n, err := d.Read(pixelBuf) - if err != nil { + if err != nil || n < bytesAllocated { + if err == nil { + err = ErrorIncompleteRead + } return nil, bytesRead, fmt.Errorf("could not read uint%d from input: %w", bitsAllocated, err) } - if n < bitsAllocated/8 { - return nil, bytesRead, - fmt.Errorf("not enough bytes in the input to read uint%d", bitsAllocated) - } if bitsAllocated == 8 { buf[(pixel*samplesPerPixel)+value] = int(pixelBuf[0]) @@ -255,7 +256,7 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr } } - bytesRead = (bitsAllocated / 8) * samplesPerPixel * pixelsPerFrame * nFrames + bytesRead = bytesAllocated * samplesPerPixel * pixelsPerFrame * nFrames return &image, bytesRead, nil } diff --git a/read_test.go b/read_test.go index 8cba5b3d..b515787b 100644 --- a/read_test.go +++ b/read_test.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "encoding/binary" + "errors" "fmt" "math/rand" "strconv" @@ -249,6 +250,19 @@ func TestReadNativeFrames(t *testing.T) { }, expectedError: nil, }, + { + Name: "insufficient bytes, uint32", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), + mustNewElement(tag.NumberOfFrames, []string{"2"}), + mustNewElement(tag.BitsAllocated, []int{32}), + mustNewElement(tag.SamplesPerPixel, []int{2}), + }}, + data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3}, + expectedPixelData: nil, + expectedError: ErrorIncompleteRead, + }, { Name: "missing Columns", existingData: Dataset{Elements: []*Element{ @@ -278,7 +292,7 @@ func TestReadNativeFrames(t *testing.T) { } pixelData, _, err := readNativeFrames(r, &tc.existingData, nil) - if err != tc.expectedError { + if !errors.Is(err, tc.expectedError) { t.Errorf("TestReadNativeFrames(%v): did not get expected error. got: %v, want: %v", tc.data, err, tc.expectedError) } From 075bb7415be1e12eed2866521fb0698b6bbdbaac Mon Sep 17 00:00:00 2001 From: "k.kashimov" Date: Tue, 22 Dec 2020 08:58:06 +0600 Subject: [PATCH 9/9] Update ReadNativeFrames to use io.ReadFull instead of Read Remove ErrorIncompleteRead as a consequence Update ReadNativeFrames test accordingly --- read.go | 9 ++------- read_test.go | 3 ++- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/read.go b/read.go index be8022ea..b9958e21 100644 --- a/read.go +++ b/read.go @@ -23,7 +23,6 @@ var ( // ErrorUnsupportedVR indicates that this VR is not supported. ErrorUnsupportedVR = errors.New("unsupported VR") errorUnableToParseFloat = errors.New("unable to parse float type") - ErrorIncompleteRead = errors.New("unable to read specified number of bytes") ) func readTag(r dicomio.Reader) (*tag.Tag, error) { @@ -230,12 +229,8 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr buf := make([]int, int(pixelsPerFrame)*samplesPerPixel) for pixel := 0; pixel < int(pixelsPerFrame); pixel++ { for value := 0; value < samplesPerPixel; value++ { - - n, err := d.Read(pixelBuf) - if err != nil || n < bytesAllocated { - if err == nil { - err = ErrorIncompleteRead - } + _, err := io.ReadFull(d, pixelBuf) + if err != nil { return nil, bytesRead, fmt.Errorf("could not read uint%d from input: %w", bitsAllocated, err) } diff --git a/read_test.go b/read_test.go index b515787b..34912731 100644 --- a/read_test.go +++ b/read_test.go @@ -6,6 +6,7 @@ import ( "encoding/binary" "errors" "fmt" + "io" "math/rand" "strconv" "testing" @@ -261,7 +262,7 @@ func TestReadNativeFrames(t *testing.T) { }}, data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3}, expectedPixelData: nil, - expectedError: ErrorIncompleteRead, + expectedError: io.ErrUnexpectedEOF, }, { Name: "missing Columns",