Add option to ignore missing metadata group length field (#269)

This change adds the capability to read DICOMs that are missing the Metadata group length field.
suyashkumar · Jun 17, 2023 · d974e00 · d974e00
1 parent 5894fa3
commit d974e00
Show file tree

Hide file tree

Showing 4 changed files with 203 additions and 19 deletions.
diff --git a/parse.go b/parse.go
@@ -222,10 +222,11 @@ type ParseOption func(*parseOptSet)
 
 // parseOptSet represents the flattened option set after all ParseOptions have been applied.
 type parseOptSet struct {
-	skipMetadataReadOnNewParserInit bool
-	allowMismatchPixelDataLength    bool
-	skipPixelData                   bool
-	skipProcessingPixelDataValue    bool
+	skipMetadataReadOnNewParserInit    bool
+	allowMismatchPixelDataLength       bool
+	skipPixelData                      bool
+	skipProcessingPixelDataValue       bool
+	allowMissingMetaElementGroupLength bool
 }
 
 func toParseOptSet(opts ...ParseOption) parseOptSet {
@@ -243,6 +244,14 @@ func AllowMismatchPixelDataLength() ParseOption {
 	}
 }
 
+// AllowMissingMetaElementGroupLength allows parser to work around missing metaelement group length tag (0x0002,0x0000) by reading elements only
+// in group 2.
+func AllowMissingMetaElementGroupLength() ParseOption {
+	return func(set *parseOptSet) {
+		set.allowMissingMetaElementGroupLength = true
+	}
+}
+
 // SkipMetadataReadOnNewParserInit makes NewParser skip trying to parse metadata. This will make the Parser default to implicit little endian byte order.
 // Any metatata tags found in the dataset will still be available when parsing.
 func SkipMetadataReadOnNewParserInit() ParseOption {

diff --git a/parse_test.go b/parse_test.go
@@ -175,6 +175,14 @@ func TestParseFile_SkipProcessingPixelDataValue(t *testing.T) {
 			}
 		})
 	})
+	t.Run("WithAllowErrorMetaElementGroupLength", func(t *testing.T) {
+		runForEveryTestFile(t, func(t *testing.T, filename string) {
+			dataset, err := dicom.ParseFile(filename, nil, dicom.AllowMissingMetaElementGroupLength())
+			if err != nil {
+				t.Errorf("Unexpected error parsing dataset: %v", dataset)
+			}
+		})
+	})
 }
 
 // BenchmarkParse runs sanity benchmarks over the sample files in testdata.

diff --git a/read.go b/read.go
@@ -167,28 +167,60 @@ func (r *reader) readHeader() ([]*Element, error) {
 		return nil, err
 	}
 
+	metaElems := []*Element{maybeMetaLen} // TODO: maybe set capacity to a reasonable initial size
+	metaElementGroupLengthDefined := true
 	if maybeMetaLen.Tag != tag.FileMetaInformationGroupLength || maybeMetaLen.Value.ValueType() != Ints {
-		return nil, ErrorMetaElementGroupLength
+		// MetaInformationGroupLength is not present or of the wrong value type.
+		if !r.opts.allowMissingMetaElementGroupLength {
+			return nil, ErrorMetaElementGroupLength
+		}
+		metaElementGroupLengthDefined = false
 	}
 
