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.

Sign up for GitHub

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

Jump to bottom

Add ability to keep track of only required Context Elements #267

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add ability to keep track of only required Context Elements #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64bfc5a
Add basic Parser.Next based benchmark
suyashkumar Apr 2, 2023
386a707
rename readElementWithContext
suyashkumar May 27, 2024
d04b27e
Add metadata elements to required context
suyashkumar May 27, 2024
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
11 changes: 6 additions & 5 deletions parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func parseInternal(in io.Reader, bytesToRead int64, frameChan chan *frame.Frame,
}

for !p.reader.rawReader.IsLimitExhausted() {
_, err := p.Next()
elem, err := p.Next()
if err != nil {
if errors.Is(err, io.EOF) {
// exiting on EOF
Expand All @@ -83,6 +83,7 @@ func parseInternal(in io.Reader, bytesToRead int64, frameChan chan *frame.Frame,
// exit on error
return p.dataset, err
}
p.dataset.Elements = append(p.dataset.Elements, elem)
}

// Close the frameChannel if needed
Expand Down Expand Up @@ -130,8 +131,9 @@ func NewParser(in io.Reader, bytesToRead int64, frameChannel chan *frame.Frame,
optSet := toParseOptSet(opts...)
p := Parser{
reader: &reader{
rawReader: dicomio.NewReader(bufio.NewReader(in), binary.LittleEndian, bytesToRead),
opts: optSet,
rawReader: dicomio.NewReader(bufio.NewReader(in), binary.LittleEndian, bytesToRead),
opts: optSet,
datasetCtx: &Dataset{},
},
frameChannel: frameChannel,
}
Expand Down Expand Up @@ -189,7 +191,7 @@ func (p *Parser) Next() (*Element, error) {
}
return nil, ErrorEndOfDICOM
}
elem, err := p.reader.readElement(&p.dataset, p.frameChannel)
elem, err := p.reader.ReadElement(p.frameChannel)
if err != nil {
// TODO: tolerate some kinds of errors and continue parsing
return nil, err
Expand All @@ -208,7 +210,6 @@ func (p *Parser) Next() (*Element, error) {
p.reader.rawReader.SetCodingSystem(cs)
}

p.dataset.Elements = append(p.dataset.Elements, elem)
return elem, nil

}
Expand Down
73 changes: 56 additions & 17 deletions read.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ var (
type reader struct {
rawReader *dicomio.Reader
opts parseOptSet
// datasetCtx is the top-level context Dataset holding only elements that
// may be needed to parse future elements (e.g. like tag.Rows). See
// requiredContextElements for elements that will be automatically
// included in this context.
// TODO(suyashkumar): should this be initialized to the Metadata elements?
datasetCtx *Dataset
}

func (r *reader) readTag() (*tag.Tag, error) {
Expand Down Expand Up @@ -172,7 +178,7 @@ func (r *reader) readHeader() ([]*Element, error) {

// Must read metadata as LittleEndian explicit VR
// Read the length of the metadata elements: (0002,0000) MetaElementGroupLength
maybeMetaLen, err := r.readElement(nil, nil)
maybeMetaLen, err := r.readElementWithContext(r.datasetCtx, nil)
if err != nil {
return nil, err
}
Expand All @@ -197,7 +203,7 @@ func (r *reader) readHeader() ([]*Element, error) {
}
defer r.rawReader.PopLimit()
for !r.rawReader.IsLimitExhausted() {
elem, err := r.readElement(nil, nil)
elem, err := r.readElementWithContext(r.datasetCtx, nil)
if err != nil {
// TODO: see if we can skip over malformed elements somehow
return nil, err
Expand All @@ -224,7 +230,7 @@ func (r *reader) readHeader() ([]*Element, error) {
if group != 0x0002 {
break
}
elem, err := r.readElement(nil, nil)
elem, err := r.readElementWithContext(r.datasetCtx, nil)
if err != nil {
// TODO: see if we can skip over malformed elements somehow
return nil, err
Expand Down Expand Up @@ -516,7 +522,7 @@ func (r *reader) readSequence(t tag.Tag, vr string, vl uint32, d *Dataset) (Valu
seqElements := &Dataset{}
if vl == tag.VLUndefinedLength {
for {
subElement, err := r.readElement(seqElements, nil)
subElement, err := r.readElementWithContext(seqElements, nil)
if err != nil {
// Stop reading due to error
log.Println("error reading subitem, ", err)
Expand All @@ -543,7 +549,7 @@ func (r *reader) readSequence(t tag.Tag, vr string, vl uint32, d *Dataset) (Valu
return nil, err
}
for !r.rawReader.IsLimitExhausted() {
subElement, err := r.readElement(seqElements, nil)
subElement, err := r.readElementWithContext(seqElements, nil)
if err != nil {
// TODO: option to ignore errors parsing subelements?
return nil, err
Expand All @@ -569,7 +575,7 @@ func (r *reader) readSequenceItem(t tag.Tag, vr string, vl uint32, d *Dataset) (

if vl == tag.VLUndefinedLength {
for {
subElem, err := r.readElement(&seqElements, nil)
subElem, err := r.readElementWithContext(&seqElements, nil)
if err != nil {
return nil, err
}
Expand All @@ -587,7 +593,7 @@ func (r *reader) readSequenceItem(t tag.Tag, vr string, vl uint32, d *Dataset) (
}

for !r.rawReader.IsLimitExhausted() {
subElem, err := r.readElement(&seqElements, nil)
subElem, err := r.readElementWithContext(&seqElements, nil)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -743,17 +749,24 @@ func (r *reader) readInt(t tag.Tag, vr string, vl uint32) (Value, error) {
return retVal, err
}

// readElement reads the next element. If the next element is a sequence element,
// it may result in a collection of Elements. It takes a pointer to the Dataset of
// elements read so far, since previously read elements may be needed to parse
// ReadElement reads the next element. This is the top level function that should
// be called to read elements. If the next element is a sequence element,
// it may result in reading a collection of Elements.
func (r *reader) ReadElement(fc chan<- *frame.Frame) (*Element, error) {
return r.readElementWithContext(r.datasetCtx, fc)
}

// readElementWithContext reads the next element. If the next element is a sequence element,
// it may result in reading a collection of Elements. It takes a pointer to the Dataset of
// context elements, since previously read elements may be needed to parse
// certain Elements (like native PixelData). If the Dataset is nil, it is
// treated as an empty Dataset.
func (r *reader) readElement(d *Dataset, fc chan<- *frame.Frame) (*Element, error) {
func (r *reader) readElementWithContext(datasetCtx *Dataset, fc chan<- *frame.Frame) (*Element, error) {
t, err := r.readTag()
if err != nil {
return nil, err
}
debug.Logf("readElement: tag: %s", t.String())
debug.Logf("readElementWithContext: tag: %s", t.String())

readImplicit := r.rawReader.IsImplicit()
if *t == tag.Item {
Expand All @@ -765,22 +778,23 @@ func (r *reader) readElement(d *Dataset, fc chan<- *frame.Frame) (*Element, erro
if err != nil {
return nil, err
}
debug.Logf("readElement: vr: %s", vr)
debug.Logf("readElementWithContext: vr: %s", vr)

vl, err := r.readVL(readImplicit, *t, vr)
if err != nil {
return nil, err
}
debug.Logf("readElement: vl: %d", vl)
debug.Logf("readElementWithContext: vl: %datasetCtx", vl)

val, err := r.readValue(*t, vr, vl, readImplicit, d, fc)
val, err := r.readValue(*t, vr, vl, readImplicit, datasetCtx, fc)
if err != nil {
log.Println("error reading value ", err)
return nil, err
}

return &Element{Tag: *t, ValueRepresentation: tag.GetVRKind(*t, vr), RawValueRepresentation: vr, ValueLength: vl, Value: val}, nil

elem := &Element{Tag: *t, ValueRepresentation: tag.GetVRKind(*t, vr), RawValueRepresentation: vr, ValueLength: vl, Value: val}
addToContextIfNeeded(datasetCtx, elem)
return elem, nil
}

// Read an Item object as raw bytes, useful when parsing encapsulated PixelData.
Expand Down Expand Up @@ -839,3 +853,28 @@ func (r *reader) readRawItem(shouldSkip bool) ([]byte, bool, error) {
func (r *reader) moreToRead() bool {
return !r.rawReader.IsLimitExhausted()
}

// requiredContextElements holds the set of DICOM Element Tags that should
// be included in the context Dataset. These are elements that may be needed to
// read downstream elements in the future.
// addToContextIfNeeded also always adds all Metadata group 2 elements.
var requiredContextElements = tag.Tags{
&tag.Rows,
&tag.Columns,
&tag.NumberOfFrames,
&tag.BitsAllocated,
&tag.SamplesPerPixel,
}

// addToContextIfNeeded adds the element to the provided dataset context if
// the tag is in requiredContextElements or if the tag has a group of 2
// (meaning it is a metadata element). In the future we can probably filter this
// down further.
func addToContextIfNeeded(datasetCtx *Dataset, e *Element) {
if datasetCtx == nil || e == nil {
return
}
if e.Tag.Group == 0x0002 || requiredContextElements.Contains(&e.Tag) {
datasetCtx.Elements = append(datasetCtx.Elements, e)
}
}
Loading