Page MenuHomePhabricator
Create Task
Maniphest T2473

locale issues in DICOM file reading
Closed, ResolvedPublic
Actions

Description

While tesing import of DICOM images, I found that one computer with Ubuntu 9.04 64bit read certain DICOM files upside-down and without spacing information. (A couple of other computers with identical OS did not have this problem)

After checking all kind of differences in ITK build options, I found that one difference between the systems was the locale. The computer with the error had a German locale, while all others were set to US English.

Changing the users's locale to US proved to be a workaround for the problem.

To allow users to keep their locale, I want to encapsulate DICOM reading in mitk::DataTreeNodeFactory in a block that

  • remembers the current locale
  • changes the locale to "C"
  • reads DICOM
  • resets the locale to the original setting

This seemed to be a good workaround for the current DICOM reader implementation's problem.

MITK tests and manual testing that is going on currently will show if this fix has side effects.

Related Objects
Search...

Event Timeline

maleike added projects: MITK, Bugzilla.Sep 12 2009, 11:47 AM
maleike added a subscriber: maleike.

[SVN revision 18922]
FIX (#2473): change locale to "C" before reading DICOM, because the current DICOM reader does not read spacing information correctly with e.g. a German locale

nolden added a subscriber: nolden.Sep 12 2009, 1:18 PM

Maybe it's related to this comment from the DCMTK documentaion (http://support.dcmtk.org/docs/file_macros.html)

DISABLE_OFSTD_ATOF

Affected: all modules
Type of modification: Disables feature
Explanation: by default, DCMTK uses it's own implementation of atof to
  convert strings to double numbers in a locale-independent manner.
  This flag forces DCMTK to use the standard sscanf() function
  instead, which is normally much faster and gives a higher precision
  than DCMTK's built in code, but is locale dependent, i.e. cannot be
  used with locales such as German since DICOM decimal strings always
  use the Posix locale.

German locale uses . and , different than international locales. Maybe some sscanf or other parsing code in MITK is affected as well. Will create a new bug for this.

maleike added a comment.Jan 21 2010, 11:33 AM

[SVN revision 21101]
ENH (#2473): test to reproduce erroneous DICOM parsing with German locale

maleike added a comment.Jan 21 2010, 11:36 AM

Just added a test that is able to reproduce erronous spacing parsing at least on my machine. I'll see whether it affects the continuous clients, hope that the test will fail on all of them. If so, I'll deactivate the corresponding test code line so that development can go on. It can be re-activated to verify potential fixes.

maleike added a comment.Jan 21 2010, 11:51 AM

[SVN revision 21102]
COMP (#2473): deactivate test until a workaround for wrong DICOM parsing is implemented.

maleike added a comment.Jan 21 2010, 11:53 AM

Just to comment on the bug history: at 2009-09-12 11:47:48 I meant to set the active_assignee flag and NOT the needs_core_modification flag. Sorry. I hope test cases are still committable without the core_modification flag.

maleike added a comment.Jan 25 2010, 4:00 PM

Attached proposed solution:

In DataTreeNodeFactory, while reading a DICOM series:

  • remeber current locale as "previous locale"
  • set locale for C and std::cin to "C"
  • read DICOM using GDCM
  • reset locale for C and std::cin to "previous locale"

Testcase has been changed to verify fix

patch-2473.patch2 KBDownload

maleike added a comment.Jan 25 2010, 4:15 PM

Change request at: http://proxy.mitk.org/wiki/ChangeRequests/2473

maleike added a comment.Jan 25 2010, 4:18 PM

[SVN revision 21114]
FIX (#2473): use 'C' locale while reading DICOM to ensure proper number parsing

maleike added a comment.Jan 25 2010, 4:20 PM

... I messed it up a bit, wantd to set the flag to ? ... fix is comitted already. Marco, you could double check it..

maleike added a comment.Jan 25 2010, 4:52 PM

Reopening for comitting

maleike added a comment.Jan 25 2010, 4:52 PM

[SVN revision 21116]
COMP (#2473): if a locale does not exist on given platform, do not count this as an error

maleike added a comment.Jan 25 2010, 5:00 PM

now the test is passing all continuous clients

maleike added a comment.Jan 26 2010, 5:45 PM

[SVN revision 21126]
FIX (#2473): add test condition that at least one German locale should be tested

maleike added a comment.Jan 26 2010, 7:28 PM

[SVN revision 21127]
COMP (#2473): add more locales and output initial locale

maleike added a comment.Jan 26 2010, 8:14 PM

[SVN revision 21128]
COMP (#2473): add more locales and output initial locale

maleike added a comment.Jan 26 2010, 8:30 PM

[SVN revision 21129]
COMP (#2473): add Windows specific locales

zelzer added a subscriber: zelzer.Nov 9 2010, 1:59 PM

[SVN revision 27412]
FIX (#2473): Don't treat missing German locale as error

As discussed in todays MITK meeting, the absence of German
locales should not lead to a test failure. External users
of MITK with only their native locale installed should still
be able to run the tests successfully.

However, internally people should install at least one
German locale so that the test makes sense.

MITK · MIC · IMSY · DKFZ