Utils function for read/write value from unprocessed data #306
Conversation
| // This avoids the case that we can not handle it, yet. | ||
| func IsSafeForUnprocessedValueDataHandling(info *PixelDataMetadata, unprocessedValueData []byte) error { | ||
| // https://dicom.innolitics.com/ciods/enhanced-mr-image/enhanced-mr-image/00280006 | ||
| if info.PlanarConfiguration == 1 { |
There was a problem hiding this comment.
Can I use the testdata from https://github.com/pydicom/pydicom for the next MR supporting this case.
I am not good at understanding how licence works. So I am not sure whether it is appropriate.
There was a problem hiding this comment.
I think it's okay if we include their full license, but can you point me to the specific file, so I can see if it's distributed with a particular license?
There was a problem hiding this comment.
Thanks for this! Had a couple questions in the comments below. Is the main way this saves memory by only loading one frame at a time into the larger ints?
In general, having utilities for folks to work with the unprocessed pixel data after parsing does seem reasonable to me. That said, I wonder if we can potentially share some of the existing code to read native pixel data... will take a closer look next week or the following weekend to see if I can give a more direct suggestion. Thanks!
| // This avoids the case that we can not handle it, yet. | ||
| func IsSafeForUnprocessedValueDataHandling(info *PixelDataMetadata, unprocessedValueData []byte) error { | ||
| // https://dicom.innolitics.com/ciods/enhanced-mr-image/enhanced-mr-image/00280006 | ||
| if info.PlanarConfiguration == 1 { |
There was a problem hiding this comment.
I think it's okay if we include their full license, but can you point me to the specific file, so I can see if it's distributed with a particular license?
| for _, tc := range cases { | ||
| t.Run(tc.Name, func(t *testing.T) { | ||
| var filesOut bytes.Buffer | ||
| require.NoError(t, dicom.Write(io.Writer(&filesOut), tc.existingData)) |
There was a problem hiding this comment.
nit: let's not use require/assert, and instead just use Go code to do comparisons, see https://google.github.io/styleguide/go/decisions#assert
| // ReadUnprocessedValueData read the value of Dicom image directly | ||
| // from the byte array of PixelData.UnprocessedValueData with given frame ID. | ||
| // This ease the memory usage of reading DICOM image. | ||
| func ReadUnprocessedValueData(info *PixelDataMetadata, unprocessedValueData []byte, frameIndex int) ([][]int, error) { |
There was a problem hiding this comment.
Since this returns [][]int, this will still have the same issue of expanding the memory from the smaller int to the 64-bit int right? Is the main place this saves memory in the fact that it only does one frame at a time?
There was a problem hiding this comment.
Is the main place this saves memory in the fact that it only does one frame at a time
Yes, most of the DICOM with the issues contains multiple frames.
But, I think I would change to a new structure of storing data.
B/c users need to adapt to new way of read/write pixel data anyway.
Rationale:
[[[sample for sample in pixel] for pixel in frame] for frame in image]. B/c the number of pixel per frame is usually 1M (1000 * 1000 image) and sample per pixel can be 1 => this structure takes 4x of memory.Solution: