Add option to skip Element Value processing of PixelData, while preserving roundtrip read/write

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.

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

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
+	}
+}

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)
+			})
+		}
+	}
+}

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 {

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 {

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{}

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])

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)
 				}