fix issue with odd number of pixel bytes #233
Conversation
There was a problem hiding this comment.
Thanks for the PR! Overall looking good, just had a couple comments for you to consider as well.
And indeed historically there have been EOF errors that result from various edge cases that we've been fixing example by example over time--like this one! So appreciate it!
read_test.go
Outdated
| @@ -392,6 +392,135 @@ func TestReadNativeFrames(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestReadNativeFrames_BytePixels(t *testing.T) { | |||
There was a problem hiding this comment.
question, in order to test the padded byte case, can we just better parametrize the above test? Esp because the test body seems like it's the same.
One thought could be to introduce a dataBytes parameter into the above test case. If len(data) ==0 in the test case, then we pass dataBytes to binary.Write instead.
What do you think?
There was a problem hiding this comment.
I cleaned it up like you suggested, see what you think
read_test.go
Outdated
| @@ -392,6 +392,135 @@ func TestReadNativeFrames(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestReadNativeFrames_BytePixels(t *testing.T) { | |||
| cases := []struct { | |||
There was a problem hiding this comment.
Can we add a test case to test for the condition where vl%2 != 0 and assert that we get an expectedError we want? We would probably also want to have some kind of sentinel error we return to make checking this easy (e.g. kinda like ErrorUnsupportedBitsAllocated
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
Co-authored-by: Suyash Kumar <suyashkumar2003@gmail.com>
There have been several issues that mention EOF errors. (e.g. #62) This PR fixes an error that manifested itself with an EOF error, so it may address those.
The problem occurs when there is an odd number of bytes in the pixel data, and even though the value length is even, the parser doesn't count the last padding byte.
Regarding the implementation, it might be more robust to skip ahead the difference between
vlandbytesReadinreadNativeFrames, but my understanding of this library and the DICOM standard is that the difference will always be 0 or 1.