Introduce element-by-element Writer API, add SkipMetadataReadOnNewPar…

…serInit Parser Option, add ParseOptions to Parser API. (#208)

This change introduces an element-by-element Writer API, adds support for Parse Options (similar in structure to the WriteOption) and adds a SkipMetadataReadOnNewParserInit option. 

Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>

parse.go

@@ -25,7 +25,6 @@ import (
 	"encoding/binary"
 	"errors"
 	"io"
-	"log"
 	"os"
 
 	"github.com/suyashkumar/dicom/pkg/charset"
@@ -109,7 +108,8 @@ type Parser struct {
 //
 // frameChannel is an optional channel (can be nil) upon which DICOM image frames will be sent as they are parsed (if
 // provided).
-func NewParser(in io.Reader, bytesToRead int64, frameChannel chan *frame.Frame) (*Parser, error) {
+func NewParser(in io.Reader, bytesToRead int64, frameChannel chan *frame.Frame, opts ...ParseOption) (*Parser, error) {
+	optSet := toParseOptSet(opts...)
 	reader, err := dicomio.NewReader(bufio.NewReader(in), binary.LittleEndian, bytesToRead)
 	if err != nil {
 		return nil, err
@@ -120,12 +120,16 @@ func NewParser(in io.Reader, bytesToRead int64, frameChannel chan *frame.Frame)
 		frameChannel: frameChannel,
 	}
 
-	debug.Log("NewParser: readHeader")
-	elems, err := p.readHeader()
-	if err != nil {
-		return nil, err
+	elems := []*Element{}
+
+	if !optSet.skipMetadataReadOnNewParserInit {
+		debug.Log("NewParser: readHeader")
+		elems, err = p.readHeader()
+		if err != nil {
+			return nil, err
+		}
+		debug.Log("NewParser: readHeader complete")
 	}
-	debug.Log("NewParser: readHeader complete")
 
 	p.dataset = Dataset{Elements: elems}
 	// TODO(suyashkumar): avoid storing the metadata pointers twice (though not that expensive)
@@ -138,13 +142,13 @@ func NewParser(in io.Reader, bytesToRead int64, frameChannel chan *frame.Frame)
 
 	ts, err := p.dataset.FindElementByTag(tag.TransferSyntaxUID)
 	if err != nil {
-		log.Println("WARN: could not find transfer syntax uid in metadata, proceeding with little endian implicit")
+		debug.Log("WARN: could not find transfer syntax uid in metadata, proceeding with little endian implicit")
 	} else {
 		bo, implicit, err = uid.ParseTransferSyntaxUID(MustGetStrings(ts.Value)[0])
 		if err != nil {
 			// TODO(suyashkumar): should we attempt to parse with LittleEndian
 			// Implicit here?
-			log.Println("WARN: could not parse transfer syntax uid in metadata")
+			debug.Log("WARN: could not parse transfer syntax uid in metadata")
 		}
 	}
 	p.reader.SetTransferSyntax(bo, implicit)
@@ -191,6 +195,11 @@ func (p *Parser) GetMetadata() Dataset {
 	return p.metadata
 }
 
+// SetTransferSyntax sets the transfer syntax for the underlying dicomio.Reader.
+func (p *Parser) SetTransferSyntax(bo binary.ByteOrder, implicit bool) {
+	p.reader.SetTransferSyntax(bo, implicit)
+}
+
 // readHeader reads the DICOM magic header and group two metadata elements.
 func (p *Parser) readHeader() ([]*Element, error) {
 	// Check to see if magic word is at byte offset 128. If not, this is a
@@ -242,3 +251,27 @@ func (p *Parser) readHeader() ([]*Element, error) {
 	}
 	return metaElems, nil
 }
+
+// ParseOption represents an option that can be passed to NewParser.
+type ParseOption func(*parseOptSet)
+
+// parseOptSet represents the flattened option set after all ParseOptions have been applied.
+type parseOptSet struct {
+	skipMetadataReadOnNewParserInit bool
+}
+
+func toParseOptSet(opts ...ParseOption) *parseOptSet {
+	optSet := &parseOptSet{}
+	for _, opt := range opts {
+		opt(optSet)
+	}
+	return optSet
+}
+
+// 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 {
+	return func(set *parseOptSet) {
+		set.skipMetadataReadOnNewParserInit = true
+	}
+}

parse_test.go

@@ -45,6 +45,30 @@ func TestParse(t *testing.T) {
 	}
 }
 
+// TestNewParserSkipMetadataReadOnNewParserInit tests that NewParser with the SkipMetadataReadOnNewParserInit option
+// parses the specified dataset but not its header metadata.
+func TestNewParserSkipMetadataReadOnNewParserInit(t *testing.T) {
+	fStat, err := os.Stat("./testdata/1.dcm")
+	if err != nil {
+		t.Fatalf("Unable to stat %s. Error: %v", fStat.Name(), err)
+	}
+
+	f, err := os.Open("./testdata/1.dcm")
+	if err != nil {
+		t.Fatalf("Unable to open %s. Error: %v", f.Name(), err)
+	}
+
+	p, err := dicom.NewParser(f, fStat.Size(), nil, dicom.SkipMetadataReadOnNewParserInit())
+	if err != nil {
+		t.Fatalf("dicom.Parse(%s) unexpected error: %v", f.Name(), err)
+	}
+
+	metadata := p.GetMetadata()
+	if len(metadata.Elements) > 0 {
+		t.Fatalf("Found %d metadata elements despite SkipMetadataReadOnNewParserInit()", len(metadata.Elements))
+	}
+}
+
 // BenchmarkParse runs sanity benchmarks over the sample files in testdata.
 func BenchmarkParse(b *testing.B) {
 	files, err := ioutil.ReadDir("./testdata")

write.go

@@ -28,39 +28,56 @@ var (
 	ErrorUnsupportedBitsPerSample = errors.New("unsupported BitsPerSample value")
 )
 
-// TODO(suyashkumar): consider adding an element-by-element write API.
+// Writer is a struct that allows element-by element writing to a DICOM writer.
+type Writer struct {
+	writer dicomio.Writer
+	optSet *writeOptSet
+}
 
-// Write will write the input DICOM dataset to the provided io.Writer as a complete DICOM (including any header
-// information if available).
-func Write(out io.Writer, ds Dataset, opts ...WriteOption) error {
-	optSet := toOptSet(opts...)
+// NewWriter returns a new Writer, that points to the provided io.Writer.
+func NewWriter(out io.Writer, opts ...WriteOption) *Writer {
+	optSet := toWriteOptSet(opts...)
 	w := dicomio.NewWriter(out, nil, false)
+
+	return &Writer{
+		writer: w,
+		optSet: optSet,
+	}
+}
+
+// SetTransferSyntax sets the transfer syntax for the underlying dicomio.Writer.
+func (w *Writer) SetTransferSyntax(bo binary.ByteOrder, implicit bool) {
+	w.writer.SetTransferSyntax(bo, implicit)
+}
+
+// writeDataset writes the provided DICOM dataset to the Writer, including headers if available.
+func (w *Writer) writeDataset(ds Dataset) error {
 	var metaElems []*Element
 	for _, elem := range ds.Elements {
 		if elem.Tag.Group == tag.MetadataGroup {
 			metaElems = append(metaElems, elem)
 		}
 	}
 
-	err := writeFileHeader(w, &ds, metaElems, *optSet)
+	err := writeFileHeader(w.writer, &ds, metaElems, *w.optSet)
 	if err != nil {
 		return err
 	}
 
 	endian, implicit, err := ds.transferSyntax()
-	if (err != nil && err != ErrorElementNotFound) || (err == ErrorElementNotFound && !optSet.defaultMissingTransferSyntax) {
+	if (err != nil && err != ErrorElementNotFound) || (err == ErrorElementNotFound && !w.optSet.defaultMissingTransferSyntax) {
 		return err
 	}
 
-	if err == ErrorElementNotFound && optSet.defaultMissingTransferSyntax {
-		w.SetTransferSyntax(binary.LittleEndian, true)
+	if err == ErrorElementNotFound && w.optSet.defaultMissingTransferSyntax {
+		w.writer.SetTransferSyntax(binary.LittleEndian, true)
 	} else {
-		w.SetTransferSyntax(endian, implicit)
+		w.writer.SetTransferSyntax(endian, implicit)
 	}
 
 	for _, elem := range ds.Elements {
 		if elem.Tag.Group != tag.MetadataGroup {
-			err = writeElement(w, elem, *optSet)
+			err = writeElement(w.writer, elem, *w.optSet)
 			if err != nil {
 				return err
 			}
@@ -70,6 +87,18 @@ func Write(out io.Writer, ds Dataset, opts ...WriteOption) error {
 	return nil
 }
 
+// WriteElement writes a single DICOM element to a Writer.
+func (w *Writer) WriteElement(e *Element) error {
+	return writeElement(w.writer, e, *w.optSet)
+}
+
+// Write will write the input DICOM dataset to the provided io.Writer as a complete DICOM (including any header
+// information if available).
+func Write(out io.Writer, ds Dataset, opts ...WriteOption) error {
+	w := NewWriter(out, opts...)
+	return w.writeDataset(ds)
+}
+
 // WriteOption represents an option that can be passed to WriteDataset. Later options will override previous options if
 // applicable.
 type WriteOption func(*writeOptSet)
@@ -106,7 +135,7 @@ type writeOptSet struct {
 	defaultMissingTransferSyntax bool
 }
 
-func toOptSet(opts ...WriteOption) *writeOptSet {
+func toWriteOptSet(opts ...WriteOption) *writeOptSet {
 	optSet := &writeOptSet{}
 	for _, opt := range opts {
 		opt(optSet)
@@ -187,7 +216,7 @@ func writeElement(w dicomio.Writer, elem *Element, opts writeOptSet) error {
 	if err != nil {
 		return err
 	}
-	
+
 	if !opts.skipValueTypeVerification && elem.Value != nil {
 		err := verifyValueType(elem.Tag, elem.Value, vr)
 		if err != nil {

write_test.go

@@ -3,11 +3,12 @@ package dicom
 import (
 	"bytes"
 	"encoding/binary"
-	"github.com/suyashkumar/dicom/pkg/vrraw"
 	"io/ioutil"
 	"os"
 	"testing"
 
+	"github.com/suyashkumar/dicom/pkg/vrraw"
+
 	"github.com/suyashkumar/dicom/pkg/frame"
 
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -632,16 +633,16 @@ func TestWriteOtherWord(t *testing.T) {
 		expectedErr  error
 	}{
 		{
-			name:  "OtherWord",
-			value: []byte{0x1, 0x2, 0x3, 0x4},
-			vr:    "OW",
+			name:         "OtherWord",
+			value:        []byte{0x1, 0x2, 0x3, 0x4},
+			vr:           "OW",
 			expectedData: []byte{0x1, 0x2, 0x3, 0x4},
 			expectedErr:  nil,
 		},
 		{
-			name:  "OtherBytes",
-			value: []byte{0x1, 0x2, 0x3, 0x4},
-			vr:    "OB",
+			name:         "OtherBytes",
+			value:        []byte{0x1, 0x2, 0x3, 0x4},
+			vr:           "OB",
 			expectedData: []byte{0x1, 0x2, 0x3, 0x4},
 			expectedErr:  nil,
 		},
@@ -667,3 +668,44 @@ func setUndefinedLength(e *Element) *Element {
 	e.ValueLength = tag.VLUndefinedLength
 	return e
 }
+
+// TestWriteElement tests a dataset written using writer.WriteElement can be parsed into an identical dataset using NewParser.
+func TestWriteElement(t *testing.T) {
+	writeDS := 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.PatientName, []string{"Bob", "Jones"}),
+		mustNewElement(tag.Rows, []int{128}),
+		mustNewElement(tag.FloatingPointValue, []float64{128.10}),
+		mustNewElement(tag.DimensionIndexPointer, []int{32, 36950}),
+		mustNewElement(tag.RedPaletteColorLookupTableData, []byte{0x1, 0x2, 0x3, 0x4}),
+	}}
+
+	buf := bytes.Buffer{}
+	w := NewWriter(&buf)
+	w.SetTransferSyntax(binary.LittleEndian, true)
+
+	for _, e := range writeDS.Elements {
+		err := w.WriteElement(e)
+		if err != nil {
+			t.Errorf("error in writing element %s: %s", e.String(), err.Error())
+		}
+	}
+
+	p, err := NewParser(&buf, int64(buf.Len()), nil, SkipMetadataReadOnNewParserInit())
+	if err != nil {
+		t.Fatalf("failed to create parser: %v", err)
+	}
+
+	for _, writtenElem := range writeDS.Elements {
+		readElem, err := p.Next()
+		if err != nil {
+			t.Errorf("error in reading element %s: %s", readElem.String(), err.Error())
+		}
+
+		if diff := cmp.Diff(writtenElem, readElem, cmp.AllowUnexported(allValues...), cmpopts.IgnoreFields(Element{}, "ValueLength")); diff != "" {
+			t.Errorf("unexpected diff in element: %s", diff)
+		}
+	}
+}