-	metaLen := maybeMetaLen.Value.GetValue().([]int)[0]
-
-	metaElems := []*Element{maybeMetaLen} // TODO: maybe set capacity to a reasonable initial size
+	if metaElementGroupLengthDefined {
+		metaLen := maybeMetaLen.Value.GetValue().([]int)[0]
 
-	// Read the metadata elements
-	err = r.rawReader.PushLimit(int64(metaLen))
-	if err != nil {
-		return nil, err
-	}
-	defer r.rawReader.PopLimit()
-	for !r.rawReader.IsLimitExhausted() {
-		elem, err := r.readElement(nil, nil)
+		// Read the metadata elements
+		err = r.rawReader.PushLimit(int64(metaLen))
 		if err != nil {
-			// TODO: see if we can skip over malformed elements somehow
 			return nil, err
 		}
-		// log.Printf("Metadata Element: %s\n", elem)
-		metaElems = append(metaElems, elem)
+		defer r.rawReader.PopLimit()
+		for !r.rawReader.IsLimitExhausted() {
+			elem, err := r.readElement(nil, nil)
+			if err != nil {
+				// TODO: see if we can skip over malformed elements somehow
+				return nil, err
+			}
+			// log.Printf("Metadata Element: %s\n", elem)
+			metaElems = append(metaElems, elem)
+		}
+	} else {
+		// We cannot use the limit functionality
+		debug.Log("Proceeding without metadata group length")
+		for {
+			// Lets peek into the tag field until we get to end-of-header
+			group_bytes, err := r.rawReader.Peek(2)
+			if err != nil {
+				return nil, ErrorMetaElementGroupLength
+			}
+			var group uint16
+			buff := bytes.NewBuffer(group_bytes)
+			if err := binary.Read(buff, binary.LittleEndian, &group); err != nil {
+				return nil, err
+			}
+			debug.Logf("header-group: %v", group)
+			// Only read group 2 data
+			if group != 0x0002 {
+				break
+			}
+			elem, err := r.readElement(nil, nil)
+			if err != nil {
+				// TODO: see if we can skip over malformed elements somehow
+				return nil, err
+			}
+			metaElems = append(metaElems, elem)
+		}
 	}
 	return metaElems, nil
 }

diff --git a/read_test.go b/read_test.go
@@ -595,6 +595,141 @@ func TestReadPixelData_SkipPixelData(t *testing.T) {
 	}
 }
 
