dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

All other questions regarding DCMTK

Moderator: Moderator Team

Post Reply
Message
Author
Shaeto
Posts: 147
Joined: Tue, 2009-01-20, 17:50
Location: CA, USA
Contact:

dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#1 Post by Shaeto »

switch (photometricInterpretation) in

dcmjpeg/libsrc/djcodece.cc: DJCodecEncoder::encode

does not catch EPI_YBR_RCT and EPI_YBR_ICT and there is no default:, so, it produces wrong Pixel Representation context with NULL pixel sequence

how to reproduce: get or create 8 bits image dicom having

(0028,0004) CS [YBR_RCT] # 8, 1 PhotometricInterpretation

and try to compress it

dcmcjpeg +eb input.dcm output.dcm

produces
Segmentation fault (core dumped)

sorry i can't attach test dicom

not sure how to fix it, may be process RCT/ICT as EPI_RGB

J. Riesmeier
DCMTK Developer
Posts: 2498
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#2 Post by J. Riesmeier »

Which version of the DCMTK did you use? As far as I can see, there is no such thing as EPI_YBR_RCT or EPI_YBR_ICT (enums) since these photometric interpretations (YBR_RCT​ and YBR_ICT) are only allowed for compressed images, not for uncompressed images. The same is true for YBR_PARTIAL_420.
switch (photometricInterpretation) in

dcmjpeg/libsrc/djcodece.cc: DJCodecEncoder::encode

does not catch EPI_YBR_RCT and EPI_YBR_ICT and there is no default:
If a switch statement does not catch all cases and there would be no default case, I would expect the compiler to note this. Don't you agree?

Shaeto
Posts: 147
Joined: Tue, 2009-01-20, 17:50
Location: CA, USA
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#3 Post by Shaeto »

i tested this case using master

I understand what YBR_RCT​ and YBR_ICT are only allowed for compressed images but it seems not all developers understand it also, so, our equipment sometime receives such dicoms from some "client tele-medical environment"

it seems CMAKE_CXX_FLAGS is lack of -Wall thats why there is no warning

cmake is 3.11.4

DCMTKConfig.cmake:set(DCMTK_CMAKE_CXX_FLAGS -fvisibility=hidden -D_XOPEN_SOURCE_EXTENDED -D_XOPEN_SOURCE=500 -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_BSD_COMPAT -D_OSF_SOURCE -D_POSIX_C_SOURCE=199506L)
DCMTKConfig.cmake:set(DCMTK_CMAKE_CXX_FLAGS_DEBUG -g -DDEBUG)
DCMTKConfig.cmake:set(DCMTK_CMAKE_CXX_FLAGS_RELEASE -O2 -DNDEBUG)
DCMTKConfig.cmake:set(DCMTK_CMAKE_CXX_FLAGS_MINSIZEREL -Os -DNDEBUG)
DCMTKConfig.cmake:set(DCMTK_CMAKE_CXX_FLAGS_RELWITHDEBINFO -O2 -g -DNDEBUG)

J. Riesmeier
DCMTK Developer
Posts: 2498
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#4 Post by J. Riesmeier »

I understand what YBR_RCT​ and YBR_ICT are only allowed for compressed images but it seems not all developers understand it also, so, our equipment sometime receives such dicoms from some "client tele-medical environment"
Here's the reference to the relevant section in the DICOM standard: http://dicom.nema.org/medical/dicom/cur ... t_8.2.html

YBR_RCT, YBR_ICT and YBR_PARTIAL_420 make no sense for uncompressed images.
it seems CMAKE_CXX_FLAGS is lack of -Wall thats why there is no warning
When I compile the DCMTK with c++ (GNU C++ compiler), I get a warning when a switch statement does not check all cases (or does not include a default case). However, I usually prefer GNU Autoconf instead of CMake on Unix-like systems, so -Wall is usually set, which implies -Wswitch. I will ask my CMake-affine colleagues why -Wall is not set by default.

Anyway, where does the crash happen?

Shaeto
Posts: 147
Joined: Tue, 2009-01-20, 17:50
Location: CA, USA
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#5 Post by Shaeto »

YBR_RCT, YBR_ICT and YBR_PARTIAL_420 make no sense for uncompressed images.
agree! but can't change this world - wrong dicom is came
Anyway, where does the crash happen?
it crashed in dcmdata/libsrc/dcpixel.cc:1195
1195 return (*current)->pixSeq->loadAllDataIntoMemory();

pixSeq is null (because of missed switch/case)

just to make sure: i don't want to compress this wrong dicom, bad condition returned from chooseRepresentation and no crash is enough

J. Riesmeier
DCMTK Developer
Posts: 2498
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#6 Post by J. Riesmeier »

it crashed in dcmdata/libsrc/dcpixel.cc:1195
Do you really use "master"? http://git.dcmtk.org/?p=dcmtk.git;a=blo ... HEAD#l1195
1195 return (*current)->pixSeq->loadAllDataIntoMemory();
It's line 1168 in "master".

Anyway, I tested what you wrote in your first posting:
how to reproduce: get or create 8 bits image dicom having

(0028,0004) CS [YBR_RCT] # 8, 1 PhotometricInterpretation

and try to compress it

dcmcjpeg +eb input.dcm output.dcm

produces
Segmentation fault (core dumped)
I get no crash but the following error message:

Code: Select all

F: no conversion to transfer syntax JPEG Baseline possible!

Shaeto
Posts: 147
Joined: Tue, 2009-01-20, 17:50
Location: CA, USA
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#7 Post by Shaeto »

yes i use a fork of master but seems it is modified to support jpeg-2000 classes

in dcmimgle/include/dcmtk/dcmimgle/diutils.h we added

const SP_Interpretation PhotometricInterpretationNames[] =
.....
{"YBRICT", "YBR_ICT", EPI_YBR_ICT},
{"YBRRCT", "YBR_RCT", EPI_YBR_RCT},
{"YUVRCT", "YUV_RCT", EPI_YBR_RCT},
....

so, my fork can recognize and return EPI_YBR_ICT in that switch(...)

anyway, i think it is good idea to add "default" something like:


case EPI_Unknown:
default:

J. Riesmeier
DCMTK Developer
Posts: 2498
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#8 Post by J. Riesmeier »

So, the issue (crash) is caused by an incomplete modification of DCMTK's source code, right?
anyway, i think it is good idea to add "default"
Yes, one could do this, e.g. you could/should do this as part of your fork, but I am not sure whether this really makes sense for the public DCMTK source code. For example, if DICOM would ever introduce new values for Photometric Interpretation the compiler will (usually, see above) report all occurrences where the new enum (EPI_xxx) is not yet handled.

Shaeto
Posts: 147
Joined: Tue, 2009-01-20, 17:50
Location: CA, USA
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#9 Post by Shaeto »

Okay thank you i will modify our fork,

last question is -Wall for cxx flags in cmake configuration

J. Riesmeier
DCMTK Developer
Posts: 2498
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#10 Post by J. Riesmeier »

last question is -Wall for cxx flags in cmake configuration
I've already forwarded this question to the DCMTK team members who prefer to use CMake (on non-Windows systems).

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT

#11 Post by Michael Onken »

-Wall is not part of our CMake configuration right now. We consider to enable it per default.

Best regards,
Michael

Post Reply

Who is online

Users browsing this forum: Ahrefs [Bot], paperini and 1 guest