diff --git a/dataset.go b/dataset.go index c9cfdc35..e5653157 100644 --- a/dataset.go +++ b/dataset.go @@ -71,13 +71,14 @@ func (d *Dataset) FindElementByTagNested(tag tag.Tag) (*Element, error) { // If for some reason your code will not exhaust the iterator (read all // elements), be sure to call ExhaustElementChannel to prevent leaving the // underlying Goroutine alive (you can safely do this in a defer). -// c := dataset.FlatIterator() -// defer ExhaustElementChannel(c) -// for elem := range c { -// // Even if you exit before reading everything in c (e.g. due to an -// // error) -// // things will be ok. -// } +// +// c := dataset.FlatIterator() +// defer ExhaustElementChannel(c) +// for elem := range c { +// // Even if you exit before reading everything in c (e.g. due to an +// // error) +// // things will be ok. +// } // // Note that the sequence element itself is sent on the channel in addition to // the child elements in the sequence. diff --git a/element.go b/element.go index 3c3d62a5..3c16be17 100644 --- a/element.go +++ b/element.go @@ -296,17 +296,32 @@ func (s *sequencesValue) MarshalJSON() ([]byte, error) { // PixelDataInfo is a representation of DICOM PixelData. type PixelDataInfo struct { - // IntentionallySkipped indicates if parsing/processing this PixelData tag - // was intentionally skipped. This is likely true if the dicom.SkipPixelData - // option was set. If true, the rest of this PixelDataInfo will be empty. - IntentionallySkipped bool - Frames []frame.Frame + // IntentionallySkipped indicates that reading the PixelData value was + // intentionally skipped and no Value data for this tag was read. + // This is likely true if the dicom.SkipPixelData option was set. If true, + // the rest of this PixelDataInfo will be empty. + IntentionallySkipped bool `json:"intentionallySkipped"` + + // Frames hold the processed PixelData frames (either Native or Encapsulated + // PixelData). + Frames []frame.Frame + // ParseErr indicates if there was an error when reading this Frame from the DICOM. // If this is set, this means fallback behavior was triggered to blindly write the PixelData bytes to an encapsulated frame. // The ParseErr will contain details about the specific error encountered. ParseErr error `json:"parseErr"` IsEncapsulated bool `json:"isEncapsulated"` Offsets []uint32 + + // IntentionallyUnprocessed indicates that the PixelData Value was actually + // read (as opposed to skipped over, as in IntentionallySkipped above) and + // blindly placed into RawData (if possible). Writing this element back out + // should work. This will be true if the + // dicom.SkipProcessingPixelDataValue flag is set with a PixelData tag. + IntentionallyUnprocessed bool `json:"intentionallyUnprocessed"` + // UnprocessedValueData holds the unprocessed Element value data if + // IntentionallyUnprocessed=true. + UnprocessedValueData []byte } // pixelDataValue represents DICOM PixelData diff --git a/parse.go b/parse.go index 08b588ff..89e3776b 100644 --- a/parse.go +++ b/parse.go @@ -113,7 +113,6 @@ type Parser struct { // provided). func NewParser(in io.Reader, bytesToRead int64, frameChannel chan *frame.Frame, opts ...ParseOption) (*Parser, error) { optSet := toParseOptSet(opts...) - p := Parser{ reader: &reader{ rawReader: dicomio.NewReader(bufio.NewReader(in), binary.LittleEndian, bytesToRead), @@ -211,6 +210,7 @@ type parseOptSet struct { skipMetadataReadOnNewParserInit bool allowMismatchPixelDataLength bool skipPixelData bool + skipProcessingPixelDataValue bool } func toParseOptSet(opts ...ParseOption) parseOptSet { @@ -236,11 +236,29 @@ func SkipMetadataReadOnNewParserInit() ParseOption { } } -// SkipPixelData skips parsing/processing the PixelData tag, wherever it appears +// SkipPixelData skips reading data from the PixelData tag, wherever it appears // (e.g. even if within an IconSequence). A PixelDataInfo will be added to the -// Dataset with the IntentionallySkipped property set to true. +// Dataset with the IntentionallySkipped property set to true, and no other +// data. Use this option if you don't need the PixelData value to be in the +// Dataset at all, and want to save both CPU and Memory. If you need the +// PixelData value in the Dataset (e.g. so it can be written out identically +// later) but _don't_ want to process/parse the value, see the +// SkipProcessingPixelDataValue option below. func SkipPixelData() ParseOption { return func(set *parseOptSet) { set.skipPixelData = true } } + +// SkipProcessingPixelDataValue will attempt to skip processing the _value_ +// of any PixelData elements. Unlike SkipPixelData(), this means the PixelData +// bytes will still be read into the Dataset, and can be written back out via +// this library's write functionality. But, if possible, the value will be read +// in as raw bytes with no further processing instead of being parsed. In the +// future, we may be able to extend this functionality to support on-demand +// processing of elements elsewhere in the library. +func SkipProcessingPixelDataValue() ParseOption { + return func(set *parseOptSet) { + set.skipProcessingPixelDataValue = true + } +} diff --git a/parse_test.go b/parse_test.go index 3c77f270..8d845e57 100644 --- a/parse_test.go +++ b/parse_test.go @@ -17,8 +17,8 @@ import ( "github.com/suyashkumar/dicom" ) -// 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. +// 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("./testdata") if err != nil { @@ -69,40 +69,85 @@ func TestNewParserSkipMetadataReadOnNewParserInit(t *testing.T) { } } -func TestNewParserSkipPixelData(t *testing.T) { +func TestParseFile_SkipPixelData(t *testing.T) { t.Run("WithSkipPixelData", func(t *testing.T) { - dataset, err := dicom.ParseFile("./testdata/1.dcm", nil, dicom.SkipPixelData()) - if err != nil { - t.Errorf("Unexpected error parsing dataset: %v", dataset) - } - el, err := dataset.FindElementByTag(tag.PixelData) - if err != nil { - t.Errorf("Unexpected error when finding PixelData in Dataset: %v", err) - } - pixelData := dicom.MustGetPixelDataInfo(el.Value) - if !pixelData.IntentionallySkipped { - t.Errorf("Expected pixelData.IntentionallySkipped=true, got false") - } - if got := len(pixelData.Frames); got != 0 { - t.Errorf("unexpected frames length. got: %v, want: %v", got, 0) - } + runForEveryTestFile(t, func(t *testing.T, filename string) { + dataset, err := dicom.ParseFile(filename, nil, dicom.SkipPixelData()) + if err != nil { + t.Errorf("Unexpected error parsing dataset: %v", dataset) + } + el, err := dataset.FindElementByTag(tag.PixelData) + if err != nil { + t.Errorf("Unexpected error when finding PixelData in Dataset: %v", err) + } + pixelData := dicom.MustGetPixelDataInfo(el.Value) + if !pixelData.IntentionallySkipped { + t.Errorf("Expected pixelData.IntentionallySkipped=true, got false") + } + if got := len(pixelData.Frames); got != 0 { + t.Errorf("unexpected frames length. got: %v, want: %v", got, 0) + } + }) }) t.Run("WithNOSkipPixelData", func(t *testing.T) { - dataset, err := dicom.ParseFile("./testdata/1.dcm", nil) - if err != nil { - t.Errorf("Unexpected error parsing dataset: %v", dataset) - } - el, err := dataset.FindElementByTag(tag.PixelData) - if err != nil { - t.Errorf("Unexpected error when finding PixelData in Dataset: %v", err) - } - pixelData := dicom.MustGetPixelDataInfo(el.Value) - if pixelData.IntentionallySkipped { - t.Errorf("Expected pixelData.IntentionallySkipped=false when SkipPixelData option not present, got true") - } - if len(pixelData.Frames) == 0 { - t.Errorf("unexpected frames length when SkipPixelData=false. got: %v, want: >0", len(pixelData.Frames)) - } + runForEveryTestFile(t, func(t *testing.T, filename string) { + dataset, err := dicom.ParseFile(filename, nil) + if err != nil { + t.Errorf("Unexpected error parsing dataset: %v", dataset) + } + el, err := dataset.FindElementByTag(tag.PixelData) + if err != nil { + t.Errorf("Unexpected error when finding PixelData in Dataset: %v", err) + } + pixelData := dicom.MustGetPixelDataInfo(el.Value) + if pixelData.IntentionallySkipped { + t.Errorf("Expected pixelData.IntentionallySkipped=false when SkipPixelData option not present, got true") + } + if len(pixelData.Frames) == 0 { + t.Errorf("unexpected frames length when SkipPixelData=false. got: %v, want: >0", len(pixelData.Frames)) + } + }) + }) +} + +func TestParseFile_SkipProcessingPixelDataValue(t *testing.T) { + t.Run("WithSkipProcessingPixelDataValue", func(t *testing.T) { + runForEveryTestFile(t, func(t *testing.T, filename string) { + dataset, err := dicom.ParseFile(filename, nil, dicom.SkipProcessingPixelDataValue()) + if err != nil { + t.Errorf("Unexpected error parsing dataset: %v", dataset) + } + el, err := dataset.FindElementByTag(tag.PixelData) + if err != nil { + t.Errorf("Unexpected error when finding PixelData in Dataset: %v", err) + } + pixelData := dicom.MustGetPixelDataInfo(el.Value) + if !pixelData.IntentionallyUnprocessed { + t.Errorf("Expected pixelData.IntentionallyUnprocessed=true, got false") + } + if got := len(pixelData.Frames); got != 0 { + t.Errorf("unexpected frames length. got: %v, want: %v", got, 0) + } + }) + }) + t.Run("WithNOSkipProcessingPixelDataValue", func(t *testing.T) { + runForEveryTestFile(t, func(t *testing.T, filename string) { + dataset, err := dicom.ParseFile(filename, nil) + if err != nil { + t.Errorf("Unexpected error parsing dataset: %v", dataset) + } + el, err := dataset.FindElementByTag(tag.PixelData) + if err != nil { + t.Errorf("Unexpected error when finding PixelData in Dataset: %v", err) + } + pixelData := dicom.MustGetPixelDataInfo(el.Value) + if pixelData.IntentionallyUnprocessed { + t.Errorf("Expected pixelData.IntentionallyUnprocessed=false when TestParseFile_SkipProcessingPixelDataValue option not present, got true") + } + if len(pixelData.Frames) == 0 { + t.Errorf("unexpected frames length when TestParseFile_SkipProcessingPixelDataValue=false. got: %v, want: >0", len(pixelData.Frames)) + } + }) }) } @@ -175,3 +220,18 @@ func Example_getImageFrames() { _ = f.Close() } } + +func runForEveryTestFile(t *testing.T, testFunc func(t *testing.T, filename string)) { + files, err := ioutil.ReadDir("./testdata") + if err != nil { + t.Fatalf("unable to read testdata/: %v", err) + } + for _, f := range files { + if !f.IsDir() && strings.HasSuffix(f.Name(), ".dcm") { + fullName := "./testdata/" + f.Name() + t.Run(fullName, func(t *testing.T) { + testFunc(t, fullName) + }) + } + } +} diff --git a/pkg/tag/tag.go b/pkg/tag/tag.go index 5c250e54..00b326cf 100644 --- a/pkg/tag/tag.go +++ b/pkg/tag/tag.go @@ -66,6 +66,19 @@ func (t Tag) String() string { return fmt.Sprintf("(%04x,%04x)", t.Group, t.Element) } +// Tags represents a set of Tag structs. +type Tags []*Tag + +// Contains returns true if the passed in tag exists within the Tags. +func (t Tags) Contains(item *Tag) bool { + for _, elem := range t { + if elem.Equals(*item) { + return true + } + } + return false +} + // Info stores detailed information about a Tag defined in the DICOM // standard. type Info struct { @@ -180,7 +193,7 @@ func MustFind(tag Tag) Info { // not part of the DICOM standard, or is retired from the standard, it returns // an error. // -// Example: FindTagByName("TransferSyntaxUID") +// Example: FindTagByName("TransferSyntaxUID") func FindByName(name string) (Info, error) { maybeInitTagDict() for _, ent := range tagDict { diff --git a/read.go b/read.go index 88cd21f4..6e653be5 100644 --- a/read.go +++ b/read.go @@ -233,6 +233,13 @@ func (r *reader) readPixelData(vl uint32, d *Dataset, fc chan<- *frame.Frame) (V return &pixelDataValue{PixelDataInfo{IntentionallySkipped: true}}, nil } + if r.opts.skipProcessingPixelDataValue { + val := &pixelDataValue{PixelDataInfo{IntentionallyUnprocessed: true}} + val.PixelDataInfo.UnprocessedValueData = make([]byte, vl) + _, err := io.ReadFull(r.rawReader, val.PixelDataInfo.UnprocessedValueData) + return val, err + } + // Assume we're reading NativeData data since we have a defined value length as per Part 5 Sec A.4 of DICOM spec. // We need Elements that have been already parsed (rows, cols, etc) to parse frames out of NativeData Pixel data if d == nil { diff --git a/read_test.go b/read_test.go index b553244e..9b8b3a63 100644 --- a/read_test.go +++ b/read_test.go @@ -595,6 +595,32 @@ func TestReadPixelData_SkipPixelData(t *testing.T) { } } +func TestReadPixelData_TrySkipProcessingPixelDataValue(t *testing.T) { + opts := parseOptSet{skipProcessingPixelDataValue: true} + valueBytes := []byte{1, 2, 3, 4, 5, 6} + dcmdata := bytes.NewBuffer(valueBytes) + + r := &reader{ + rawReader: dicomio.NewReader(bufio.NewReader(dcmdata), binary.LittleEndian, int64(dcmdata.Len())), + opts: opts, + } + val, err := r.readPixelData(6, &Dataset{}, nil) + if err != nil { + t.Errorf("unexpected error in readPixelData: %v", err) + } + pixelVal, ok := val.GetValue().(PixelDataInfo) + if !ok { + t.Errorf("Expected value to be of type PixelDataInfo") + } + if !pixelVal.IntentionallyUnprocessed { + t.Errorf("Expected PixelDataInfo to have IntentionallyUnprocessed=true") + } + if !cmp.Equal(pixelVal.UnprocessedValueData, valueBytes) { + t.Errorf("expected UnprocessedValueData to match valueBytes. got: %v, want: %v", pixelVal.UnprocessedValueData, valueBytes) + } + +} + func makeEncapsulatedSequence(t *testing.T) []byte { t.Helper() buf := &bytes.Buffer{} diff --git a/write.go b/write.go index 4bb6d88a..d5f63caa 100644 --- a/write.go +++ b/write.go @@ -562,6 +562,12 @@ func writePixelData(w dicomio.Writer, t tag.Tag, value Value, vr string, vl uint return err } } else { + // For now, IntentionallyUnprocessed will only happen for Native + // PixelData. + if image.IntentionallyUnprocessed { + w.WriteBytes(image.UnprocessedValueData) + return nil + } numFrames := len(image.Frames) numPixels := len(image.Frames[0].NativeData.Data) numValues := len(image.Frames[0].NativeData.Data[0]) diff --git a/write_test.go b/write_test.go index 3b682002..17852423 100644 --- a/write_test.go +++ b/write_test.go @@ -34,6 +34,7 @@ func TestWrite(t *testing.T) { extraElems []*Element expectedError error opts []WriteOption + parseOpts []ParseOption cmpOpts []cmp.Option }{ { @@ -441,6 +442,24 @@ func TestWrite(t *testing.T) { }}, expectedError: nil, }, + { + name: "PixelData with IntentionallyUnprocessed=true", + dataset: Dataset{Elements: []*Element{ + mustNewElement(tag.MediaStorageSOPClassUID, []string{"1.2.840.10008.5.1.4.1.1.1.2"}), + mustNewElement(tag.MediaStorageSOPInstanceUID, []string{"1.2.3.4.5.6.7"}), + mustNewElement(tag.TransferSyntaxUID, []string{uid.ImplicitVRLittleEndian}), + mustNewElement(tag.BitsAllocated, []int{8}), + mustNewElement(tag.FloatingPointValue, []float64{128.10}), + mustNewElement(tag.DimensionIndexPointer, []int{32, 36950}), + mustNewElement(tag.PixelData, PixelDataInfo{ + IntentionallyUnprocessed: true, + UnprocessedValueData: []byte{1, 2, 3, 4}, + IsEncapsulated: false, + }), + }}, + parseOpts: []ParseOption{SkipProcessingPixelDataValue()}, + expectedError: nil, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -464,7 +483,7 @@ func TestWrite(t *testing.T) { t.Fatalf("Unexpected error state file: %s: %v", file.Name(), err) } - readDS, err := Parse(f, info.Size(), nil) + readDS, err := Parse(f, info.Size(), nil, tc.parseOpts...) if err != nil { t.Errorf("Parse of written file, unexpected error: %v", err) }