While fixing a Windows link failure in BRAINSTools, I found the same vendored copy of itk::DCMTKFileReader in four independent projects, all of them in namespace itk with ITK’s original class names. Three are actively maintained. I’d like input on whether ITK should absorb this need, and if so, in what form.
Background
ITK commit
db492d9b092,
“ENH: Make ITKDCMTK a private dependency of ITKIODCMTK”, moved the
header:
ITK 5.4 : Modules/IO/DCMTK/include/itkDCMTKFileReader.h <- public, symbols exported
ITK main : Modules/IO/DCMTK/src/itkDCMTKFileReader.h <- private
That change was right, and I am not proposing to revert it. The header hard-includes nine DCMTK headers (dcdict.h, dcdicent.h, dcxfer.h, dcvrds.h, dcstack.h, dcdatset.h, dcitem.h, dcvrobow.h, dcsequen.h). Privatising it took DCMTK’s include directories out of
The public interface of ITKIODCMTK, which every consumer of that module benefits from.
But DCMTKFileReader was, in practice, a used API. When it went private,
consumers copied it rather than losing it.
What the copies do
Each copy keeps the class in namespace itk with the original names (DCMTKItem, DCMTKSequence, DCMTKFileReader), and each ships a .cxx, so the symbols are strong, not weak template instantiations. Against ITK 5.4 — where ITK still exports those same symbols — that is two definitions of one mangled name.
ELF and Mach-O resolve it silently in favour of the object file, so only MSVC diagnoses it. In BRAINSTools’ case that surfaced as LNK2005 on Slicer’s Windows build (Slicer#9316). The ODR violation is present on every platform regardless, and if a vendored copy ever drifts from ITK’s, the silent-wrong-behaviour case replaces the link error.
Most of the copies also carry this shim:
#ifndef ITKIODCMTK_EXPORT
# define ITKIODCMTK_EXPORT
#endif
which strips __declspec(dllimport) and is what promotes a redeclaration into a duplicate definition on MSVC.
The evidence
Found via gh search code on filename and on class name, then filtering
out ITK forks and installed-ITK trees:
| Project | Location | Last push | Status |
|---|---|---|---|
Slicer/Slicer |
Modules/CLI/PETStandardUptakeValueComputation/ |
2026-08-12 | active |
QIICR/Slicer-PETDICOMExtension |
SUVFactorCalculatorCLI/ |
2026-03-13 | active |
zyq1569/starviewer |
PacsViewer/src/core/ |
2026-05-04 | active |
QIICR/Slicer-SUVFactorCalculator |
repo root | 2016-05-13 | dormant |
BRAINSia/BRAINSTools |
DWIConvert/ |
2026-08-13 | fixed |
BRAINSTools resolved its copy in BRAINSia/BRAINSTools#622 by renaming to brains::DCMTKFileReader — 56 exact mangled-symbol collisions against libITKIODCMTK-5.4 went to 0. That fixes the link error but keeps the duplicated code, which is what prompts this post. After this was done, I started thinking that all the duplication and all the separate implementations may not be the best option.
Five Slicer forks and archives inherit Slicer’s copy. They are not independent consumers but they do inherit the defect.
starviewer is the data point I find most telling: a Qt PACS viewer with no Slicer lineage. The copy has propagated outside the family that created it.
Method caveats. gh search code only indexes default branches of public repositories. Private repos, GitLab, non-default branches, and tarball redistribution are invisible to it. Four is a floor.
What these consumers actually need
The used surface is narrow and, importantly, almost entirely free of DCMTK types in its signatures:
SetFileName()/LoadFile()- roughly twenty
GetElement*(group, element, target)typed accessors —
GetElementDS,GetElementFD,GetElementCS,GetElementUS, … —
takingunsigned shortand fillingstd::string,double,int GetElementSQ()/GetElementItem(), which returnDCMTKSequence/
DCMTKItem
There is a pattern in who these consumers are. Three of the four are PET/SUV tools, and SUV factor calculation needsRadiopharmaceuticalInformationSequence. DWIConvert needs Siemens, Philips, and GE private sequences. Every known consumer needs sequence
traversal.
That matters because ITK already exposes the flat-tag half of this publicly and without DCMTK. DCMTKFileReader::PopulateMetaDataDictionary()emits GDCM-compatible "gggg|eeee" keys for every element, andDCMTKImageIO already calls it (itkDCMTKImageIO.cxx:481), so the result is reachable through itk::ImageIOBase::GetMetaDataDictionary(). Its one structural gap is exactly the thing every consumer needs:
if (vr == EVR_SQ)
{
// Sequences are nested datasets, not byte arrays; getUint8Array() does
// not return their content. Skip rather than emit an empty entry.
continue;
}
Options
A. Represent nested sequences in the MetaDataDictionary.
No new public class, no new public dependency, and it works through an API that is already public and already DCMTK-free. Consumers move to GetMetaDataDictionary() and delete their copies. The design question is how to key nested items — GDCM-style flattening, or a nested dictionary object.
B. Republish DCMTKFileReader as a supported public class, using pimpl. An opaque std::unique_ptr<Impl> keeps all nine DCMTK headers out of the public header, so DCMTK stays a private dependency of ITKIODCMTK and db492d9b092’s benefit is preserved. Adding a public class is additive API — it costs downstream nothing and needs no version bump. The real cost is the long-term maintenance commitment.
C. Do nothing, and document it. State plainly that the reader is private, that copying it into namespace itk is an ODR violation, and that consumers who copy it must rename. Cheapest, and defensible at a population of four — but it leaves every consumer maintaining a private
fork of ITK code that reads DICOM.
On naming, if B is chosen
If the class is republished it should be itk::DCMTKFileReader — the same name, in the same namespace, as ITK 5.4.
Downstreams have to build against 5.4 and 6.x simultaneously for years. An identical spelling means their code compiles unchanged against both and the vendored copies are simply deleted. Any new namespace forces #if ITK_VERSION_MAJOR into every consumer, which is a good way to persuade people to keep their copy instead.
I considered itk::Detail and rejected it. ITK already usesnamespace detail in seven ITK-proper headers (itkMathDeterminant.h, itkSymmetricEigenAnalysis.h, itkVectorContainer.h, …) for in-header SFINAE and instantiation helpers. Its established meaning here is “no stability guarantee, do not name this type” — the opposite of what we would be asking four projects to rely on. A itk::Utility namespace has no ITK precedent at all; our sub-namespaces are domain-scoped (Statistics, Math, Function, Testing, Concept), not
organisation-scoped.
Questions
- Is a population of four — three active, one outside the Slicer family — enough to justify ITK owning this? I can argue either way.
- If yes: option A or option B?
- For A, what is the right dictionary encoding for nested sequences? Is there prior art in
GDCMImageIOworth matching so the two IO backends agree? - Does anyone know of any consumers I missed? The search method cannot see private or non-GitHub code.
I have not opened any PRs against Slicer, the QIICR extensions, or ITK. Happy to do the work in whichever direction this lands.