Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add WriteOption to override missing transfer syntax in dataset #332

Merged
merged 3 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,10 @@ type headerData struct {
// 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, err := NewWriter(&buff)
if err != nil {
return []byte{}, err
}
dcmWriter.SetTransferSyntax(binary.LittleEndian, true)

for _, e := range elements {
Expand Down
83 changes: 63 additions & 20 deletions write.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,17 @@ type Writer struct {
}

// NewWriter returns a new Writer, that points to the provided io.Writer.
func NewWriter(out io.Writer, opts ...WriteOption) *Writer {
func NewWriter(out io.Writer, opts ...WriteOption) (*Writer, error) {
optSet := toWriteOptSet(opts...)
if err := optSet.validate(); err != nil {
return nil, err
}
w := dicomio.NewWriter(out, nil, false)

return &Writer{
writer: w,
optSet: optSet,
}
}, nil
}

// SetTransferSyntax sets the transfer syntax for the underlying dicomio.Writer.
Expand All @@ -66,16 +69,20 @@ func (w *Writer) writeDataset(ds Dataset) error {
return err
}

endian, implicit, err := ds.transferSyntax()
if (err != nil && err != ErrorElementNotFound) || (err == ErrorElementNotFound && !w.optSet.defaultMissingTransferSyntax) {
bo, implicit, err := ds.transferSyntax()
if errors.Is(err, ErrorElementNotFound) && w.optSet.defaultMissingTransferSyntax {
bo = binary.LittleEndian
implicit = true
} else if errors.Is(err, ErrorElementNotFound) && w.optSet.overrideMissingTransferSyntaxUID != "" {
bo, implicit, err = uid.ParseTransferSyntaxUID(w.optSet.overrideMissingTransferSyntaxUID)
if err != nil {
return err
}
} else if err != nil {
return err
}

if err == ErrorElementNotFound && w.optSet.defaultMissingTransferSyntax {
w.writer.SetTransferSyntax(binary.LittleEndian, true)
} else {
w.writer.SetTransferSyntax(endian, implicit)
}
w.writer.SetTransferSyntax(bo, implicit)

for _, elem := range ds.Elements {
if elem.Tag.Group != tag.MetadataGroup {
Expand All @@ -97,7 +104,10 @@ func (w *Writer) WriteElement(e *Element) error {
// 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...)
w, err := NewWriter(out, opts...)
if err != nil {
return err
}
return w.writeDataset(ds)
}

Expand All @@ -124,17 +134,43 @@ func SkipValueTypeVerification() WriteOption {
// transferSyntax should not raise an error, and instead the default
// LittleEndian Implicit transfer syntax should be used and written out as a
// Metadata element in the Dataset.
// TODO(suyashkumar): consider deprecating in favor of
// OverrideMissingTransferSyntax.
func DefaultMissingTransferSyntax() WriteOption {
return func(set *writeOptSet) {
set.defaultMissingTransferSyntax = true
}
}

// OverrideMissingTransferSyntax indicates that if the Dataset to be written
// does _not_ have a transfer syntax UID in its metadata, the Dataset should
// be written out with the provided transfer syntax UID if possible. A
// transfer syntax uid element with the specified transfer syntax will be
// written to the metadata as well.
//
// If the Writer is unable to recognize or write the dataset using the provided
// transferSyntaxUID, an error will be returned at initialization time.
func OverrideMissingTransferSyntax(transferSyntaxUID string) WriteOption {
return func(set *writeOptSet) {
set.overrideMissingTransferSyntaxUID = transferSyntaxUID
}
}

// writeOptSet represents the flattened option set after all WriteOptions have been applied.
type writeOptSet struct {
skipVRVerification bool
skipValueTypeVerification bool
defaultMissingTransferSyntax bool
skipVRVerification bool
skipValueTypeVerification bool
defaultMissingTransferSyntax bool
overrideMissingTransferSyntaxUID string
}

func (w *writeOptSet) validate() error {
if w.overrideMissingTransferSyntaxUID != "" {
if _, _, err := uid.ParseTransferSyntaxUID(w.overrideMissingTransferSyntaxUID); err != nil {
return fmt.Errorf("unable to parse OverrideMissingTransferSyntax transfer syntax uid %v due to: %s", w.overrideMissingTransferSyntaxUID, err)
}
}
return nil
}

func toWriteOptSet(opts ...WriteOption) *writeOptSet {
Expand All @@ -155,26 +191,33 @@ func writeFileHeader(w *dicomio.Writer, ds *Dataset, metaElems []*Element, opts
tagsUsed[tag.FileMetaInformationGroupLength] = true

err := writeMetaElem(subWriter, tag.FileMetaInformationVersion, ds, &tagsUsed, opts)
if err != nil && err != ErrorElementNotFound {
if err != nil && !errors.Is(err, ErrorElementNotFound) {
return err
}
err = writeMetaElem(subWriter, tag.MediaStorageSOPClassUID, ds, &tagsUsed, opts)
if err != nil && err != ErrorElementNotFound {
if err != nil && !errors.Is(err, ErrorElementNotFound) {
return err
}
err = writeMetaElem(subWriter, tag.MediaStorageSOPInstanceUID, ds, &tagsUsed, opts)
if err != nil && err != ErrorElementNotFound {
if err != nil && !errors.Is(err, ErrorElementNotFound) {
return err
}

err = writeMetaElem(subWriter, tag.TransferSyntaxUID, ds, &tagsUsed, opts)
if err != nil && err != ErrorElementNotFound || err == ErrorElementNotFound && !opts.defaultMissingTransferSyntax {
return err
}
if err == ErrorElementNotFound && opts.defaultMissingTransferSyntax {

if errors.Is(err, ErrorElementNotFound) && opts.defaultMissingTransferSyntax {
// Write the default transfer syntax
if err = writeElement(subWriter, mustNewElement(tag.TransferSyntaxUID, []string{uid.ImplicitVRLittleEndian}), opts); err != nil {
return err
}
} else if errors.Is(err, ErrorElementNotFound) && opts.overrideMissingTransferSyntaxUID != "" {
// Write the override transfer syntax
if err = writeElement(subWriter, mustNewElement(tag.TransferSyntaxUID, []string{opts.overrideMissingTransferSyntaxUID}), opts); err != nil {
return err
}
} else if err != nil {
// Return the error if none of the above conditions/overrides apply.
return err
}

for _, elem := range metaElems {
Expand Down
66 changes: 65 additions & 1 deletion write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,10 @@ func TestWriteElement(t *testing.T) {
}}

buf := bytes.Buffer{}
w := NewWriter(&buf)
w, err := NewWriter(&buf)
if err != nil {
t.Fatalf("NewWriter() returned unexpected error: %v", err)
}
w.SetTransferSyntax(binary.LittleEndian, true)

for _, e := range writeDS.Elements {
Expand All @@ -892,3 +895,64 @@ func TestWriteElement(t *testing.T) {
}
}
}

func TestWrite_OverrideMissingTransferSyntax(t *testing.T) {
dsWithMissingTS := 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.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}),
}}

cases := []struct {
name string
overrideTransferSyntax string
}{
{
name: "Little Endian Implicit",
overrideTransferSyntax: "1.2.840.10008.1.2",
},
{
name: "Little Endian Explicit",
overrideTransferSyntax: "1.2.840.10008.1.2.1",
},
{
name: "Big Endian Explicit",
overrideTransferSyntax: "1.2.840.10008.1.2.2",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
// Write out dicom with OverrideMissingTransferSyntax option.
writtenDICOM := &bytes.Buffer{}
if err := Write(writtenDICOM, dsWithMissingTS, OverrideMissingTransferSyntax(tc.overrideTransferSyntax)); err != nil {
t.Errorf("Write(OverrideMissingTransferSyntax(%v)) returned unexpected error: %v", tc.overrideTransferSyntax, err)
}

// Read dataset back in to ensure no roundtrip errors, and also
// check that the written out transfer syntax tag matches.
parsedDS, err := ParseUntilEOF(writtenDICOM, nil)
if err != nil {
t.Fatalf("ParseUntilEOF returned unexpected error when reading written dataset back in: %v", err)
}
tsElem, err := parsedDS.FindElementByTag(tag.TransferSyntaxUID)
if err != nil {
t.Fatalf("unable to find transfer syntax uid element in written dataset: %v", err)
}
tsVal, ok := tsElem.Value.GetValue().([]string)
if !ok {
t.Fatalf("TransferSyntaxUID tag was not of type []")
}
if len(tsVal) != 1 {
t.Errorf("TransferSyntaxUID []string contained more than one element unexpectedly: %v", tsVal)
}
if tsVal[0] != tc.overrideTransferSyntax {
t.Errorf("TransferSyntaxUID in written dicom did not contain the override transfer syntax value. got: %v, want: %v", tsVal[0], tc.overrideTransferSyntax)
}
})
}
}
Loading