gradienthealth / dicom Goto Github PK

Encapsulated and native frame data have fundamentally different representations (the former is JPEG encoded image bytes while the former is raw pixel intensities). We need a common representation of a frame.m

At some point there seems to have been an inadvertent find and replace that snuck in to change %d to %decoder. Example

Create option to only parse pixel data (ignore metadata). This may require some complicated skipping / branching, but should be doable. Useful if metadata about this image has already been pre-parsed and stored somewhere.

It's possible that making Data a *[]*[]int (or a []*[]int) would make parsing native pixel data faster...

More research needs to be done on the exact mechanics of how this occurs in Golang, but it may be possible that some memory copying operations need to be done in L591 below that wouldn't happen if we were adding a *[]int

Of course working with non-indexable *[]int s are less usable for users of this package (but not by too much).

Test our parsing against dicomParser, a js library that seems to have a high degree of compliance and it's own testing.

If we can find or generate DICOM test data where we know exactly what each parsed attribute should be, that would be quite useful for testing purposes.

We can also create fixtures based on our current parser output if we assume that currently we are parsing things correctly (to prevent regressions in this case).

As mentioned in #7, making ReadElement API independent of previously parsed data would be a good choice.

Audit existing code quality, identify any areas necessary for refactors.

Tests should be written for readNativeFrames

We should explore the process of parsing encapsulated image data (that may be encoded using many different transfer syntaxes) into a native representation (like a simple integer slice, like exists for NativePixel data already). It will be beneficial to have one in-transit representation of a dicom image, if it is feasible to safely and reliably convert an encapsulated image to a native pixel data format. To determine this, the pipelines for viewing native and encapsulated data need to be inspected to see if there are points where the data has a common representation.

Ultimately the goal is to represent both native and encapsulated pixel data in a common data structure without compromising the display pipeline for either image.

The dicomutil utility does not currently autoscale colors and intensities, but should probably do so.

We should parse and unpack Elements into native data structures that represent the elements. gradienthealth/dicom-protos already lays the foundataion to generate golang-native messages that represent each attribute, so this repository should populate those data structures. In other words, the below code should happen here and should likely be autogenerated

	// Code to unpack PhotometricInterpretation from a DataSet
	el, errFind = ds.FindElementByTag(dicomtag.PhotometricInterpretation)
	pi, errGet := el.GetString()
        // errors should be handled
	photomericInterpretation = &attrPB.PhotometricInterpretation{Value: pi}

For some DICOMs, the number of bytes read assuming it is in a native pixel format is smaller than the defined value length. It is possible that for these images the Bits / pixel are larger or that there is some special encoding that needs to be looked into further.

It is assumed that an image contains standard native pixel data when the value length of the PixelData attribute is not undefined length.

Likely either logrus or zap

Stream frame packages as they are parsed over channels to any consumers that want them.

This library seems to perform the practice of assigning errors during parsing to the dicomio.Decoder struct, however I expect that we can probably refactor things to be a bit more golang canonical and simply return errors from relevant function calls. This should perhaps be a little easier after #13 lands.

When parsing a non-conforming DICOM file that doesn't have the MetaElementGroupLength tag using NewParserFromFile (e.g. dicomutil) the error is masked by an interface conversion error.

panic: interface conversion: dicom.Parser is nil, not *dicom.parser

goroutine 1 [running]:
github.com/gradienthealth/dicom.NewParserFromFile(0x7ffeefbff937, 0x1e, 0x0, 0x2, 0x0, 0x0, 0x40)
	/Users/cmcgee/go/src/github.com/gradienthealth/dicom/parse.go:81 +0x19c
main.main()
	/Users/cmcgee/go/src/github.com/gradienthealth/dicom/dicomutil/dicomutil.go:74 +0x4dd

The cause is in the NewParserFromFile function that is trying to convert the interface, which can be nil, before checking the error from the call to NewParser() in parse.go line 81. Instead, the error should be checked before attempting to convert the interface. Alternatively, the second return value of the interface conversion could be checked to make sure that the conversion succeeds, avoiding the panic.

In accordance with #3. Lots of exported entities right now from the DICOM package -- this should really be investigated and reduced.

Currently this function iterates over all elements to find a tag match. It should be simple to make this more speed efficient using a hashmap index. memory tradeoffs should be considered

How fast is "fast"?

Please publish a few metrics.

Some hundred thousand files tried it out, always the same result: panic.

Example:
$ ./dicomutil-linux-amd64 -print-metadata xyz.dcm
2019/07/13 13:37:40 Error reading xyz.dcm: dicom.ParseSpecificCharacterSet: Unknown character set 'ISO_IR 192'. Assuming utf-8 (file offset 334)
panic: Error reading xyz.dcm: dicom.ParseSpecificCharacterSet: Unknown character set 'ISO_IR 192'. Assuming utf-8 (file offset 334)

goroutine 1 [running]:
log.Panicf (0x637eca, 0x14, 0xc00015be20, 0x2, 0x2)
         /usr/local/go/src/log/log.go:333 + 0xda
main.main ()
         /Users/suyashkumar/go-work/src/github.com/suyashkumar/dicom/cmd/dicomutil/dicomutil.go:79 + 0x919