Skip to content
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.

Sign up for GitHub

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

fix issue with odd number of pixel bytes #233

Merged
merged 12 commits into from
Jul 5, 2022
+148 −9
Merged

fix issue with odd number of pixel bytes #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8e0fcb0
fix issue with odd number of pixel bytes
jabillings Jan 29, 2022
ffe2177
prevent 0-length error
jabillings Feb 1, 2022
6b07638
address PR comments
jabillings Feb 1, 2022
b6fba09
improve error name
jabillings Feb 11, 2022
f164c9a
apply error name
jabillings Feb 11, 2022
bb50ebe
improve description
jabillings Feb 11, 2022
a447815
improve description
jabillings Feb 11, 2022
6823b58
Update read_test.go
jabillings Feb 11, 2022
59362b9
Update read_test.go
jabillings Feb 11, 2022
da8b1c7
Update read_test.go
jabillings Feb 11, 2022
b967112
update name to pixelVLOverride
jabillings Feb 11, 2022
5b7a64d
- fix nits
jabillings Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions read.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
// 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")
)

func readTag(r dicomio.Reader) (*tag.Tag, error) {
Expand Down Expand Up @@ -166,7 +167,7 @@ func readPixelData(r dicomio.Reader, t tag.Tag, vr string, vl uint32, d *Dataset
return nil, errors.New("the Dataset context cannot be nil in order to read Native PixelData")
}

i, _, err := readNativeFrames(r, d, fc)
i, _, err := readNativeFrames(r, d, fc, vl)

if err != nil {
return nil, err
Expand Down Expand Up @@ -222,7 +223,7 @@ func fillBufferSingleBitAllocated(pixelData []int, d dicomio.Reader, bo binary.B

// readNativeFrames reads NativeData frames from a Decoder based on already parsed pixel information
// that should be available in parsedData (elements like NumberOfFrames, rows, columns, etc)
func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Frame) (pixelData *PixelDataInfo,
func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Frame, vl uint32) (pixelData *PixelDataInfo,
bytesRead int, err error) {
image := PixelDataInfo{
IsEncapsulated: false,
Expand Down Expand Up @@ -323,7 +324,17 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr
}

bytesRead = bytesAllocated * samplesPerPixel * pixelsPerFrame * nFrames

if vl > 0 && uint32(bytesRead) == vl-1 {
if vl%2 != 0 {
// this error should never happen if the file conforms to the DICOM spec
return nil, bytesRead, fmt.Errorf("odd number of bytes specified for PixelData violates DICOM spec: %d : %w", vl, ErrorExpectedEvenLength)
}
err := d.Skip(1)
if err != nil {
return nil, bytesRead, fmt.Errorf("could not read padding byte: %w", err)
}
bytesRead++
}
return &image, bytesRead, nil
}

Expand Down
140 changes: 134 additions & 6 deletions read_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,10 @@ func TestReadNativeFrames(t *testing.T) {
Name string
existingData Dataset
data []uint16
dataBytes []byte
expectedPixelData *PixelDataInfo
expectedError error
pixelVLOverride uint32
}{
{
Name: "5x5, 1 frame, 1 samples/pixel",
Expand Down Expand Up @@ -364,14 +366,131 @@ func TestReadNativeFrames(t *testing.T) {
expectedPixelData: nil,
expectedError: ErrorUnsupportedBitsAllocated,
},
{
Name: "3x3, 3 frames, 1 samples/pixel, data bytes with padded 0",
existingData: Dataset{Elements: []*Element{
mustNewElement(tag.Rows, []int{3}),
mustNewElement(tag.Columns, []int{3}),
mustNewElement(tag.NumberOfFrames, []string{"3"}),
mustNewElement(tag.BitsAllocated, []int{8}),
mustNewElement(tag.SamplesPerPixel, []int{1}),
}},
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{
IsEncapsulated: false,
Frames: []frame.Frame{
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 3,
Cols: 3,
Data: [][]int{{11}, {12}, {13}, {21}, {22}, {23}, {31}, {32}, {33}},
},
},

{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 3,
Cols: 3,
Data: [][]int{{11}, {12}, {13}, {21}, {22}, {23}, {31}, {32}, {33}},
},
},
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 3,
Cols: 3,
Data: [][]int{{11}, {12}, {13}, {21}, {22}, {23}, {31}, {32}, {33}},
},
},
},
},
expectedError: nil,
},
{
Name: "1x1, 3 frames, 3 samples/pixel, data bytes with padded 0",
existingData: Dataset{Elements: []*Element{
mustNewElement(tag.Rows, []int{1}),
mustNewElement(tag.Columns, []int{1}),
mustNewElement(tag.NumberOfFrames, []string{"3"}),
mustNewElement(tag.BitsAllocated, []int{8}),
mustNewElement(tag.SamplesPerPixel, []int{3}),
}},
dataBytes: []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{
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 1,
Cols: 1,
Data: [][]int{{1, 2, 3}},
},
},
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 1,
Cols: 1,
Data: [][]int{{1, 2, 3}},
},
},
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 1,
Cols: 1,
Data: [][]int{{1, 2, 3}},
},
},
},
},
expectedError: nil,
},
{
Name: "1x1, 2 frames, 3 samples/pixel, bad pixel length",
existingData: Dataset{Elements: []*Element{
mustNewElement(tag.Rows, []int{1}),
mustNewElement(tag.Columns, []int{1}),
mustNewElement(tag.NumberOfFrames, []string{"2"}),
mustNewElement(tag.BitsAllocated, []int{8}),
mustNewElement(tag.SamplesPerPixel, []int{3}),
}},
dataBytes: []byte{1, 2, 3, 1, 2, 3},
expectedPixelData: nil,
pixelVLOverride: 7,
expectedError: ErrorExpectedEvenLength,
},
}

