[Dcmlib] Re : GDCM Library: License (gdcm 0.4 patch)
Mathieu Malaterre
mathieu.malaterre at kitware.com
Fri May 6 15:53:42 CEST 2005
Mülleder,
Thanks for the feedbacks. All your changes looks good to me. I guess at
that time we did not had a Implicit VR - Big Endian (G.E Private) DICOM
image to play with. Sorry for the troubles of implementing it. If you
have a chance I'd really appreciate if you could give a try to gdcm 1.0
and see if it reads all of your DICOM files.
As a side note gdcm 0.4 was tagged (and not branched) which means that
we cannnot easily apply your changes. Instead we would rather backport
any of your modification to the main branch or at least gdcm 1.0.
Thanks again
Mathieu
Mathieu
Mülleder Ilona wrote:
> Hello GDCM developers,
>
> I am an Austrian student and I am in my final year of "Software Engineering for Medical Purposes". It is part of my studies to do an internship in the last year, which means that I had to do a project. My project was to implement a DicomReader to import Dicom files (images), etc.
>
> When you replied to my last email (where I was asking you questions about gdcm version 0.4), you mentioned that gdcm 0.4 comes with the LGPL license which means for me to send you back any patch I apply to this version.
>
> I needed to support the tranfer syntax 1.2.840.113619.5.2 (Implicit VR - Big Endian (G.E Private)), which was not supported in version 0.4. Also, I think there is a bug in the method SwapZone () in gdcmFile.cxx.
> ...
> for(i=0;i<lgr;i++)
> ((unsigned short int*)im)[i]= ((((unsigned short int*)im)[i])>>8)
> | ((((unsigned short int*)im)[i])<<8);
> ...
>
> Shouldn't lgr be equal to lgr / (nb/8) ? I didn't want to modify SwapZone () because maybe this function is working for other BigEndian files but I don't have such BigEndian dicom files for testing, so I can't say for sure if there's an implementation problem with BigEndian data in general. Does this version of gdcm read BigEndian data properly or was this possible bug ever reported by somebody else?
>
> So, to support the required transfer syntax, I applied the following changes:
>
> gdcmParser.h(72):
> bool IsImplicitVRBigEndianGETransferSyntax(void);
>
> gdcmParser.cxx(26):
> #define UI1_2_840_113619_5_2 "1.2.840.113619.5.2"
>
> gdcmParser.cxx(313):
> bool gdcmParser:: IsImplicitVRBigEndianGETransferSyntax(void) {
> gdcmHeaderEntry* Element = GetHeaderEntryByNumber(0x0002, 0x0010);
> if ( !Element )
> return false;
> LoadHeaderEntrySafe(Element);
> std::string Transfer = Element->GetValue();
> if ( Transfer == UI1_2_840_113619_5_2 )
> return true;
> return false;
> }
>
> gdcmFile.cxx(869):
> if (Header->IsImplicitVRBigEndianGETransferSyntax()) {
>
> size_t ItemRead;
> int nb;
> std::string str_nb = Header->GetEntryByNumber(0x0028,0x0100);
> // Number of Bits Allocated for storing a Pixel
> if (str_nb == GDCM_UNFOUND ) {
> nb = 16;
> } else {
> nb = atoi(str_nb.c_str() );
> }
>
> if (nb == 8) {
> // We don't need to swap
> ItemRead = fread(destination, Header->GetPixelAreaLength(), 1, fp);
> } else if (nb == 16) {
> // We swap because it's BigEndian
> // We do it here because :
> // 1) we can evaluate Header->IsImplicitVRBigEndianGETransferSyntax()
> // (because the file fp* is open)
> // 2) when doing it in SwapZone by modifying the sw (= Header->GetSwapCode ())
> // in the gdcmParser::CheckSwap, a crash occurs. Is SwapZone well written?
> // Shouldn't lgr be equal to lgr / (nb/8) ? I didn't want to modify SwapZone
> // because maybe this function is working for other BigEndian files and I
> // don't have such dicom files for testing, so I can't say that there's an
> // implementation pb : Does this version of gdcm read BigEndian data properly?
> ItemRead = fread(destination, Header->GetPixelAreaLength(), 1, fp);
>
> // lgrtotal should be equal to Header->GetPixelAreaLength(). Should we check ?
> for(int i=0;i<((int)Header->GetPixelAreaLength() / (nb/8));i++)
> ((unsigned short int*)destination)[i]= ((((unsigned short int*)destination)[i])>>8)
> | ((((unsigned short int*)destination)[i])<<8);
>
> } else {
>
> // Should we support nb == 32?
> Header->CloseFile();
> return false;
> }
>
> if ( ItemRead != 1 ) {
> Header->CloseFile();
> return false;
> } else {
> Header->CloseFile();
> return true;
>
> }
>
> }
>
>
> I hope that this email is sufficient to fulfil the agreement of the LGPL license.
> Also, I am considering to switch to a newer GDCM version, for the reasons we were talking about in the last email.
>
> Best regards,
>
> Ilona Muelleder
>
> ________________________________
>
> From: Mathieu Malaterre [mailto:mathieu.malaterre at kitware.com]
> Sent: Thu 27/01/2005 16:47
> To: Ilona.Mülleder at fh-hagenberg.at
> Cc: Mülleder Ilona
> Subject: Re: [Dcmlib] Re : GDCM Library: License
>
>
>
>
>
>>>1. You wrote that you solved a huge number of memory leaks. It would
>>>be interesting to know roughly where the memory leaks were? The
>>>classes I used for my project are gdcmDict, gdcmDictEntry,
>>> gdcmHeader, gdcmHeaderHelper,
>>> gdcmSerieHeaderHelper, gdcmFile (mainly for getting the
>>>image data).
>>>It would be very helpful if you could remember if there were any
>>>(major) memory leaks in one of those classes.
>
>
>
> Well I do not need to remember since we use CVS. CVS has much better
> memory than any of us, so if you are ready to backport avery single
> memory patch we did. You do it using basic CVS commands :
>
> cvs dif -r1.45 -r.1.46 gdcmFile.cxx
>
> This gives you the patch you need to apply to go from 1.45 to 1.46.
>
> Thefore you can work around the namespace change if you really want to
> keep your initial work.
>
>
>>>2. The class structure of the newest version compared to version 0.4
>>>(the one I have) is quite different. Also some of the classes I used
>>>are not part of GDCM any more.
>
>
>
> There is a good chance they have been renamed. Thankfully gdcm is
> distributed with nightly tests which shows -almost- every possible way
> to use the library to achieve different goals. All you need to do then
> is figure out which test best represent the goal you are seeking.
>
> So my question is, in future, are you
>
>>>planning to implement newer versions in a way that one could easily
>>>upgrade to the newer version without changing a lot of GDCM-related
>>>code, or is there not much guarantee that the class diagram (names and
>>>hierarchy) will stay more or less the same? This is good to know in
>>>order to estimate if it is worth upgrading to the newer version.
>
>
> Going from 0.4 to 1.0 (well CVS right now) is IMHO a serious need !
> There has been way to many changes to try to stay with 0.4 right now.
> After 1.0, gdcm should -hopefully- stabilize and the API would not be as
> murderer as it has been in between 0.4 to 1.0
>
>
>
>>>Maybe these questions have been answered before, if so, could you
>>>please point at the mailing list archive or the web resources?
>>>If the questions sound very new to you, then it would be very nice
>>>from you all, if you could provide answers for my important questions
>>>and I am looking forward to hearing from you.
>
>
> Unfortunaltey 99% of our Mailing list is in french. So unless you speak
> french you may not find the answers you are looking for.
>
>
> Also without going too much into the licensing issues. I would suggest
> you -seriously- read the license that comes with GDCM. gdcm 0.4 was
> release with LGPL license (*)
>
> http://www.gnu.org/copyleft/lgpl.html
>
> Which is IMHO quite restrictive. *In particular* you need to send us
> back any patch you applied to gdcm.
>
> Newer GDCM version are released with BSD like license which should allow
> you to redistribute the code (even modified) providing you keep the
> license. Also this is mandatory that you mark gdcm as being modified if
> you plan to modify it. BTW even if it not compulsory any feedback or
> patch is more than welcome :)
>
>
> I hope you won't discard the licensing issues.
> Mathieu
>
>
>
>
More information about the Dcmlib
mailing list