Mercurial > hg > orthanc
changeset 2207:6dc3bdb4088b
Fix handling of encodings in C-FIND for worklists
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Fri, 09 Dec 2016 11:24:04 +0100 |
parents | 27106f7e3759 |
children | 90ea60bee5ff |
files | NEWS OrthancServer/DicomProtocol/DicomFindAnswers.cpp OrthancServer/DicomProtocol/DicomFindAnswers.h OrthancServer/FromDcmtkBridge.cpp OrthancServer/FromDcmtkBridge.h OrthancServer/ParsedDicomFile.cpp OrthancServer/ParsedDicomFile.h Resources/Configuration.json TODO UnitTestsSources/FromDcmtkTests.cpp |
diffstat | 10 files changed, 167 insertions(+), 90 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Thu Dec 08 16:29:34 2016 +0100 +++ b/NEWS Fri Dec 09 11:24:04 2016 +0100 @@ -30,7 +30,7 @@ Maintenance ----------- -* Fix handling of encodings in C-FIND requests +* Fix handling of encodings in C-FIND requests (including for worklists) * Use of DCMTK 3.6.1 dictionary of private tags in standalone builds * Avoid hard crash if not enough memory (handling of std::bad_alloc) * Improved robustness of Orthanc Explorer wrt. query/retrieve (maybe fix issue 24)
--- a/OrthancServer/DicomProtocol/DicomFindAnswers.cpp Thu Dec 08 16:29:34 2016 +0100 +++ b/OrthancServer/DicomProtocol/DicomFindAnswers.cpp Fri Dec 09 11:24:04 2016 +0100 @@ -33,8 +33,8 @@ #include "../PrecompiledHeadersServer.h" #include "DicomFindAnswers.h" +#include "../OrthancInitialization.h" #include "../FromDcmtkBridge.h" -#include "../ToDcmtkBridge.h" #include "../../Core/OrthancException.h" #include <memory> @@ -44,72 +44,42 @@ namespace Orthanc { - class DicomFindAnswers::Answer : public boost::noncopyable + void DicomFindAnswers::AddAnswerInternal(ParsedDicomFile* answer) { - private: - ParsedDicomFile* dicom_; - DicomMap* map_; + std::auto_ptr<ParsedDicomFile> protection(answer); - void CleanupDicom(bool isWorklist) + if (isWorklist_) { - if (isWorklist && - dicom_ != NULL) - { - // These lines are necessary when serving worklists, otherwise - // Orthanc does not behave as "wlmscpfs" - dicom_->Remove(DICOM_TAG_MEDIA_STORAGE_SOP_INSTANCE_UID); - dicom_->Remove(DICOM_TAG_SOP_INSTANCE_UID); - } - } - - public: - Answer(bool isWorklist, - ParsedDicomFile& dicom) : - dicom_(dicom.Clone()), - map_(NULL) - { - CleanupDicom(isWorklist); + // These lines are necessary when serving worklists, otherwise + // Orthanc does not behave as "wlmscpfs" + protection->Remove(DICOM_TAG_MEDIA_STORAGE_SOP_INSTANCE_UID); + protection->Remove(DICOM_TAG_SOP_INSTANCE_UID); } - Answer(bool isWorklist, - const void* dicom, - size_t size) : - dicom_(new ParsedDicomFile(dicom, size)), - map_(NULL) + protection->ChangeEncoding(encoding_); + + answers_.push_back(protection.release()); + } + + + DicomFindAnswers::DicomFindAnswers(bool isWorklist) : + encoding_(Configuration::GetDefaultEncoding()), + isWorklist_(isWorklist), + complete_(true) + { + } + + + void DicomFindAnswers::SetEncoding(Encoding encoding) + { + for (size_t i = 0; i < answers_.size(); i++) { - CleanupDicom(isWorklist); - } - - Answer(const DicomMap& map) : - dicom_(NULL), - map_(map.Clone()) - { + assert(answers_[i] != NULL); + answers_[i]->ChangeEncoding(encoding); } - ~Answer() - { - if (dicom_ != NULL) - { - delete dicom_; - } - - if (map_ != NULL) - { - delete map_; - } - } - - ParsedDicomFile& GetDicomFile() - { - if (dicom_ == NULL) - { - assert(map_ != NULL); - dicom_ = new ParsedDicomFile(*map_); - } - - return *dicom_; - } - }; + encoding_ = encoding; + } void DicomFindAnswers::SetWorklist(bool isWorklist) @@ -149,28 +119,27 @@ void DicomFindAnswers::Add(const DicomMap& map) { - answers_.push_back(new Answer(map)); + AddAnswerInternal(new ParsedDicomFile(map, encoding_)); } void DicomFindAnswers::Add(ParsedDicomFile& dicom) { - answers_.push_back(new Answer(isWorklist_, dicom)); + AddAnswerInternal(dicom.Clone()); } - void DicomFindAnswers::Add(const void* dicom, size_t size) { - answers_.push_back(new Answer(isWorklist_, dicom, size)); + AddAnswerInternal(new ParsedDicomFile(dicom, size)); } - DicomFindAnswers::Answer& DicomFindAnswers::GetAnswerInternal(size_t index) const + ParsedDicomFile& DicomFindAnswers::GetAnswer(size_t index) const { if (index < answers_.size()) { - return *answers_.at(index); + return *answers_[index]; } else { @@ -179,12 +148,6 @@ } - ParsedDicomFile& DicomFindAnswers::GetAnswer(size_t index) const - { - return GetAnswerInternal(index).GetDicomFile(); - } - - DcmDataset* DicomFindAnswers::ExtractDcmDataset(size_t index) const { return new DcmDataset(*GetAnswer(index).GetDcmtkObject().getDataset());
--- a/OrthancServer/DicomProtocol/DicomFindAnswers.h Thu Dec 08 16:29:34 2016 +0100 +++ b/OrthancServer/DicomProtocol/DicomFindAnswers.h Fri Dec 09 11:24:04 2016 +0100 @@ -39,26 +39,28 @@ class DicomFindAnswers : public boost::noncopyable { private: - class Answer; + Encoding encoding_; + bool isWorklist_; + std::vector<ParsedDicomFile*> answers_; + bool complete_; - bool isWorklist_; - std::vector<Answer*> answers_; - bool complete_; - - Answer& GetAnswerInternal(size_t index) const; + void AddAnswerInternal(ParsedDicomFile* answer); public: - DicomFindAnswers(bool isWorklist) : - isWorklist_(isWorklist), - complete_(true) - { - } + DicomFindAnswers(bool isWorklist); ~DicomFindAnswers() { Clear(); } + Encoding GetEncoding() const + { + return encoding_; + } + + void SetEncoding(Encoding encoding); + void SetWorklist(bool isWorklist); bool IsWorklist() const
--- a/OrthancServer/FromDcmtkBridge.cpp Thu Dec 08 16:29:34 2016 +0100 +++ b/OrthancServer/FromDcmtkBridge.cpp Fri Dec 09 11:24:04 2016 +0100 @@ -1897,4 +1897,50 @@ target.SetValue(ParseTag(members[i]), value.asString(), false); } } + + + void FromDcmtkBridge::ChangeStringEncoding(DcmItem& dataset, + Encoding source, + Encoding target) + { + // Recursive exploration of a dataset to change the encoding of + // each string-like element + + if (source == target) + { + return; + } + + for (unsigned long i = 0; i < dataset.card(); i++) + { + DcmElement* element = dataset.getElement(i); + if (element) + { + if (element->isLeaf()) + { + char *c = NULL; + if (element->isaString() && + element->getString(c).good() && + c != NULL) + { + std::string a = Toolbox::ConvertToUtf8(c, source); + std::string b = Toolbox::ConvertFromUtf8(a, target); + element->putString(b.c_str()); + } + } + else + { + // "All subclasses of DcmElement except for DcmSequenceOfItems + // are leaf nodes, while DcmSequenceOfItems, DcmItem, DcmDataset + // etc. are not." The following dynamic_cast is thus OK. + DcmSequenceOfItems& sequence = dynamic_cast<DcmSequenceOfItems&>(*element); + + for (unsigned long j = 0; j < sequence.card(); j++) + { + ChangeStringEncoding(*sequence.getItem(j), source, target); + } + } + } + } + } }
--- a/OrthancServer/FromDcmtkBridge.h Thu Dec 08 16:29:34 2016 +0100 +++ b/OrthancServer/FromDcmtkBridge.h Fri Dec 09 11:24:04 2016 +0100 @@ -88,6 +88,10 @@ unsigned int maxStringLength, Encoding defaultEncoding); + static void ChangeStringEncoding(DcmItem& dataset, + Encoding source, + Encoding target); + public: static void InitializeDictionary(bool loadPrivateDictionary);
--- a/OrthancServer/ParsedDicomFile.cpp Thu Dec 08 16:29:34 2016 +0100 +++ b/OrthancServer/ParsedDicomFile.cpp Fri Dec 09 11:24:04 2016 +0100 @@ -1384,4 +1384,16 @@ { return DicomFrameIndex::GetFramesCount(*pimpl_->file_); } + + + void ParsedDicomFile::ChangeEncoding(Encoding target) + { + Encoding source = GetEncoding(); + + if (source != target) // Avoid unnecessary conversion + { + ReplacePlainString(DICOM_TAG_SPECIFIC_CHARACTER_SET, GetDicomSpecificCharacterSet(target)); + FromDcmtkBridge::ChangeStringEncoding(*pimpl_->file_->getDataset(), source, target); + } + } }
--- a/OrthancServer/ParsedDicomFile.h Thu Dec 08 16:29:34 2016 +0100 +++ b/OrthancServer/ParsedDicomFile.h Fri Dec 09 11:24:04 2016 +0100 @@ -125,6 +125,7 @@ RemovePrivateTagsInternal(&toKeep); } + // WARNING: This function handles the decoding of strings to UTF8 bool GetTagValue(std::string& value, const DicomTag& tag); @@ -143,6 +144,8 @@ Encoding GetEncoding() const; + // WARNING: This function only sets the encoding, it will not + // convert the encoding of the tags. Use "ChangeEncoding()" if need be. void SetEncoding(Encoding encoding); void DatasetToJson(Json::Value& target, @@ -173,6 +176,7 @@ static ParsedDicomFile* CreateFromJson(const Json::Value& value, DicomFromJsonFlags flags); + + void ChangeEncoding(Encoding target); }; - }
--- a/Resources/Configuration.json Thu Dec 08 16:29:34 2016 +0100 +++ b/Resources/Configuration.json Fri Dec 09 11:24:04 2016 +0100 @@ -88,10 +88,10 @@ // The default encoding that is assumed for DICOM files without // "SpecificCharacterSet" DICOM tag, and that is used when answering - // C-Find requests. The allowed values are "Ascii", "Utf8", - // "Latin1", "Latin2", "Latin3", "Latin4", "Latin5", "Cyrillic", - // "Windows1251", "Arabic", "Greek", "Hebrew", "Thai", "Japanese", - // and "Chinese". + // C-Find requests (including worklists). The allowed values are + // "Ascii", "Utf8", "Latin1", "Latin2", "Latin3", "Latin4", + // "Latin5", "Cyrillic", "Windows1251", "Arabic", "Greek", "Hebrew", + // "Thai", "Japanese", and "Chinese". "DefaultEncoding" : "Latin1", // The transfer syntaxes that are accepted by Orthanc C-Store SCP
--- a/TODO Thu Dec 08 16:29:34 2016 +0100 +++ b/TODO Fri Dec 09 11:24:04 2016 +0100 @@ -114,8 +114,8 @@ * Use Semaphore::Locker everywhere (instead of explicit Release() and Acquire()) -* Avoid direct calls to FromDcmtkBridge, go through ParsedDicomFile - wherever possible +* Avoid direct calls to FromDcmtkBridge (make most of its + methods private), go through ParsedDicomFile wherever possible =================
--- a/UnitTestsSources/FromDcmtkTests.cpp Thu Dec 08 16:29:34 2016 +0100 +++ b/UnitTestsSources/FromDcmtkTests.cpp Fri Dec 09 11:24:04 2016 +0100 @@ -1169,3 +1169,49 @@ } } } + + +TEST(ParsedDicomFile, ChangeEncoding) +{ + for (unsigned int i = 0; i < testEncodingsCount; i++) + { + // 1251 codepage is not supported by the core DICOM standard, ignore it + if (testEncodings[i] != Encoding_Windows1251) + { + DicomMap m; + m.SetValue(DICOM_TAG_PATIENT_NAME, testEncodingsExpected[i], false); + + std::string tag; + + ParsedDicomFile dicom(m, Encoding_Utf8); + ASSERT_EQ(Encoding_Utf8, dicom.GetEncoding()); + ASSERT_TRUE(dicom.GetTagValue(tag, DICOM_TAG_PATIENT_NAME)); + ASSERT_EQ(tag, testEncodingsExpected[i]); + + { + Json::Value v; + dicom.DatasetToJson(v, DicomToJsonFormat_Human, DicomToJsonFlags_Default, 0); + ASSERT_STREQ(v["SpecificCharacterSet"].asCString(), "ISO_IR 192"); + ASSERT_STREQ(v["PatientName"].asCString(), testEncodingsExpected[i]); + } + + dicom.ChangeEncoding(testEncodings[i]); + + ASSERT_EQ(testEncodings[i], dicom.GetEncoding()); + + const char* c = NULL; + ASSERT_TRUE(dicom.GetDcmtkObject().getDataset()->findAndGetString(DCM_PatientName, c).good()); + EXPECT_STREQ(c, testEncodingsEncoded[i]); + + ASSERT_TRUE(dicom.GetTagValue(tag, DICOM_TAG_PATIENT_NAME)); // Decodes to UTF-8 + EXPECT_EQ(tag, testEncodingsExpected[i]); + + { + Json::Value v; + dicom.DatasetToJson(v, DicomToJsonFormat_Human, DicomToJsonFlags_Default, 0); + ASSERT_STREQ(v["SpecificCharacterSet"].asCString(), GetDicomSpecificCharacterSet(testEncodings[i])); + ASSERT_STREQ(v["PatientName"].asCString(), testEncodingsExpected[i]); + } + } + } +}