dcmqi: PixelData is encoded incorrectly when frame size is not divisible by 8

This issue was discovered while adding corresponding test dataset in #334.

Frame pixel data is initialized in dcmqi here: https://github.com/QIICR/dcmqi/blob/master/libsrc/ImageSEGConverter.cpp#L320-L328.

The frames are then concatenated to form SEG object PixelData in DcmSegmentation::concatFrames here: https://github.com/commontk/DCMTK/blob/patched-DCMTK-3.6.3_20180205/dcmseg/libsrc/segdoc.cc#L1314.

It appears that something is not quite right with the concatenation. The issue of incorrect pixel data encoding can be easily reproduced with the python code below (this code works with Python 3 and the only extra dependency is pydicom) and this sample dataset from #334.

import pydicom, sys

d = pydicom.read_file(sys.argv[1])
print(d.Rows)
print(d.Columns)
print(d.NumberOfFrames)
print(d.PixelData)

totalPixels = int(d.Rows*d.Columns*d.NumberOfFrames/8)
if totalPixels%8:
  totalPixels = totalPixels + 1
totalPixels = totalPixels + (totalPixels % 2)
print("Total pixels expected: %i" % totalPixels)

print("Total pixels actual: %i" % len(d.PixelData))


for f in range(d.NumberOfFrames):
  print("Frame %i" % f)
  for i in range(d.Rows):
    for j in range(d.Columns):
      pixelNumber = f*d.Rows*d.Columns+i*d.Columns+j
      byteNumber = int(pixelNumber/8)
      bitPosition = pixelNumber % 8
      value = (d.PixelData[byteNumber] >> bitPosition) & 1
      sys.stdout.write(str(value))
    print("")

The output of the code above appears to be identical to the image generated with dcm2pnm, as shown in https://github.com/QIICR/dcmqi/pull/334#issuecomment-370449995.

Relevant part of the DICOM standard detailing how bits should be packed in multiframe is in PS3.5 8.1.1 Pixel Data Encoding of Related Data Elements. The most important part is there should be no gap/padding between the encoded consecutive frames, and DCMTK implementation is expected to follow that rule, based on earlier communication and debugging.

@michaelonken I have not yet identified the specific problem in the DCMTK concatenation function, but it looks like that is the culprit. I will look into this later next week, but I need to work on something else now and decided to create the issue with the details, in case you want to investigate.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 47 (36 by maintainers)

Commits related to this issue

Most upvoted comments

I had time to look at this recently and I agree with you: https://github.com/DCMTK/dcmtk/blob/master/dcmseg/libsrc/segdoc.cc#L1213 is where it happens.

It should read

remainder = bytesRequired % 8;

instead since bytesRequired takes into account the number of pixels not for a single frame (old buggy code) but for all frames.

Did you already check whether this does the trick for the files you have at hand, @hmeine?

+2
michaelonken on Feb 12, 2021

I will look into this. Right now I have no time so it can take a few days…thanks for spending time to isolate this, Hans.

+2
michaelonken on Oct 29, 2020

I just wanted to give information which I believe is related to this issue: In MeVisLab, we originally exported SEG files without padding. Then, we got back such files as “invalid” from an important customer who claimed that rows “something” frames had to be padded to multiples of 8 16 bits, and we could not find information in an older DICOM standard version that prohibited that. Nowadays, the standard explicitly states that applications shall be prepared to support padded frames since this was not forbidden in older standard versions (Part 5, 8.1.1).

We now have example files without padding (which seem to be the recommended encoding by today’s standard) that make dcmqi crash, apparently because of an “Invalid Tag” error from dcmtk.

+2
hmeine on Oct 27, 2020

yes I recently pushed that into DCMTK master. I am having a report on the same issue from another party, and I am about to check whether matches this one (I think it does), that is why I did not update this thread so far. However, the patch is ok and tested, so the report may be unrelated (or deals with the same problem).

+1
michaelonken on Mar 4, 2021

@hmeine OK, should I open one?

+1
AGGDor on Mar 4, 2021

No, and I do not have my own dcmtk build at hand right now. If you could compile dcmqi against a patched dcmtk, however, you could test running segimage2itkimage on the file I attached above: https://github.com/QIICR/dcmqi/files/5458825/Lesions1.dcm.zip

+1
hmeine on Feb 12, 2021

I think there is an issue in DCMTK, and this line should also multiply by the number of frames: https://github.com/DCMTK/dcmtk/blob/master/dcmseg/libsrc/segdoc.cc#L1213.

+1
fedorov on Oct 29, 2020

Attached you find a Lesions1.dcm, zipped for GitHub which contains the described mask. The file is pseudonymized and only contains a lesion mask, so I got permission to share it with you.

+1
hmeine on Oct 29, 2020

Thanks to @michaelonken DCMTK has now been updated with the fix.

On Mon, Jun 11, 2018 at 1:01 AM, Michael Onken wrote:

the patch is available now upstream (DCMTK master) see this commit:

http://git.dcmtk.org/?p=dcmtk.git;a=commit;h=ba542273f8670f13fa222f414890849b1d1265b2

I used another commit a day later to fix the test for all compilers:

http://git.dcmtk.org/?p=dcmtk.git;a=commit;h=5f22e71e6ab1654e0ca787f2d779b0a69944feef

+1
fedorov on Jun 11, 2018
Read more comments on GitHub
← dcmqi: FATAL ERROR: Writing of the SEG dataset failed
qiita: Install instructions give error at step to install redis on mac →