diff --git a/.github/workflows/bench_pr.yml b/.github/workflows/bench_pr.yml new file mode 100644 index 00000000..bbfc8a7e --- /dev/null +++ b/.github/workflows/bench_pr.yml @@ -0,0 +1,35 @@ +name: bench_pr +on: [pull_request] +jobs: + build: + name: Benchmark (PR) + runs-on: ubuntu-latest + steps: + + - name: Set up Go 1.15 + uses: actions/setup-go@v2 + with: + go-version: 1.15 + id: go + + - name: Check out code + uses: actions/checkout@v1 + + - name: Get dependencies + run: | + go mod download + + - name: Benchmark against GITHUB_BASE_REF + run: | + export PATH=$PATH:$GOBIN + go get golang.org/x/perf/cmd/benchstat + echo "New Commit:" + git log -1 --format="%H" + go test -bench=. -benchtime=.75s -count=8 > $HOME/new.txt + git reset --hard HEAD + git checkout $GITHUB_BASE_REF + echo "Base Commit:" + git log -1 --format="%H" + go test -bench=. -benchtime=.75s -count=8 > $HOME/old.txt + benchstat $HOME/old.txt $HOME/new.txt + diff --git a/.github/workflows/bench_push.yml b/.github/workflows/bench_push.yml new file mode 100644 index 00000000..22c037af --- /dev/null +++ b/.github/workflows/bench_push.yml @@ -0,0 +1,33 @@ +name: bench_push +on: [push] +jobs: + build: + name: Benchmark (push) + runs-on: ubuntu-latest + steps: + - name: Set up Go 1.15 + uses: actions/setup-go@v2 + with: + go-version: 1.15 + id: go + + - name: Check out code + uses: actions/checkout@v1 + + - name: Get dependencies + run: | + go mod download + + - name: Benchmark (against HEAD~1) + run: | + export PATH=$PATH:$GOBIN + go get golang.org/x/perf/cmd/benchstat + echo "New Commit:" + git log -1 --format="%H" + go test -bench=. -benchtime=.75s -count=8 > $HOME/new.txt + git reset --hard HEAD + git checkout HEAD~1 + echo "Base Commit:" + git log -1 --format="%H" + go test -bench=. -benchtime=.75s -count=8 > $HOME/old.txt + benchstat $HOME/old.txt $HOME/new.txt diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index dda6f558..ff8f59ad 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -7,65 +7,26 @@ jobs: runs-on: ubuntu-latest steps: - - name: Set up Go 1.13 - uses: actions/setup-go@v1 + - name: Set up Go 1.15 + uses: actions/setup-go@v2 with: - go-version: 1.13 + go-version: 1.15 id: go - name: Check out code uses: actions/checkout@v1 - name: Get dependencies - run: | - export GOPATH=$HOME/go - export GOBIN=$GOPATH/bin - export REPO_PATH=$GOPATH/src/github.com/suyashkumar/dicom - mkdir -p $GOBIN - export PATH=$PATH:/home/runner/go/bin - mkdir -p $REPO_PATH - mv $(pwd)/* $REPO_PATH - cd $REPO_PATH + run: | go mod download - name: Build run: | - export GOPATH=$HOME/go - export GOBIN=$GOPATH/bin - export REPO_PATH=$GOPATH/src/github.com/suyashkumar/dicom - export PATH=$PATH:/home/runner/go/bin - cd $REPO_PATH - go build ./... make build-fast - name: Test run: | - export GOPATH=$HOME/go - export GOBIN=$GOPATH/bin - export REPO_PATH=$GOPATH/src/github.com/suyashkumar/dicom - export PATH=$PATH:/home/runner/go/bin - cd $REPO_PATH make test - - - name: Benchmark - run: | - export GOPATH=$HOME/go - export GOBIN=$GOPATH/bin - export REPO_PATH=$GOPATH/src/github.com/suyashkumar/dicom - export PATH=$PATH:/home/runner/go/bin - go get golang.org/x/perf/cmd/benchstat - cd $REPO_PATH - make - go test -bench . > $HOME/new.txt - cd $GITHUB_WORKSPACE - git reset --hard HEAD - git checkout $GITHUB_BASE_REF - rm -rf $REPO_PATH - mkdir -p $REPO_PATH - mv $(pwd)/* $REPO_PATH - cd $REPO_PATH - go test -bench . > $HOME/old.txt - benchstat $HOME/old.txt $HOME/new.txt diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..84c048a7 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +/build/ diff --git a/Makefile b/Makefile index fa6210cd..389c91a1 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ BINARY = dicomutil +VERSION = `git describe --tags --always` .PHONY: build build: @@ -23,10 +24,17 @@ run: release: go mod download $(MAKE) test - GOOS=linux GOARCH=amd64 go build -o build/${BINARY}-linux-amd64 ./cmd/dicomutil; - GOOS=darwin GOARCH=amd64 go build -o build/${BINARY}-darwin-amd64 ./cmd/dicomutil; - GOOS=windows GOARCH=amd64 go build -o build/${BINARY}-windows-amd64.exe ./cmd/dicomutil; + GOOS=linux GOARCH=amd64 go build -ldflags="-X 'main.GitVersion=${VERSION}'" -o build/${BINARY}-linux-amd64 ./cmd/dicomutil; + GOOS=darwin GOARCH=amd64 go build -ldflags="-X 'main.GitVersion=${VERSION}'" -o build/${BINARY}-darwin-amd64 ./cmd/dicomutil; + GOOS=windows GOARCH=amd64 go build -ldflags="-X 'main.GitVersion=${VERSION}'" -o build/${BINARY}-windows-amd64.exe ./cmd/dicomutil; cd build; \ tar -zcvf ${BINARY}-linux-amd64.tar.gz ${BINARY}-linux-amd64; \ tar -zcvf ${BINARY}-darwin-amd64.tar.gz ${BINARY}-darwin-amd64; \ zip -r ${BINARY}-windows-amd64.exe.zip ${BINARY}-windows-amd64.exe; + +bench-diff: + go test -bench . -count 5 > bench_current.txt + git checkout main + go test -bench . -count 5 > bench_main.txt + benchstat bench_main.txt bench_current.txt + git checkout - diff --git a/README.md b/README.md index d2e9080f..cbca0306 100755 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ Some notable features: To use this in your golang project, import `github.com/suyashkumar/dicom`. This repository supports Go modules, and regularly tags releases using semantic versioning. Typical usage is straightforward: ```go -dataset, _ := dicom.ParseFile("testfiles/1.dcm", nil) // See also: dicom.Parse which has a generic io.Reader API. +dataset, _ := dicom.ParseFile("testdata/1.dcm", nil) // See also: dicom.Parse which has a generic io.Reader API. // Dataset will nicely print the DICOM dataset data out of the box. fmt.Println(dataset) diff --git a/cmd/dicomutil/main.go b/cmd/dicomutil/main.go index 5d5f59be..79797817 100644 --- a/cmd/dicomutil/main.go +++ b/cmd/dicomutil/main.go @@ -17,7 +17,11 @@ import ( "github.com/suyashkumar/dicom/pkg/tag" ) +// GitVersion is the current version of dicomutil, will be replaced in release step with current git commit hash or tag. +var GitVersion = "unknown" + var ( + version = flag.Bool("version", false, "print current version and exit") filepath = flag.String("path", "", "path") extractImagesStream = flag.Bool("extract-images-stream", false, "Extract images using frame streaming capability") printJSON = flag.Bool("json", false, "Print dataset as JSON") @@ -28,6 +32,12 @@ const FrameBufferSize = 100 func main() { flag.Parse() + + if *version { + fmt.Printf("dicomutil: %s\n", GitVersion) + os.Exit(0) + } + if len(*filepath) > 0 { f, err := os.Open(*filepath) diff --git a/parse_test.go b/parse_test.go index 4098b540..fd30cf80 100644 --- a/parse_test.go +++ b/parse_test.go @@ -17,17 +17,17 @@ import ( "github.com/suyashkumar/dicom" ) -// TestParse is an end-to-end sanity check over DICOMs in testfiles/. Currently it only checks that no error is returned +// TestParse is an end-to-end sanity check over DICOMs in testdata/. Currently it only checks that no error is returned // when parsing the files. func TestParse(t *testing.T) { - files, err := ioutil.ReadDir("./testfiles") + files, err := ioutil.ReadDir("./testdata") if err != nil { - t.Fatalf("unable to read testfiles/: %v", err) + t.Fatalf("unable to read testdata/: %v", err) } for _, f := range files { if !f.IsDir() && strings.HasSuffix(f.Name(), ".dcm") { t.Run(f.Name(), func(t *testing.T) { - dcm, err := os.Open("./testfiles/" + f.Name()) + dcm, err := os.Open("./testdata/" + f.Name()) if err != nil { t.Errorf("Unable to open %s. Error: %v", f.Name(), err) } @@ -45,16 +45,16 @@ func TestParse(t *testing.T) { } } -// BenchmarkParse runs sanity benchmarks over the sample files in testfiles. +// BenchmarkParse runs sanity benchmarks over the sample files in testdata. func BenchmarkParse(b *testing.B) { - files, err := ioutil.ReadDir("./testfiles") + files, err := ioutil.ReadDir("./testdata") if err != nil { - b.Fatalf("unable to read testfiles/: %v", err) + b.Fatalf("unable to read testdata/: %v", err) } for _, f := range files { if !f.IsDir() && strings.HasSuffix(f.Name(), ".dcm") { b.Run(f.Name(), func(b *testing.B) { - dcm, err := os.Open("./testfiles/" + f.Name()) + dcm, err := os.Open("./testdata/" + f.Name()) if err != nil { b.Errorf("Unable to open %s. Error: %v", f.Name(), err) } @@ -76,7 +76,7 @@ func BenchmarkParse(b *testing.B) { func Example_readFile() { // See also: dicom.Parse, which uses a more generic io.Reader API. - dataset, _ := dicom.ParseFile("testfiles/1.dcm", nil) + dataset, _ := dicom.ParseFile("testdata/1.dcm", nil) // Dataset will nicely print the DICOM dataset data out of the box. fmt.Println(dataset) @@ -99,12 +99,12 @@ func Example_streamingFrames() { } }() - dataset, _ := dicom.ParseFile("testfiles/1.dcm", frameChan) + dataset, _ := dicom.ParseFile("testdata/1.dcm", frameChan) fmt.Println(dataset) // The full dataset } func Example_getImageFrames() { - dataset, _ := dicom.ParseFile("testfiles/1.dcm", nil) + dataset, _ := dicom.ParseFile("testdata/1.dcm", nil) pixelDataElement, _ := dataset.FindElementByTag(tag.PixelData) pixelDataInfo := dicom.MustGetPixelDataInfo(pixelDataElement.Value) for i, fr := range pixelDataInfo.Frames { diff --git a/pkg/frame/native.go b/pkg/frame/native.go index 6e7a5c47..9d00e459 100644 --- a/pkg/frame/native.go +++ b/pkg/frame/native.go @@ -35,7 +35,7 @@ func (n *NativeFrame) GetEncapsulatedFrame() (*EncapsulatedFrame, error) { func (n *NativeFrame) GetImage() (image.Image, error) { i := image.NewGray16(image.Rect(0, 0, n.Cols, n.Rows)) for j := 0; j < len(n.Data); j++ { - i.SetGray16(j%n.Cols, j/n.Rows, color.Gray16{Y: uint16(n.Data[j][0])}) // for now, assume we're not overflowing uint16, assume gray image + i.SetGray16(j%n.Cols, j/n.Cols, color.Gray16{Y: uint16(n.Data[j][0])}) // for now, assume we're not overflowing uint16, assume gray image } return i, nil } diff --git a/pkg/frame/native_test.go b/pkg/frame/native_test.go new file mode 100644 index 00000000..3d3ed3ba --- /dev/null +++ b/pkg/frame/native_test.go @@ -0,0 +1,93 @@ +package frame_test + +import ( + "image" + "testing" + + "github.com/suyashkumar/dicom/pkg/frame" +) + +// point represents a 2D point for testing. +type point struct { + x int + y int +} + +func TestNativeFrame_GetImage(t *testing.T) { + cases := []struct { + Name string + NativeFrame frame.NativeFrame + SetPoints []point + }{ + { + Name: "Square", + NativeFrame: frame.NativeFrame{ + Rows: 2, + Cols: 2, + Data: [][]int{{0}, {0}, {1}, {0}}, + }, + SetPoints: []point{{0, 1}}, + }, + { + Name: "Rectangle", + NativeFrame: frame.NativeFrame{ + Rows: 3, + Cols: 2, + Data: [][]int{{0}, {0}, {0}, {0}, {1}, {0}}, + }, + SetPoints: []point{{0, 2}}, + }, + { + Name: "Rectangle - multiple points", + NativeFrame: frame.NativeFrame{ + Rows: 5, + Cols: 3, + Data: [][]int{{0}, {0}, {0}, {0}, {1}, {1}, {0}, {0}, {0}, {0}, {1}, {0}, {0}, {0}, {0}}, + }, + SetPoints: []point{{1, 1}, {2, 1}, {1, 3}}, + }, + } + + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { + img, err := tc.NativeFrame.GetImage() + if err != nil { + t.Fatalf("GetImage(%v) got unexpected error: %v", tc.NativeFrame, err) + } + imgGray, ok := img.(*image.Gray16) + if !ok { + t.Fatalf("GetImage(%v) did not return an image convertible to Gray16", tc.NativeFrame) + } + + // Check that all pixels are zero except at the + // (ExpectedSetPointX, ExpectedSetPointY) point. + for x := 0; x < tc.NativeFrame.Cols; x++ { + for y := 0; y < tc.NativeFrame.Rows; y++ { + currValue := imgGray.Gray16At(x, y).Y + if within(point{x, y}, tc.SetPoints) { + if currValue != 1 { + t.Errorf("GetImage(%v), unexpected value at set point. got: %v, want: %v", tc.NativeFrame, currValue, 1) + } + continue + } + + if currValue != 0 { + t.Errorf("GetImage(%v), unexpected value outside of set point. At (%d, %d) got: %v, want: %v", tc.NativeFrame, x, y, currValue, 0) + } + + } + } + }) + + } +} + +// within returns true if pt is in the []point +func within(pt point, set []point) bool { + for _, item := range set { + if pt.x == item.x && pt.y == item.y { + return true + } + } + return false +} diff --git a/read.go b/read.go index fc671135..14e7fbf0 100644 --- a/read.go +++ b/read.go @@ -9,7 +9,6 @@ import ( "log" "strconv" "strings" - "unicode" "github.com/suyashkumar/dicom/pkg/dicomio" @@ -94,7 +93,7 @@ func readValue(r dicomio.Reader, t tag.Tag, vr string, vl uint32, isImplicit boo return readString(r, t, vr, vl) case tag.VRDate: return readDate(r, t, vr, vl) - case tag.VRUInt16List, tag.VRUInt32List, tag.VRInt16List, tag.VRInt32List: + case tag.VRUInt16List, tag.VRUInt32List, tag.VRInt16List, tag.VRInt32List, tag.VRTagList: return readInt(r, t, vr, vl) case tag.VRSequence: return readSequence(r, t, vr, vl) @@ -224,30 +223,30 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr Data: make([][]int, int(pixelsPerFrame)), }, } + buf := make([]int, int(pixelsPerFrame)*samplesPerPixel) for pixel := 0; pixel < int(pixelsPerFrame); pixel++ { - currentPixel := make([]int, samplesPerPixel) for value := 0; value < samplesPerPixel; value++ { if bitsAllocated == 8 { val, err := d.ReadUInt8() if err != nil { - return nil, bytesRead, errors.New("") + return nil, bytesRead, err } - currentPixel[value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(val) } else if bitsAllocated == 16 { val, err := d.ReadUInt16() if err != nil { - return nil, bytesRead, errors.New("") + return nil, bytesRead, err } - currentPixel[value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(val) } else if bitsAllocated == 32 { val, err := d.ReadUInt32() if err != nil { - return nil, bytesRead, errors.New("") + return nil, bytesRead, err } - currentPixel[value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(val) } } - currentFrame.NativeData.Data[pixel] = currentPixel + currentFrame.NativeData.Data[pixel] = buf[pixel*samplesPerPixel : (pixel+1)*samplesPerPixel] } image.Frames[frameIdx] = currentFrame if fc != nil { @@ -460,7 +459,7 @@ func readInt(r dicomio.Reader, t tag.Tag, vr string, vl uint32) (Value, error) { retVal := &intsValue{value: make([]int, 0, vl/2)} for !r.IsLimitExhausted() { switch vr { - case "US": + case "US", "AT": val, err := r.ReadUInt16() if err != nil { return nil, err @@ -553,9 +552,6 @@ func readRawItem(r dicomio.Reader) ([]byte, bool, error) { return nil, true, err } - if err != nil { - return nil, true, err - } if *t == tag.SequenceDelimitationItem { if vl != 0 { log.Printf("SequenceDelimitationItem's VL != 0: %d", vl) diff --git a/read_test.go b/read_test.go index 742f8444..8cba5b3d 100644 --- a/read_test.go +++ b/read_test.go @@ -5,9 +5,12 @@ import ( "bytes" "encoding/binary" "fmt" + "math/rand" + "strconv" "testing" "github.com/suyashkumar/dicom/pkg/dicomio" + "github.com/suyashkumar/dicom/pkg/frame" "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" @@ -132,3 +135,235 @@ func TestReadFloat_float32(t *testing.T) { }) } } + +func TestReadNativeFrames(t *testing.T) { + cases := []struct { + Name string + existingData Dataset + data []uint16 + expectedPixelData *PixelDataInfo + expectedError error + }{ + { + Name: "5x5, 1 frame, 1 samples/pixel", + 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}), + }}, + 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}, + 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}}, + }, + }, + }, + }, + expectedError: nil, + }, + { + Name: "2x2, 3 frames, 1 samples/pixel", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), + mustNewElement(tag.NumberOfFrames, []string{"3"}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + }}, + data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 0}, + expectedPixelData: &PixelDataInfo{ + IsEncapsulated: false, + Frames: []frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 2, + Cols: 2, + Data: [][]int{{1}, {2}, {3}, {2}}, + }, + }, + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 2, + Cols: 2, + Data: [][]int{{1}, {2}, {3}, {2}}, + }, + }, + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 2, + Cols: 2, + Data: [][]int{{1}, {2}, {3}, {0}}, + }, + }, + }, + }, + expectedError: nil, + }, + { + Name: "2x2, 2 frames, 2 samples/pixel", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{2}), + mustNewElement(tag.Columns, []int{2}), + mustNewElement(tag.NumberOfFrames, []string{"2"}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{2}), + }}, + data: []uint16{1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 2, 1, 2, 3, 5}, + expectedPixelData: &PixelDataInfo{ + IsEncapsulated: false, + Frames: []frame.Frame{ + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 2, + Cols: 2, + Data: [][]int{{1, 2}, {3, 2}, {1, 2}, {3, 2}}, + }, + }, + { + Encapsulated: false, + NativeData: frame.NativeFrame{ + BitsPerSample: 16, + Rows: 2, + Cols: 2, + Data: [][]int{{1, 2}, {3, 2}, {1, 2}, {3, 5}}, + }, + }, + }, + }, + expectedError: nil, + }, + { + Name: "missing Columns", + existingData: Dataset{Elements: []*Element{ + mustNewElement(tag.Rows, []int{5}), + mustNewElement(tag.NumberOfFrames, []string{"1"}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{1}), + }}, + 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}, + expectedPixelData: nil, + expectedError: ErrorElementNotFound, + }, + } + + 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") + } + } + + r, err := dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())) + if err != nil { + t.Errorf("TestReadFloat: unable to create new dicomio.Reader") + } + + pixelData, _, err := readNativeFrames(r, &tc.existingData, nil) + if err != tc.expectedError { + t.Errorf("TestReadNativeFrames(%v): did not get expected error. got: %v, want: %v", tc.data, err, tc.expectedError) + } + + if diff := cmp.Diff(tc.expectedPixelData, pixelData); diff != "" { + t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v", tc.data, diff) + } + }) + } +} + +func BenchmarkReadNativeFrames(b *testing.B) { + cases := []struct { + Name string + Rows int + Cols int + NumFrames int + SamplesPerPixel int + }{ + { + Name: "10x10, 10 frames, 1 sample/pixel", + Rows: 10, + Cols: 10, + NumFrames: 10, + SamplesPerPixel: 1, + }, + { + Name: "100x100, 10 frames, 1 sample/pixel", + Rows: 100, + Cols: 100, + NumFrames: 10, + SamplesPerPixel: 1, + }, + { + Name: "512x512, 10 frames, 1 sample/pixel", + Rows: 512, + Cols: 512, + NumFrames: 10, + SamplesPerPixel: 1, + }, + { + Name: "512x512, 10 frames, 5 sample/pixel", + Rows: 512, + Cols: 512, + NumFrames: 10, + SamplesPerPixel: 5, + }, + } + for _, c := range cases { + b.Run(c.Name, func(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) + } + }) + } +} + +func buildReadNativeFramesInput(rows, cols, numFrames, samplesPerPixel int, b *testing.B) (*Dataset, dicomio.Reader) { + b.Helper() + dataset := Dataset{ + Elements: []*Element{ + mustNewElement(tag.Rows, []int{rows}), + mustNewElement(tag.Columns, []int{cols}), + mustNewElement(tag.NumberOfFrames, []string{strconv.Itoa(numFrames)}), + mustNewElement(tag.BitsAllocated, []int{16}), + mustNewElement(tag.SamplesPerPixel, []int{samplesPerPixel}), + }, + } + dcmdata := bytes.Buffer{} + for fr := 0; fr < numFrames; fr++ { + for r := 0; r < rows; r++ { + for c := 0; c < cols; c++ { + for pxs := 0; pxs < samplesPerPixel; pxs++ { + if err := binary.Write(&dcmdata, binary.LittleEndian, uint16(rand.Intn(100))); err != nil { + b.Fatalf("TestReadNativeFrames: Unable to setup test buffer") + } + } + } + } + } + r, err := dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())) + if err != nil { + b.Errorf("buildReadNativeFramesInput: unable to create new dicomio.Reader") + } + + return &dataset, r +} diff --git a/testfiles/1.dcm b/testdata/1.dcm similarity index 100% rename from testfiles/1.dcm rename to testdata/1.dcm diff --git a/testfiles/2.dcm b/testdata/2.dcm similarity index 100% rename from testfiles/2.dcm rename to testdata/2.dcm diff --git a/testfiles/3.dcm b/testdata/3.dcm similarity index 100% rename from testfiles/3.dcm rename to testdata/3.dcm diff --git a/testfiles/4.dcm b/testdata/4.dcm similarity index 100% rename from testfiles/4.dcm rename to testdata/4.dcm diff --git a/testfiles/5.dcm b/testdata/5.dcm similarity index 100% rename from testfiles/5.dcm rename to testdata/5.dcm diff --git a/testfiles/data_details.md b/testdata/data_details.md similarity index 99% rename from testfiles/data_details.md rename to testdata/data_details.md index 0fb12dec..87975be5 100644 --- a/testfiles/data_details.md +++ b/testdata/data_details.md @@ -1,4 +1,4 @@ -# Testfiles Details +# Test files Details This document contains citations and further details for the test DICOMs used here. Eventually I would like to store a way more expansive set of test DICOMs @@ -60,4 +60,4 @@ Clark K, Vendt B, Smith K, Freymann J, Kirby J, Koppel P, Moore S, Phillips S, M #### File 5.dcm This file was sourced from [cornerstone](https://github.com/cornerstonejs/dicomParser/blob/master/testImages/encapsulated/multi-frame/CT0012.explicit_little_endian.dcm) -(which is MIT licensed, see the license reproduced in included_licenses.md) \ No newline at end of file +(which is MIT licensed, see the license reproduced in included_licenses.md) diff --git a/testfiles/included_licenses.md b/testdata/included_licenses.md similarity index 93% rename from testfiles/included_licenses.md rename to testdata/included_licenses.md index f4b66238..0002f139 100644 --- a/testfiles/included_licenses.md +++ b/testdata/included_licenses.md @@ -1,6 +1,6 @@ # Included Licenses -This is where licenses related to some of the testfiles are included as per the +This is where licenses related to some of the test files are included as per the clause in some licenses that require them to be reproduced in redistributions of portions of the software @@ -28,4 +28,4 @@ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. \ No newline at end of file +SOFTWARE. diff --git a/write.go b/write.go index fb1fa365..ddaf7f13 100644 --- a/write.go +++ b/write.go @@ -170,7 +170,10 @@ func writeFileHeader(w dicomio.Writer, ds *Dataset, metaElems []*Element, opts w if err != nil { return err } - w.WriteBytes(metaBytes.Bytes()) + err = w.WriteBytes(metaBytes.Bytes()) + if err != nil { + return err + } return nil } @@ -214,7 +217,10 @@ func writeElement(w dicomio.Writer, elem *Element, opts writeOptSet) error { if elem.Value != nil { // Write the bytes to the original writer - w.WriteBytes(valueData.Bytes()) + err = w.WriteBytes(valueData.Bytes()) + if err != nil { + return err + } } return nil } @@ -252,7 +258,7 @@ func verifyValueType(t tag.Tag, value Value, vr string) error { valueType := value.ValueType() var ok bool switch vr { - case "US", "UL", "SL", "SS": + case "US", "UL", "SL", "SS", "AT": ok = valueType == Ints case "SQ": ok = valueType == Sequences @@ -266,8 +272,6 @@ func verifyValueType(t tag.Tag, value Value, vr string) error { } case "FL", "FD": ok = valueType == Floats - case "AT": - fallthrough default: ok = valueType == Strings } @@ -343,7 +347,9 @@ func writeRawItem(w dicomio.Writer, data []byte) error { if err := writeVRVL(w, tag.Item, "NA", length); err != nil { return err } - w.WriteBytes(data) + if err := w.WriteBytes(data); err != nil { + return err + } return nil } @@ -442,7 +448,8 @@ func writeBytes(w dicomio.Writer, values []byte, vr string) error { func writeInts(w dicomio.Writer, values []int, vr string) error { for _, value := range values { switch vr { - case "US", "SS": + // TODO(suyashkumar): consider additional validation of VR=AT elements. + case "US", "SS", "AT": if err := w.WriteUInt16(uint16(value)); err != nil { return err } diff --git a/write_test.go b/write_test.go index 7b32b136..d0e450f2 100644 --- a/write_test.go +++ b/write_test.go @@ -16,6 +16,13 @@ import ( "github.com/suyashkumar/dicom/pkg/uid" ) +// TestWrite tests the write package by ensuring that it is consistent with the +// Parse implementation. In particular, it is tested by writing out known +// collections of Element and reading them back in using the Parse API and +// ensuing the read in collection is equal to the initial collection. +// +// This also serves to test that the Parse implementation is consistent with the +// Write implementation (e.g. it kinda goes both ways and covers Parse too). func TestWrite(t *testing.T) { cases := []struct { name string @@ -34,6 +41,7 @@ func TestWrite(t *testing.T) { mustNewElement(tag.PatientName, []string{"Bob", "Jones"}), mustNewElement(tag.Rows, []int{128}), mustNewElement(tag.FloatingPointValue, []float64{128.10}), + mustNewElement(tag.DimensionIndexPointer, []int{32, 36950}), }}, expectedError: nil, },