From 8e1fe98f7659cec85bcbce8551e28f8e45971370 Mon Sep 17 00:00:00 2001 From: frog Date: Fri, 18 Jun 2004 12:26:53 +0000 Subject: [PATCH] * In order to fix writing of dicom files: - Test/TestWriteSimple.cxx: a simpler example of writing. - Test/CMakeLists.txt changed accordingly. - src/gdcmDocument.cxx: -- The destructor now recursilvely removes potential sequences. -- Bug fix in ::IsJPEG2000() -- ::ReplaceOrCreateByNumber(std::string, guint16, guint16) now handles promotion of gdcmDocEntry to gdcmValEntry in a cleaner manner. -- ::GetValEntryByNumber(guint16, guint16) now defined (as opposed to only declared) and build on top of ::GetDocEntryByNumber(guint16, guint16). -- ::SetEntryByNumber() now uses GetValEntryByNumber(group, element) - src/gdcmElementSet.[h|cxx]: added ::RemoveEntry(gdcmDocEntry *) for usage in destructor and treatement of promotion in ::ReplaceOrCreateByNumber(). - src/gdcmSQItem.cxx: destructor should better handle his job. Test/TestWriteSimple now runs (or at least it DOES something). * We can now start hutting memory links. A good starting point is: valgrind -q --skin=memcheck --leak-check=yes --leak-resolution=high --num-callers=100 --show-reachable=yes gdcmTests TestWriteSimple $(GDCMDATA_HOME)/012345.002.050.dcm foo.dcm --- ChangeLog | 24 +++++++ Testing/CMakeLists.txt | 1 + Testing/TestWriteSimple.cxx | 39 +++++++++++ src/gdcmBinEntry.cxx | 26 +++++-- src/gdcmDocument.cxx | 135 ++++++++++++++++++++++++++---------- src/gdcmDocument.h | 6 +- src/gdcmElementSet.cxx | 20 +++++- src/gdcmElementSet.h | 7 +- src/gdcmSQItem.cxx | 12 +++- 9 files changed, 218 insertions(+), 52 deletions(-) create mode 100644 Testing/TestWriteSimple.cxx diff --git a/ChangeLog b/ChangeLog index 01960353..adc27f6f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +2004-06-18 Eric Boix + * In order to fix writing of dicom files: + - Test/TestWriteSimple.cxx: a simpler example of writing. + - Test/CMakeLists.txt changed accordingly. + - src/gdcmDocument.cxx: + -- The destructor now recursilvely removes potential sequences. + -- Bug fix in ::IsJPEG2000() + -- ::ReplaceOrCreateByNumber(std::string, guint16, guint16) + now handles promotion of gdcmDocEntry to gdcmValEntry in a cleaner + manner. + -- ::GetValEntryByNumber(guint16, guint16) now defined (as opposed + to only declared) and build on top of + ::GetDocEntryByNumber(guint16, guint16). + -- ::SetEntryByNumber() now uses GetValEntryByNumber(group, element) + - src/gdcmElementSet.[h|cxx]: added ::RemoveEntry(gdcmDocEntry *) + for usage in destructor and treatement of promotion in + ::ReplaceOrCreateByNumber(). + - src/gdcmSQItem.cxx: destructor should better handle his job. + Test/TestWriteSimple now runs (or at least it DOES something). + * We can now start hutting memory links. A good starting point is: + valgrind -q --skin=memcheck --leak-check=yes --leak-resolution=high + --num-callers=100 --show-reachable=yes gdcmTests TestWriteSimple + $(GDCMDATA_HOME)/012345.002.050.dcm foo.dcm + 2004-06-18 Eric Boix * Valgrind note: after Mathieu Malaterre teached me how to read the valgrind FAQ ;-] (see http://valgrind.kde.org/faq.html), I diff --git a/Testing/CMakeLists.txt b/Testing/CMakeLists.txt index f6dba283..0540b7ee 100644 --- a/Testing/CMakeLists.txt +++ b/Testing/CMakeLists.txt @@ -12,6 +12,7 @@ SET(TEST_SOURCES TestDcm2Acr.cxx TestHash.cxx TestWrite.cxx + TestWriteSimple.cxx ) # add tests that require data diff --git a/Testing/TestWriteSimple.cxx b/Testing/TestWriteSimple.cxx new file mode 100644 index 00000000..33294911 --- /dev/null +++ b/Testing/TestWriteSimple.cxx @@ -0,0 +1,39 @@ +#include "gdcmHeader.h" +#include "gdcmFile.h" +#include "gdcmDebug.h" + + +int TestWriteSimple(int argc, char* argv[]) +{ + + if (argc < 3) + { + std::cerr << "Usage :" << std::endl << argv[0] << + " InputHeader OutputDicom" << std::endl; + return 0; + } + + const char *header = argv[1]; + const char *output = argv[2]; + + dbg.SetDebug(4); + gdcmHeader *f1 = new gdcmHeader( header ); + gdcmFile *f2 = new gdcmFile( f1 ); + + f2->GetImageData(); //EXTREMELY IMPORTANT + //Otherwise ReadPixel == -1 -> the dicom writing fails completely + + int dataSize = f2->GetImageDataSize(); + // unsigned char cast is necessary to be able to delete the buffer + // since deleting a void* is not allowed in c++ + char *imageData = (char*)f2->GetImageData(); + + f2->SetImageData( imageData, dataSize); + + f2->WriteDcmExplVR( output ); + + delete[] imageData; + + return 0; +} + diff --git a/src/gdcmBinEntry.cxx b/src/gdcmBinEntry.cxx index 4498e27f..c9b15621 100644 --- a/src/gdcmBinEntry.cxx +++ b/src/gdcmBinEntry.cxx @@ -1,7 +1,23 @@ -// gdcmBinEntry.cxx -//----------------------------------------------------------------------------- -// +/*========================================================================= + + Program: gdcm + Module: $RCSfile: gdcmBinEntry.cxx,v $ + Language: C++ + Date: $Date: 2004/06/18 12:26:54 $ + Version: $Revision: 1.9 $ + + Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de + l'Image). All rights reserved. See Doc/License.txt or + http://www.creatis.insa-lyon.fr/Public/Gdcm/License.htm for details. + + This software is distributed WITHOUT ANY WARRANTY; without even + the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR + PURPOSE. See the above copyright notices for more information. + +=========================================================================*/ + #include "gdcmBinEntry.h" +#include "gdcmDebug.h" //----------------------------------------------------------------------------- @@ -46,7 +62,9 @@ gdcmBinEntry::~gdcmBinEntry(){ void gdcmBinEntry::Print(std::ostream &os) { PrintCommonPart(os); - std::cout << " gdcmBinEntry : Print, so WHAT ?" <second; + if ( gdcmSeqEntry* SeqEntry = dynamic_cast(entry) ) + { + delete SeqEntry; + RemoveEntry(SeqEntry); + } + else + { + RemoveEntry(entry); + } + } + tagHT.clear(); } //----------------------------------------------------------------------------- // Print -/** - /** * \brief Prints The Dict Entries of THE public Dicom Dictionary @@ -385,8 +397,8 @@ bool gdcmDocument::IsJPEGLossless(void) */ bool gdcmDocument::IsJPEG2000(void) { - return ( IsGivenTransferSyntax(UI1_2_840_10008_1_2_4_70) - || IsGivenTransferSyntax(UI1_2_840_10008_1_2_4_90) ); + return ( IsGivenTransferSyntax(UI1_2_840_10008_1_2_4_90) + || IsGivenTransferSyntax(UI1_2_840_10008_1_2_4_91) ); } /** @@ -557,21 +569,53 @@ gdcmValEntry * gdcmDocument::ReplaceOrCreateByNumber( std::string Value, guint16 Group, guint16 Elem ){ - gdcmDocEntry* a; - gdcmValEntry* b; - a = GetDocEntryByNumber( Group, Elem); - if (a == NULL) { - a =NewDocEntryByNumber(Group, Elem); - if (a == NULL) - return NULL; - b = new gdcmValEntry(a); - AddEntry(b); - } + gdcmDocEntry* CurrentEntry; + gdcmValEntry* ValEntry; + + CurrentEntry = GetDocEntryByNumber( Group, Elem); + if (!CurrentEntry) + { + // The entry wasn't present and we simply create the required ValEntry: + CurrentEntry = NewDocEntryByNumber(Group, Elem); + if (!CurrentEntry) + { + dbg.Verbose(0, "gdcmDocument::ReplaceOrCreateByNumber: call to" + " NewDocEntryByNumber failed."); + return (gdcmValEntry *)0; + } + ValEntry = new gdcmValEntry(CurrentEntry); + if ( !AddEntry(ValEntry)) + { + dbg.Verbose(0, "gdcmDocument::ReplaceOrCreateByNumber: AddEntry" + " failed allthough this is a creation."); + } + } + else + { + ValEntry = dynamic_cast< gdcmValEntry* >(CurrentEntry); + if ( !ValEntry ) + { + // We need to promote the gdcmDocEntry to a gdcmValEntry: + ValEntry = new gdcmValEntry(CurrentEntry); + if (!RemoveEntry(CurrentEntry)) + { + dbg.Verbose(0, "gdcmDocument::ReplaceOrCreateByNumber: removal" + " of previous DocEntry failed."); + return (gdcmValEntry *)0; + } + if ( !AddEntry(ValEntry)) + { + dbg.Verbose(0, "gdcmDocument::ReplaceOrCreateByNumber: adding" + " promoted ValEntry failed."); + return (gdcmValEntry *)0; + } + } + } + SetEntryByNumber(Value, Group, Elem); - b->SetValue(Value); - return (gdcmValEntry*)b; -} + return ValEntry; +} /* * \brief Modifies the value of a given Header Entry (Dicom Element) @@ -754,32 +798,29 @@ bool gdcmDocument::SetEntryByNumber(std::string content, guint16 group, guint16 element) { - TagKey key = gdcmDictEntry::TranslateToKey(group, element); - if ( ! tagHT.count(key)) + gdcmValEntry* ValEntry = GetValEntryByNumber(group, element); + if (!ValEntry) + { + dbg.Verbose(0, "gdcmDocument::SetEntryByNumber: no corresponding", + " ValEntry (try promotion first)."); return false; - int l = content.length(); - if(l%2) // Non even length are padded with a space (020H). - { - l++; - content = content + '\0'; } + + // Non even content must be padded with a space (020H). + if((content.length())%2) + content = content + '\0'; - gdcmValEntry * a; - a = (gdcmValEntry *)tagHT[key]; - - a->SetValue(content); - - VRKey vr = a->GetVR(); + ValEntry->SetValue(content); - guint32 lgr; + // Integers have a special treatement for their length: + VRKey vr = ValEntry->GetVR(); if( (vr == "US") || (vr == "SS") ) - lgr = 2; + ValEntry->SetLength(2); else if( (vr == "UL") || (vr == "SL") ) - lgr = 4; + ValEntry->SetLength(4); else - lgr = l; + ValEntry->SetLength(content.length()); - a->SetLength(lgr); return true; } @@ -1006,6 +1047,25 @@ gdcmDocEntry* gdcmDocument::GetDocEntryByNumber(guint16 group, guint16 element) return tagHT.find(key)->second; } +/** + * \brief Same as \ref gdcmDocument::GetDocEntryByNumber except it only + * returns a result when the corresponding entry is of type + * ValEntry. + * @return When present, the corresponding ValEntry. + */ +gdcmValEntry* gdcmDocument::GetValEntryByNumber(guint16 group, guint16 element) +{ + gdcmDocEntry* CurrentEntry = GetDocEntryByNumber(group, element); + if (! CurrentEntry) + return (gdcmValEntry*)0; + if ( gdcmValEntry* ValEntry = dynamic_cast(CurrentEntry) ) + { + return ValEntry; + } + dbg.Verbose(0, "gdcmDocument::GetValEntryByNumber: unfound ValEntry."); + return (gdcmValEntry*)0; +} + /** * \brief Loads the element while preserving the current * underlying file position indicator as opposed to @@ -1229,7 +1289,6 @@ bool gdcmDocument::WriteEntries(FILE *_fp,FileType type) for (TagDocEntryHT::iterator it = tagHT.begin(); it != tagHT.end(); ++it ) { gdcmDocEntry * entry = it->second; - entry->Print(); if ( type == gdcmACR ){ if (entry->GetGroup() < 0x0008) diff --git a/src/gdcmDocument.h b/src/gdcmDocument.h index ee7b20a4..bc12f0d5 100644 --- a/src/gdcmDocument.h +++ b/src/gdcmDocument.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDocument.h,v $ Language: C++ - Date: $Date: 2004/06/18 00:11:45 $ - Version: $Revision: 1.10 $ + Date: $Date: 2004/06/18 12:26:54 $ + Version: $Revision: 1.11 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -16,8 +16,6 @@ =========================================================================*/ -// gdcmDocument.h -//----------------------------------------------------------------------------- #ifndef GDCMDOCUMENT_H #define GDCMDOCUMENT_H diff --git a/src/gdcmElementSet.cxx b/src/gdcmElementSet.cxx index 36ebbd2b..e98a3446 100644 --- a/src/gdcmElementSet.cxx +++ b/src/gdcmElementSet.cxx @@ -70,7 +70,8 @@ bool gdcmElementSet::AddEntry( gdcmDocEntry *NewEntry) { if(tagHT.count(key) == 1) { - dbg.Verbose(1, "gdcmElementSet::AddEntry key already present: ", key.c_str()); + dbg.Verbose(1, "gdcmElementSet::AddEntry key already present: ", + key.c_str()); return(false); } else @@ -80,3 +81,20 @@ bool gdcmElementSet::AddEntry( gdcmDocEntry *NewEntry) { } } +/** + * \brief Clear the hash table from given entry. + * @param EntryToRemove Entry to remove. + */ +bool gdcmElementSet::RemoveEntry( gdcmDocEntry *EntryToRemove) +{ + TagKey key = EntryToRemove->GetKey(); + if(tagHT.count(key) == 1) + { + tagHT.erase(key); + dbg.Verbose(1, "gdcmElementSet::RemoveEntry: one element erased."); + return true; + } + + dbg.Verbose(0, "gdcmElementSet::RemoveEntry: key not present: "); + return(false); +} diff --git a/src/gdcmElementSet.h b/src/gdcmElementSet.h index a410629e..826b85cf 100644 --- a/src/gdcmElementSet.h +++ b/src/gdcmElementSet.h @@ -17,14 +17,13 @@ class GDCM_EXPORT gdcmElementSet : public gdcmDocEntrySet public: gdcmElementSet(int); ~gdcmElementSet(void); - virtual bool AddEntry(gdcmDocEntry *Entry); // add to the H Table + virtual bool AddEntry(gdcmDocEntry *Entry); + virtual bool RemoveEntry(gdcmDocEntry *EntryToRemove); - virtual void Print (std::ostream &os = std::cout); + virtual void Print(std::ostream &os = std::cout); protected: - // Variables - /// Hash Table (map), to provide fast access TagDocEntryHT tagHT; diff --git a/src/gdcmSQItem.cxx b/src/gdcmSQItem.cxx index a3d51616..2dd62176 100644 --- a/src/gdcmSQItem.cxx +++ b/src/gdcmSQItem.cxx @@ -2,6 +2,7 @@ //----------------------------------------------------------------------------- // #include "gdcmSQItem.h" +#include "gdcmSeqEntry.h" #include "gdcmGlobal.h" #include "gdcmUtil.h" #include "gdcmValEntry.h" @@ -27,8 +28,17 @@ gdcmSQItem::~gdcmSQItem() cc != docEntries.end(); ++cc) { - delete *cc; + gdcmDocEntry* DocEntry = *cc; + if ( gdcmSeqEntry* SeqEntry = dynamic_cast(DocEntry) ) + { + delete SeqEntry; + } + else + { + delete DocEntry; + } } + docEntries.clear(); } -- 2.48.1