Page MenuHomePhabricator

ITKImageIO should handle images with ArbitraryTimeGeometry
Closed, ResolvedPublic

Description

ITKImageIO should be extended that it handles ArbitraryTimeGeometries (especially for NRRD).

Best way is to encode the information as MetaData and pass it to the internal itk io. The itk io will store the data if possible.

To encode it will be checked if the needed MetaDatas are available after loading.
If the MetaData is available, the time geometry will be initialized appropriatly.

Event Timeline

User floca has pushed new remote branch:

bug-19438-Arbitrary-timegeometry-support-in-ITKImageIO

Hi Ralf,

the changes look good so far, thank you.

Could you please extend the mitkItkImageIOTest so that writing an image with an ArbitraryTimeGeometry is tested as well?

And I have one question:
Is the geometry information only in case of nrrd images persisted or does this apply to all ITK-supported file formats? If not we should think about some notification mechanism for telling the user that the timesteps are not persisted and will be lost.

Another point, which is more a subject of discussion and not necessarily a blocker for this bug:
We have several ITK-based image writers, which write their custom nrrd header, like you are doing it. (Examples: DiffusionImageIO, LabelSetImageIO)
It might be confusing, that in one case, ArbitraryTimeGeometry information is persisted and in other cases this information is ignored.

I suggest we should talk about that in the next MITK meeting.

Hi Ralf,

Test extension: I will do so.

ITK-support: Technically it is not limited to nrrd. It depends on the implementation of the specific itk reader/writer. If the implementation stores/loads information of the image meta data dictionary, it will automatically handle the time geometry as well.
But there is no systematic was to reflect this capability via runtime, e.g. by an interface. So for a warning feature so only option you have is to build an heuristic: checking the implementation of all reader/writer and making a white or a black list.
But even if we want to fix it, I would propose to open an other bug. Because we also would have the same proplem with the old implementation as well, thus if you have a ProportionalTimeGeometry with a duration != 1 ms the information will get lost, when you store it.

multiple *ImageIO: Yes we should discuss that. It makes sens, that they have the same behaviour/feature on such a basic level (escpecially LabelSetImageIO, because 3D+t is a probable situation there). I think the best would be to generalize in a different bug that they can use the same ressources.

Dealing with these issues in separate bugs is ok. We should then add dependencies to this bug.

Regarding the heuristic, we could for now add some documentation to the mitkItkImageIO, which states that the timestep information is only persisted for certain file formats.

In addition I think we should add the information about the time step duration of the ProportionalTimeGeometry to the meta dictionary as well. Otherwise the behaviour would be inconsistent. This can also be done in a separate bug.

Just re-request the core change flag after you finished the test.

One final remark:
The branch you pushed does not compile since the changes for T17819 are missing.
Although this makes it easier to review the changes, it adds a commit to the history which fails to built. This could be problematic e.g. if you use git bisect.
For the final commit it would therefore be great if you could checky-pick your commit on a new branch that has T17819 already included and hence does not lead to build errors.

User floca has pushed new remote branch:

bug-19438-integration-branch

Added test as asked by Andreas.

The other remarks/ideas given are added as bug (#19526) or forwarded for the internal discussion.

Hi Ralf,

thanks for adding the test.
I noticed that the test just checks whether the file was successfully saved / loaded but not whether loaded data is valid. I expected the test to check at least if the image's geometry is of type "ArbitraryTimeGeometry". Ideally it should also check, whether the timepoints are correct.

And could you please add documentation to the mitkItkImageIO.h which clarifies that in case of arbitraryTimeGeometries, the geometry information is just serialized if the image is saved as NRRD.

And can you please just file a new bug for inconstisten custom nrrd headers?

Thanks!

[8319e1]: Merge branch 'bug-19438-integration-branch'

Merged commits:

2016-02-04 00:20:27 Ralf Floca [b038d7]
reworked test and added checks for the timegeometries.

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>


2016-02-03 14:53:02 Ralf Floca [c78ed4]
changed MITKData to new test data hash

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>


2016-02-02 21:34:50 Ralf Floca [995d32]
added tests for mitkItkImageIoTest to check different geometry types.

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>


2015-11-13 14:23:10 Ralf Floca [9531bd]
fixed errror due to wrong usage of MetaDictionary

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>


2015-11-12 17:03:32 Ralf Floca [58c565]
changed type for string constants to avoid segfaults under linux while autoloading

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>


2015-11-12 15:06:28 Ralf Floca [994809]
fixed error in timepoint streaming; added mitk prefix to nrrd keys/property names

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>


2015-11-11 23:52:24 Ralf Floca [28f5d7]
patch for T19438

Signed-off-by: Ralf Floca <r.floca@dkfz-heidelberg.de>