+// Used to encode the data from the generated headers.
+type headerData struct {
+	// The byte encoded header data.
+	HeaderBytes *bytes.Buffer
+	// The decoded elements conforming the header.
+	Elements []*Element
+}
+
+// Write a collection of elements and return them as an encoded buffer of bytes.
+func writeElements(elements []*Element) ([]byte, error) {
+	buff := bytes.Buffer{}
+	dcmWriter := NewWriter(&buff)
+	dcmWriter.SetTransferSyntax(binary.LittleEndian, true)
+
+	for _, e := range elements {
+		err := dcmWriter.WriteElement(e)
+		if err != nil {
+			return nil, err
+		}
+	}
+	data := buff.Bytes()
+	return data, nil
+}
+
+// Returns a fake DICOM group 2 header with the FileMetaInformationGroupLength tag missing (0x0002,0x0000).
+func headerWithNoFileMetaInformationGroupLength() (*headerData, error) {
+	headerData := new(headerData)
+
+	elements := []*Element{
+		mustNewElement(tag.MediaStorageSOPClassUID, []string{"SecondaryCapture"}),
+		mustNewElement(tag.MediaStorageSOPInstanceUID, []string{"1.3.6.1.4.1.35190.4.1.20210608.607733549593"}),
+		mustNewElement(tag.TransferSyntaxUID, []string{"=RLELossless"}),
+		mustNewElement(tag.ImplementationClassUID, []string{"1.6.6.1.4.1.9590.100.1.0.100.4.0"}),
+		mustNewElement(tag.SOPInstanceUID, []string{"1.3.6.1.4.1.35190.4.1.20210608.607733549593"}),
+	}
+	data, err := writeElements(elements)
+	if err != nil {
+		return nil, err
+	}
+
+	// Construct valid DICOM header preamble.
+	magicWord := []byte("DICM")
+	preamble := make([]byte, 128)
+	preamble = append(preamble, magicWord...)
+	headerBytes := append(preamble, data...)
+	headerData.HeaderBytes = bytes.NewBuffer(headerBytes)
+	headerData.Elements = elements[0 : len(elements)-1]
+	return headerData, nil
+}
+
+// Returns a fake DICOM group 2 header with a FileMetaInformationGroupLength tag (0x0002,0x0000).
+func headerWithFileMetaInformationGroupLength() (*headerData, error) {
+	headerData := new(headerData)
+
+	sopInstanceUidElement := mustNewElement(tag.SOPInstanceUID, []string{"1.3.6.1.4.1.35190.4.1.20210608.607733549593"})
+	elements := []*Element{
+		mustNewElement(tag.FileMetaInformationVersion, []byte{0x00, 0x01}),
+		mustNewElement(tag.MediaStorageSOPClassUID, []string{"1.2.276.0.7230010.3.1.0.1"}),
+		mustNewElement(tag.MediaStorageSOPInstanceUID, []string{"1.3.6.1.4.1.35190.4.1.20210608.607733549593"}),
+		mustNewElement(tag.TransferSyntaxUID, []string{"=RLELossless"}),
+		mustNewElement(tag.ImplementationClassUID, []string{"1.2.276.0.7230010.3.0.3.6.7"}),
+		mustNewElement(tag.ImplementationVersionName, []string{"OFFIS_DCMTK_367"}),
+	}
+	dataHeader, err := writeElements(elements)
+	if err != nil {
+		return nil, err
+	}
+	fileMetaInfoElement := mustNewElement(tag.FileMetaInformationGroupLength, []int{len(dataHeader)})
+	dataFileMetaInfo, err := writeElements([]*Element{fileMetaInfoElement})
+	if err != nil {
+		return nil, err
+	}
+	dataSopInstanceUid, err := writeElements([]*Element{sopInstanceUidElement})
+	if err != nil {
+		return nil, err
+	}
+	data := append(dataFileMetaInfo, dataHeader...)
+	data = append(data, dataSopInstanceUid...)
+
+	// Construct valid DICOM header preamble.
+	magicWord := []byte("DICM")
+	preamble := make([]byte, 128)
+	preamble = append(preamble, magicWord...)
+	headerBytes := append(preamble, data...)
+	headerData.HeaderBytes = bytes.NewBuffer(headerBytes)
+	headerData.Elements = append([]*Element{fileMetaInfoElement}, elements...)
+	return headerData, nil
+}
+
+func TestReadHeader_TryAllowErrorMetaElementGroupLength(t *testing.T) {
+	opts := parseOptSet{allowMissingMetaElementGroupLength: true}
+
+	t.Run("NoFileMetaInformationGroupLength", func(t *testing.T) {
+		dcmheaderNoInfoGrpLen, err := headerWithNoFileMetaInformationGroupLength()
+		if err != nil {
+			t.Fatalf("unsuccesful generation of fake header data")
+		} else {
+			r := &reader{
+				rawReader: dicomio.NewReader(bufio.NewReader(dcmheaderNoInfoGrpLen.HeaderBytes), binary.LittleEndian, int64(dcmheaderNoInfoGrpLen.HeaderBytes.Len())),
+				opts:      opts,
+			}
+			r.rawReader.SetTransferSyntax(binary.LittleEndian, true)
+			wantElements, err := r.readHeader()
+			if err != nil {
+				t.Errorf("unsuccessful readHeader when parse option %v is turned on and header has no MetaElementGroupLength tag", opts.allowMissingMetaElementGroupLength)
+			}
+			// Ensure dataset read from readHeader and the test header are the same except for the ValueLength field.
+			if diff := cmp.Diff(wantElements, dcmheaderNoInfoGrpLen.Elements, cmp.AllowUnexported(allValues...), cmpopts.IgnoreFields(Element{}, "ValueLength")); diff != "" {
+				t.Errorf("Elements parsed from test header do not match: %v", diff)
+			}
+		}
+	})
+
+	t.Run("WithFileMetaInformationGroupLength", func(t *testing.T) {
+		dcmHeaderInfoGrpLen, err := headerWithFileMetaInformationGroupLength()
+		if err != nil {
+			t.Fatalf("unsuccesful generation of fake header data with FileMetaInformationGroupLength")
+		} else {
+			r := &reader{
+				rawReader: dicomio.NewReader(bufio.NewReader(dcmHeaderInfoGrpLen.HeaderBytes), binary.LittleEndian, int64(dcmHeaderInfoGrpLen.HeaderBytes.Len())),
+				opts:      opts,
+			}
+			r.rawReader.SetTransferSyntax(binary.LittleEndian, true)
+			wantElements, err := r.readHeader()
+			if err != nil {
+				t.Errorf("unsuccesful readHeader when parse option %v is turned on and header has no MetaElementGroupLength tag", opts.allowMissingMetaElementGroupLength)
+			}
+			// Ensure dataset read from readHeader and the test header are the same except for the ValueLength field.
+			if diff := cmp.Diff(wantElements, dcmHeaderInfoGrpLen.Elements, cmp.AllowUnexported(allValues...), cmpopts.IgnoreFields(Element{}, "ValueLength")); diff != "" {
+				t.Errorf("Elements parsed from test header do not match: %v", diff)
+			}
+		}
+	})
+}
+
 func TestReadPixelData_TrySkipProcessingPixelDataValue(t *testing.T) {
 	opts := parseOptSet{skipProcessingPixelDataValue: true}
 	valueBytes := []byte{1, 2, 3, 4, 5, 6}