[Dcmlib] Never trust the programer :)

[Jean-Pierre ROUX]

At 14:41 -0400 21/10/05, Mathieu Malaterre wrote:
>God ! I wish we never switch to speak english, it is making so much confusion.
>

[...]

>JP:
>So far the only mistake*s* (plural form) I corrected were on *our* 
>side. So blaming manufacturer all the time, is kind of lame. First 
>we should admit that our ACR-NEMA dict is not perfect, then correct 
>it.

I know there are probabely tons of misstyping in the VM : I added 
them manually, and nobody used them till now.

Anyway, there are very few entry we do care about the VM 
(orientation, position, spacing. any other ?)
I'm not sure it's very usefull to waste time during parsing to check 
the VM for ecah Entry :

Suppose you find an anti-slash in the Patient Birthdate.
'Official' VM is 1; VM on disk is 2;
Do you really want to abort the reading process?

For the *very few* entries (position, orientation) in which VM is 
important, we *know* what we're expecting, without having to check 
the VM.

For (at least) one entry (Pixel spacing), we know sometimes -in old 
images- the header is wrong, and we already coded the heuristic to 
bypass the problem.

Moreover, the VM value "1-n" doesn't mean 'any value between 1 and 
n', but 'a *given* value, depending on an other entry value'.
e.g. : in a multi frame image holding, say 125 frames, a given (I 
don't remember the tag but I can find it) '1-n' field *must* be 125 
long.
Any other value is wrong.
How could we check that automaticaly?

Even if this tag is missing (evil), a radiologist may look a the 
image to make a diagnostic, and save -hope so- the patient.
In this case, checking the VM would be the origin of the problem, not 
the solution !

If a mathematician wants to buid a volume, starting for n projections 
-not slices : projections- he will not be able to do the job.
His application will hang at the processing time -due to a special 
checking in the, say 'GetAngles()' method-, not at the reading time.


My suggestion is to keep a CheckVM (or check everything you want -one 
check per modality ?-) that could be run on demand (just like efilm 
'dicom dump', or David Clunie's dicom3tools), and not to perform the 
checks at reading time.

JP


 


> And then after blame the constructor :). Seriously none of the VM 
>for Orientation and Position were right... I mean com'on !
>
>
>But I do agree that my assert is too strong in particular for the 
>super duper 'special' case where Spacing is written with a VM of 3. 
>I'll have to think about it. But right now I am just going over the 
>ACR-NEMA dict which is IMHO not up to date at all.
>
>My 2 cents, sorry if I hurt your feelings :(
>Mathieu
>
>Jean-Pierre Roux wrote:
>>Mathieu Malaterre wrote:
>>
>>>Benoit,
>>>
>>>   I just added an assert for kick and it is seg faulting. For instance:
>>
>>
>>
>>Mathieu.
>>
>>Be carefull with the VM : I added them manualy in our dicomV3.dic.
>>Nobody used them till now ...
>>
>>And think of the bozos that produce supposed to be Dicom images.
>>We coded a lot of heuristics to allow gdcm Reader to go on when a 
>>duscrepancy is found (dicomV3.dic vs current Dicom Image -being 
>>read-)
>>ex : Pixel Spacing has a VM = 2;
>>We have images where a single value is found (assume the pixel is square)
>>We have images where 3 values are found (We know the middle one is 
>>always 0 ?!?, just ignore it)
>>If you check too much, you'll have more 'gdcm breaker images'.
>>
>>VM should be usefull to enforce consistency, no allowing user to 
>>*write* illegal stuff.
>>Or inside a checker, that would warn user the image is not kosker.
>>But *not* hanging gdcm.
>>IMO...
>>
>>JPRx
>>
>>>
>>> DataEntry *entry = GetDataEntry(0x0020,0x0032);
>>>  if( !entry )
>>>  {
>>>     gdcmWarningMacro( "Unfound Image Position Patient (0020,0032)");
>>>     entry = GetDataEntry(0x0020,0x0030);
>>>     if( !entry )
>>>     {
>>>        gdcmWarningMacro( "Unfound Image Position (RET) (0020,0030)");
>>>        return 0.0f;
>>>     }
>>>  }
>>>
>>>  if( entry->GetValueCount() == 3 )
>>>  {
>>>     gdcmAssertMacro( entry->IsValueCountValid() );
>>>     return (float)entry->GetValue(0);
>>>  }
>>>  return 0.0f;
>>>
>>>so if 0x0020,0x0032 is not found, you check 0x0020,0x0030, which 
>>>is then found. But if it is found THEN you compare 
>>>entry->GetValueCount() against 3, but how do you know that VM is 
>>>the same for 0020,0032 and 0020,0030 ?
>>>
>>>Anyway this could have been safe, unfortunately 0020,0030 was 
>>>entered as VM=1 in the dict.
>>>
>>>Question: Is this safe to let the programmer use GetValueCount ? 
>>>Or should we enforce IsValueCountValid ? Maybe an alternative 
>>>would be to run IsValueCountValid on the whole gdcmData just to 
>>>see how respectful the DICOM data are to 'our' dict...
>>>
>>>Mathieu
>>>_______________________________________________
>>>Dcmlib mailing list
>>>Dcmlib at creatis.insa-lyon.fr
>>>http://www.creatis.insa-lyon.fr/mailman/listinfo/dcmlib
>>>
>>
>>
>
>_______________________________________________
>Dcmlib mailing list
>Dcmlib at creatis.insa-lyon.fr
>http://www.creatis.insa-lyon.fr/mailman/listinfo/dcmlib

   Jean-Pierre ROUX
   CREATIS - CNRS UMR 5515, INSERM U 630
   Laboratoire de Radiologie Experimentale
   Hopital Cardiologique
   28 Avenue du Doyen LEPINE
   B.P. Lyon-Montchat
   69394 Lyon Cedex 03
 
   Tel      : (+33) 04 72 35 74 12
   Fax      : (+33) 04 72 68 49 16
   URL      : http://www.creatis.univ-lyon1.fr
   e-mail   : jpr at creatis.univ-lyon1.fr