Create Frame Constructs #19
Conversation
suyashkumar
changed the title
element.go
Outdated
| // Frame represents a single encapsulated or native image frame | ||
| type Frame struct { | ||
| IsEncapsulated bool | ||
| EncapsulatedFrame EncapsulatedFrame |
There was a problem hiding this comment.
We can flatten the Frame data structure if we just dump EncapsulatedFrames []byte and NativeFrames [][]int inside the Frame and bring along other attributes like Rows, Cols, and BitsPerSample along for each Frame...even though those attributes are really only needed for work with NativeFrames
There was a problem hiding this comment.
It turns out those attributes are required for both encapsulated and native images so including them in the top level of Frame would not be too bad. They just make less sense for encapsulated images to have included with the Frame itself, but that's okay...
There was a problem hiding this comment.
for now, will likely leave as is, if a refactor is needed it can happen later. Including those elements with every Frame doesn't really add value for EncapsulatedFrames since that information already exists in the encapsulated frame data. We also assume that clients have the previously parsed elements if they happen to need those elements, so no need to duplicate storage of that data for all types of frames.
| @@ -40,12 +40,12 @@ type parser struct { | |||
| decoder *dicomio.Decoder | |||
| parsedElements *DataSet | |||
| op ParseOptions | |||
| frameChannel chan [][]int | |||
| frameChannel chan *Frame | |||
There was a problem hiding this comment.
we'll use this to stream pointers of parsed frames over to a consumer
element.go
Outdated
| s := fmt.Sprintf("image{offsets: %v, frames: [", data.Offsets) | ||
| for i := 0; i < len(data.Frames); i++ { | ||
| if data.Frames[i].IsEncapsulated { | ||
| csum := sha256.Sum256(data.Frames[i].EncapsulatedFrame.Data) |
There was a problem hiding this comment.
semantically, it may be better for a Frame to contain EncapsulatedData instead of an EncapsulatedFrame... so we'd have expressions like
data.Frames[i].EncapsulatedData.Dataor
data.Frames[i].Encapsulated.Data
| // Frame wraps a single encapsulated or native image frame | ||
| type Frame struct { | ||
| IsEncapsulated bool | ||
| EncapsulatedData EncapsulatedFrame |
There was a problem hiding this comment.
Field called EncapsulatedData to better represent the hierarchy of Frames containing EncapsulatedData instead of Frames containing other EncapsulatedFrames...ideally we can come up with a better name for the struct too. Perhaps EncapsulatedFrameData?
suyashkumar
changed the title
|
Not blocking on the naming for now, we can revisit as things develop |
This change creates the concept of a
Framewhich wrapsNativeFrameandEncapsulatedFrameinto oneFrameentity. ThisFrameentity will be streamable via a channel as a DICOM is being parsed. This change adds more abstraction (and therefore a little more boilerplate) when working with Frames.