Introduce option to SkipPixelData (#255)

This introduces an option to SkipPixelData processing, making use of the new reader struct introduced in #254.

See also, review discussion in #243 that helped motivate the changes I wanted to make in #254.

element.go

@@ -56,7 +56,7 @@ func (e *Element) String() string {
 //			// or
 //			s := myvalue.GetValue().([]string)
 //			break;
-// 		case dicom.Bytes:
+//		case dicom.Bytes:
 //			// ...
 //	}
 //
@@ -296,7 +296,11 @@ func (s *sequencesValue) MarshalJSON() ([]byte, error) {
 
 // PixelDataInfo is a representation of DICOM PixelData.
 type PixelDataInfo struct {
-	Frames []frame.Frame
+	// 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
 	// 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.

parse.go

@@ -210,6 +210,7 @@ type ParseOption func(*parseOptSet)
 type parseOptSet struct {
 	skipMetadataReadOnNewParserInit bool
 	allowMismatchPixelDataLength    bool
+	skipPixelData                   bool
 }
 
 func toParseOptSet(opts ...ParseOption) parseOptSet {
@@ -234,3 +235,12 @@ func SkipMetadataReadOnNewParserInit() ParseOption {
 		set.skipMetadataReadOnNewParserInit = true
 	}
 }
+
+// SkipPixelData skips parsing/processing 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.
+func SkipPixelData() ParseOption {
+	return func(set *parseOptSet) {
+		set.skipPixelData = true
+	}
+}

parse_test.go

@@ -69,6 +69,43 @@ func TestNewParserSkipMetadataReadOnNewParserInit(t *testing.T) {
 	}
 }
 
+func TestNewParserSkipPixelData(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)
+		}
+	})
+	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))
+		}
+	})
+}
+
 // BenchmarkParse runs sanity benchmarks over the sample files in testdata.
 func BenchmarkParse(b *testing.B) {
 	files, err := ioutil.ReadDir("./testdata")

read.go

@@ -124,7 +124,7 @@ func (r *reader) readValue(t tag.Tag, vr string, vl uint32, isImplicit bool, d *
 	case tag.VRItem:
 		return r.readSequenceItem(t, vr, vl, d)
 	case tag.VRPixelData:
-		return r.readPixelData(t, vr, vl, d, fc)
+		return r.readPixelData(vl, d, fc)
 	case tag.VRFloat32List, tag.VRFloat64List:
 		return r.readFloat(t, vr, vl)
 	default:
@@ -185,20 +185,20 @@ func (r *reader) readHeader() ([]*Element, error) {
 	return metaElems, nil
 }
 
-func (r *reader) readPixelData(t tag.Tag, vr string, vl uint32, d *Dataset, fc chan<- *frame.Frame) (Value,
+func (r *reader) readPixelData(vl uint32, d *Dataset, fc chan<- *frame.Frame) (Value,
 	error) {
 	if vl == tag.VLUndefinedLength {
 		var image PixelDataInfo
 		image.IsEncapsulated = true
 		// The first Item in PixelData is the basic offset table. Skip this for now.
 		// TODO: use basic offset table
-		_, _, err := r.readRawItem()
+		_, _, err := r.readRawItem(true /*shouldSkip*/)
 		if err != nil {
 			return nil, err
 		}
 
 		for !r.rawReader.IsLimitExhausted() {
-			data, endOfItems, err := r.readRawItem()
+			data, endOfItems, err := r.readRawItem(r.opts.skipPixelData /*shouldSkip*/)
 			if err != nil {
 				break
 			}
@@ -220,9 +220,19 @@ func (r *reader) readPixelData(t tag.Tag, vr string, vl uint32, d *Dataset, fc c
 
 			image.Frames = append(image.Frames, f)
 		}
+		image.IntentionallySkipped = r.opts.skipPixelData
 		return &pixelDataValue{PixelDataInfo: image}, nil
 	}
 
+	if r.opts.skipPixelData {
+		// If we're here, it means the VL isn't undefined length, so we should
+		// be able to safely skip the native PixelData.
+		if err := r.rawReader.Skip(int64(vl)); err != nil {
+			return nil, err
+		}
+		return &pixelDataValue{PixelDataInfo{IntentionallySkipped: true}}, nil
+	}
+
 	// 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 {
@@ -719,7 +729,7 @@ func (r *reader) readElement(d *Dataset, fc chan<- *frame.Frame) (*Element, erro
 // Read an Item object as raw bytes, useful when parsing encapsulated PixelData.
 // This returns the read raw item, an indication if this is the end of the set
 // of items, and a possible errorawReader.
-func (r *reader) readRawItem() ([]byte, bool, error) {
+func (r *reader) readRawItem(shouldSkip bool) ([]byte, bool, error) {
 	t, err := r.readTag()
 	if err != nil {
 		return nil, true, err
@@ -752,13 +762,20 @@ func (r *reader) readRawItem() ([]byte, bool, error) {
 		return nil, true, fmt.Errorf("readRawItem: expected VR=NA, got VR=%s", vr)
 	}
 
-	data := make([]byte, vl)
-	_, err = io.ReadFull(r.rawReader, data)
-	if err != nil {
-		log.Println(err)
-		return nil, false, err
+	if shouldSkip {
+		if err := r.rawReader.Skip(int64(vl)); err != nil {
+			return nil, false, err
+		}
+	} else {
+		data := make([]byte, vl)
+		_, err = io.ReadFull(r.rawReader, data)
+		if err != nil {
+			log.Println(err)
+			return nil, false, err
+		}
+		return data, false, nil
 	}
-	return data, false, nil
+	return nil, false, nil
 }
 
 // moreToRead returns true if there is more to read from the underlying dicom.

read_test.go

@@ -502,6 +502,7 @@ func TestReadNativeFrames(t *testing.T) {
 	}
 
 	for _, tc := range cases {
+		tc := tc
 		t.Run(tc.Name, func(t *testing.T) {
 			dcmdata := bytes.Buffer{}
 			var expectedBytes int
@@ -535,6 +536,7 @@ func TestReadNativeFrames(t *testing.T) {
 				rawReader: dicomio.NewReader(bufio.NewReader(&dcmdata), binary.LittleEndian, int64(dcmdata.Len())),
 				opts:      tc.parseOptSet,
 			}
+
 			pixelData, bytesRead, err := r.readNativeFrames(&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)
@@ -550,6 +552,67 @@ func TestReadNativeFrames(t *testing.T) {
 	}
 }
 
+func TestReadPixelData_SkipPixelData(t *testing.T) {
+	cases := []struct {
+		name string
+		vl   uint32
+		data []byte
+	}{
+		{
+			name: "NativePixelData",
+			vl:   6,
+			data: []byte{1, 2, 3, 4, 5, 6},
+		},
+		{
+			name: "EncapsulatedPixelData",
+			vl:   tag.VLUndefinedLength,
+			data: makeEncapsulatedSequence(t),
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			opts := parseOptSet{skipPixelData: true}
+			dcmdata := bytes.NewBuffer(tc.data)
+
+			r := &reader{
+				rawReader: dicomio.NewReader(bufio.NewReader(dcmdata), binary.LittleEndian, int64(dcmdata.Len())),
+				opts:      opts,
+			}
+			val, err := r.readPixelData(tc.vl, &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.IntentionallySkipped {
+				t.Errorf("Expected PixelDataInfo to have IntentionallySkipped=true")
+			}
+		})
+	}
+}
+
+func makeEncapsulatedSequence(t *testing.T) []byte {
+	t.Helper()
+	buf := &bytes.Buffer{}
+	w := dicomio.NewWriter(buf, binary.LittleEndian, true)
+
+	writePixelData(w, tag.PixelData, &pixelDataValue{PixelDataInfo{IsEncapsulated: true, Frames: []frame.Frame{
+		{
+			Encapsulated: true,
+			EncapsulatedData: frame.EncapsulatedFrame{
+				Data: []byte{1, 2, 3, 4},
+			},
+		},
+	}}}, "", tag.VLUndefinedLength)
+
+	return buf.Bytes()
+
+}
+
 func TestReadNativeFrames_OneBitAllocated(t *testing.T) {
 	cases := []struct {
 		Name              string