[Gdcm-hackers] [ gdcm-Bugs-3073386 ] ReadUpToTag more strict than before?

[SourceForge.net <no...@so...>]

Bugs item #3073386, was opened at 2010-09-22 08:13
Message generated for change (Comment added) made by malat
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=739587&aid=3073386&group_id=137895

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: General (default)
Group: 2.3
Status: Open
Resolution: None
Priority: 9
Private: No
Submitted By: Niels Dekker (niels_dekker)
Assigned to: Mathieu Malaterre (malat)
Summary: ReadUpToTag more strict than before?

Initial Comment:
It appears that the behavior of Reader::ReadUpToTag has recently changed. As I wrote in "[Gdcm2] Reader::ReadUpToTag more strict than before?":
----------------------------------------------------------------------
The 2.0.16 version of Reader::ReadUpToTag fails to read the following files:

 DMCPACS_ExplicitImplicit_BogusIOP.dcm
 ExplicitVRforPublicElementsImplicitVRforShadowElements.dcm
 LIBIDO-16-ACR_NEMA-Volume.dcm

While the old GDCM 2.0.12 version did actually support those files. Is that 
an intended behavior change?

I think it's caused by the following SVN revision:

  Revision: 6492
  Author: malat
  Date: 19:48:58, Friday, January 08, 2010
  Message:
  Synchronize Read / readuptotag
  ----
  Modified : /trunk/Source/DataStructureAndEncodingDefinition/gdcmReader.cxx

I think so, because I encountered a throw "Virtual Big Endian Implicit is 
not defined by DICOM", while trying to read those files.
----------------------------------------------------------------------
From: https://sourceforge.net/mailarchive/message.php?msg_id=7DBE4F292C3E41CE927311E40B9742DA%40lumcnet.prod.intern

Please either restore the old behavior, or explain to me why the new behavior is the better one  :-)

----------------------------------------------------------------------

>Comment By: Mathieu Malaterre (malat)
Date: 2012-06-19 00:48

Message:
Another update: ExplicitVRforPublicElementsImplicitVRforShadowElements.dcm
is working as it should:

./bin/gdcmDSEDTests TestReaderUpToTag
../gdcm/Testing/Data/ExplicitVRforPublicElementsImplicitVRforShadowElements.dcm
{
"../gdcm/Testing/Data/ExplicitVRforPublicElementsImplicitVRforShadowElements.dcm",9538
},


DMCPACS_ExplicitImplicit_BogusIOP.dcm in theory should be working. However
it does not:


./bin/gdcmDSEDTests TestReaderUpToTag
../gdcm/Testing/Data/DMCPACS_ExplicitImplicit_BogusIOP.dcm

Warning: In
/home/mathieu/Perso/gdcm/Source/DataStructureAndEncodingDefinition/gdcmReader.cxx,
line 544, function <unknow>
Attempt to read file with VR16bits


Warning: In
/home/mathieu/Perso/gdcm/Source/DataStructureAndEncodingDefinition/gdcmVR16ExplicitDataElement.txx,
line 124, function <unknow>
Assuming 16 bits VR for Tag=(0008,0080) in order to read a buggy DICOM
file.


Warning: In
/home/mathieu/Perso/gdcm/Source/DataStructureAndEncodingDefinition/gdcmVR16ExplicitDataElement.txx,
line 124, function <unknow>
Assuming 16 bits VR for Tag=(3f3f,083f) in order to read a buggy DICOM
file.


Warning: In
/home/mathieu/Perso/gdcm/Source/DataStructureAndEncodingDefinition/gdcmVR16ExplicitDataElement.txx,
line 124, function <unknow>
Assuming 16 bits VR for Tag=(b500,b700) in order to read a buggy DICOM
file.


{ "../gdcm/Testing/Data/DMCPACS_ExplicitImplicit_BogusIOP.dcm",66618 },


Since we reach a tag (b500,b700) readuptotag simply stops. However
(b500,b700) is not a tag. Had we read just another tag gdcm would have
realised the parsing was wrong. This file is technically possible to handle
but somewhat hard to detect.

Finally support for LIBIDO-16-ACR_NEMA-Volume.dcm has been removed from
GDCM long time ago. Those type of file should not occur "in the wild"
anymore.

Anyway in summary:
- ExplicitVRforPublicElementsImplicitVRforShadowElements.dcm works as
expected
- LIBIDO-16-ACR_NEMA-Volume.dcm is not supported by GDCM 2.2.x anymore
- Behavior for DMCPACS_ExplicitImplicit_BogusIOP.dcm should be tweaked

----------------------------------------------------------------------

Comment By: Niels Dekker (niels_dekker)
Date: 2011-06-23 08:21

Message:
Hi Mathieu. You wrote: "I think the main issue is fixed." So is the latest
version (git, master branch) able to read those test files, just like GDCM
2.13 did? 

I mean the following test files, from gdcmData:
  DMCPACS_ExplicitImplicit_BogusIOP.dcm
  ExplicitVRforPublicElementsImplicitVRforShadowElements.dcm
  LIBIDO-16-ACR_NEMA-Volume.dcm


----------------------------------------------------------------------

Comment By: Mathieu Malaterre (malat)
Date: 2011-06-23 07:17

Message:
I think the main issue is fixed.

----------------------------------------------------------------------

Comment By: Mathieu Malaterre (malat)
Date: 2010-09-22 08:58

Message:
Adding my own comments:

I am not sure about the exception you are talking about. I did not get
it when using DMCPACS_ExplicitImplicit_BogusIOP.dcm.

Anyway, when evaluating gdcm::Scanner please note that this class
requires valid DICOM file. In order to get it working we need to redo
some low level implementation details.
The way I envision gdcm::Scanner initially was that an institution
would be running a gdcmconv deamon as proxy in between the outside
world and the inside of the institution. In this case this would make
sure that the file is:
1) Valid and does not fool gdcm::scanner implementation in thinking it
reached passed the last possible tag
2) The tag are properly ordered.

1) is solvable with some effort. 2) is not in the general case. You
should definitely put that in your design.

BTW the reason why I think that bogus files are fooling gdcm:Scanner is
twofold:
* convert the file using gdcmconv
DMCPACS_ExplicitImplicit_BogusIOP.dcm clean.dcm
* try: gdcmscanner -t 0008,0030 -t 0028,0004 -t ffe0,0000 ...

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=739587&aid=3073386&group_id=137895