Skip to content

Commit

Permalink
Return an error if Pixel Representation is signed and a signed native
Browse files Browse the repository at this point in the history 
pixel data value is found.
  • Loading branch information
@suyashkumar
suyashkumar committed Nov 6, 2023
1 parent 8f1f151 commit a87c2fa
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 15 deletions.
52 changes: 46 additions & 6 deletions read.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<<n)
if (1 << n & uint8(data)) > 0 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
71 changes: 62 additions & 9 deletions read_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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{
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
4 changes: 4 additions & 0 deletions write_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit a87c2fa

Please sign in to comment.