dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
Moderator: Moderator Team
dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
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
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
-
- DCMTK Developer
- Posts: 2506
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
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.
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?switch (photometricInterpretation) in
dcmjpeg/libsrc/djcodece.cc: DJCodecEncoder::encode
does not catch EPI_YBR_RCT and EPI_YBR_ICT and there is no default:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
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)
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)
-
- DCMTK Developer
- Posts: 2506
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
Here's the reference to the relevant section in the DICOM standard: http://dicom.nema.org/medical/dicom/cur ... t_8.2.htmlI 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"
YBR_RCT, YBR_ICT and YBR_PARTIAL_420 make no sense for uncompressed images.
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.it seems CMAKE_CXX_FLAGS is lack of -Wall thats why there is no warning
Anyway, where does the crash happen?
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
agree! but can't change this world - wrong dicom is cameYBR_RCT, YBR_ICT and YBR_PARTIAL_420 make no sense for uncompressed images.
it crashed in dcmdata/libsrc/dcpixel.cc:1195Anyway, where does the crash happen?
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
-
- DCMTK Developer
- Posts: 2506
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
Do you really use "master"? http://git.dcmtk.org/?p=dcmtk.git;a=blo ... HEAD#l1195it crashed in dcmdata/libsrc/dcpixel.cc:1195
It's line 1168 in "master".1195 return (*current)->pixSeq->loadAllDataIntoMemory();
Anyway, I tested what you wrote in your first posting:
I get no crash but the following error message: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)
Code: Select all
F: no conversion to transfer syntax JPEG Baseline possible!
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
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:
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:
-
- DCMTK Developer
- Posts: 2506
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
So, the issue (crash) is caused by an incomplete modification of DCMTK's source code, right?
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.anyway, i think it is good idea to add "default"
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
Okay thank you i will modify our fork,
last question is -Wall for cxx flags in cmake configuration
last question is -Wall for cxx flags in cmake configuration
-
- DCMTK Developer
- Posts: 2506
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
I've already forwarded this question to the DCMTK team members who prefer to use CMake (on non-Windows systems).last question is -Wall for cxx flags in cmake configuration
-
- DCMTK Developer
- Posts: 2052
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: dcmjpeg encoder crashes on EPI_YBR_RCT and EPI_YBR_ICT
-Wall is not part of our CMake configuration right now. We consider to enable it per default.
Best regards,
Michael
Best regards,
Michael
Who is online
Users browsing this forum: Ahrefs [Bot], Baidu [Spider], Google [Bot] and 1 guest