for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
dcmdata := bytes.Buffer{}
for _, item := range tc.data {
if err := binary.Write(&dcmdata, binary.LittleEndian, item); err != nil {
t.Errorf("TestReadNativeFrames: Unable to setup test 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")
}
}
} 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")
}
}
}

Expand All @@ -380,10 +499,19 @@ func TestReadNativeFrames(t *testing.T) {
t.Errorf("TestReadFloat: unable to create new dicomio.Reader")
}

pixelData, _, err := readNativeFrames(r, &tc.existingData, nil)
var vl uint32
if tc.pixelVLOverride > 0 {
vl = tc.pixelVLOverride
} else {
vl = uint32(dcmdata.Len())
}
pixelData, bytesRead, err := readNativeFrames(r, &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)
}
if err == nil && bytesRead != expectedBytes {
t.Errorf("TestReadNativeFrames(%v): did not read expected number of bytes. got: %d, want: %d", tc.data, bytesRead, expectedBytes)
}

if diff := cmp.Diff(tc.expectedPixelData, pixelData); diff != "" {
t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v", tc.data, diff)
Expand Down Expand Up @@ -475,7 +603,7 @@ func TestReadNativeFrames_OneBitAllocated(t *testing.T) {
t.Errorf("TestReadFloat: unable to create new dicomio.Reader")
}

pixelData, _, err := readNativeFrames(r, &tc.existingData, nil)
pixelData, _, err := readNativeFrames(r, &tc.existingData, nil, uint32(dcmdata.Len()))
if !errors.Is(err, tc.expectedError) {
t.Errorf("TestReadNativeFrames(%v): did not get expected error. got: %v, want: %v", tc.data, err, tc.expectedError)
}
Expand Down Expand Up @@ -529,7 +657,7 @@ func BenchmarkReadNativeFrames(b *testing.B) {
dataset, r := buildReadNativeFramesInput(c.Rows, c.Cols, c.NumFrames, c.SamplesPerPixel, b)
b.ResetTimer()
for i := 0; i < b.N; i++ {
_, _, _ = readNativeFrames(r, dataset, nil)
_, _, _ = readNativeFrames(r, dataset, nil, uint32(c.Rows*c.Cols*c.NumFrames))
}
})
}
Expand Down