Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,59 @@ jobs:
- name: Test
run: go test -count=1 -p 4 ./...

cross-build:
name: cross-build (32-bit and codec-less platforms)
runs-on: ubuntu-24.04
defaults:
run:
shell: bash
steps:
- uses: actions/checkout@v4
with:
submodules: recursive

- uses: actions/setup-go@v5
with:
go-version: '1.26'
cache: true

- name: Fetch multi-frame test data
run: bash scripts/fetch-testdata.sh

# godicom has to build everywhere Go does, not only on the six platforms
# where golibjpeg and goopenjpeg ship a native library. Those two return
# ErrUnsupportedPlatform at run time instead of failing to build, which is
# only worth anything if godicom builds there too.
#
# go vet rather than go build, so the _test.go files are type-checked as
# well: an int that should have been a uint32 is just as wrong in a test.
#
# 32-bit targets are the ones that earn their place here. A DICOM value
# length is unsigned 32 bits, so 0xFFFFFFFF does not fit in an int on a
# 32-bit platform, and forcing it in turns the undefined-length sentinel
# into -1. mips and mipsle are left out deliberately: purego does not
# build for them (undefined: addStruct), which is upstream, not ours.
- name: Build and vet where no prebuilt codec exists
run: |
set -euo pipefail
for target in \
windows/386 \
linux/386 linux/arm linux/riscv64 linux/ppc64le \
js/wasm wasip1/wasm; do
echo "== $target"
GOOS="${target%/*}" GOARCH="${target#*/}" CGO_ENABLED=0 go vet ./...
done
echo "== linux/arm GOARM=5"
GOOS=linux GOARCH=arm GOARM=5 CGO_ENABLED=0 go vet ./...

# Compiling is not enough. An int is one bit too narrow for a mask like
# 0xFF00FFFF, and that only shows when the code runs: every repeater mask
# parsed to zero, so every tag matched the first one and the entire data
# dictionary answered US. A static 386 binary runs on an amd64 kernel, so
# this is a real test run rather than another compile.
- name: Test on linux/386
run: GOARCH=386 CGO_ENABLED=0 go test -count=1 ./...

coverage:
runs-on: ubuntu-latest
steps:
Expand Down
53 changes: 53 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- **The data dictionary answered the wrong VR for nearly every tag on a 32-bit
platform.** The 88 repeater masks were parsed with `fmt.Sscanf` into `int`
fields with the error discarded; a mask such as `0xFF00FFFF` does not fit in a
32-bit `int`, so all 88 came out zero, and `(tag ^ value) & 0 == 0` matches
every tag. Whichever repeater happened to be first in map iteration order
became the answer for every non-private tag — `LookupVR` returned `US` for
`PatientName`, group lengths, everything — and `IsRepeaterTag` was true for all
of them. The masks are now built by nibble arithmetic into `uint32`, which has
no error to drop and no width to overflow. 64-bit platforms were unaffected
- **`ParseTag` could not parse any tag with a group of `0x8000` or above from its
hex-string form**, on every platform. It used `strconv.ParseInt(v, 16, 32)`,
and a signed 32-bit parse rejects everything from `0x80000000` up — so
`ParseTag("FFFEE000")` failed with `unknown tag keyword` even though the item,
item-delimiter and sequence-delimiter tags are exactly the ones godicom's own
encapsulation and sequence code is built from, and exactly the ones a
`JSONKey` round-trip is most likely to hit. Now `ParseUint`, matching
pydicom's `int(arg, 16)`, which has no width at all. The parenthesised form
`(FFFE,E000)` always worked, because it parses two 16-bit halves
- godicom now builds on 32-bit platforms at all. `encaps.Encapsulate`'s Basic
Offset Table overflow guard was written `total > (1<<32)-1`, which is exact in
pydicom because Python integers are arbitrary-precision but overflows `int` at
compile time on a 32-bit target

### Changed
- **BREAKING (internal-facing): the declared value length is carried as `uint32`
rather than `int`** through the header, deferred-read and sequence paths. A
DICOM length is unsigned 32 bits and `0xFFFFFFFF` is the undefined-length
sentinel; in an `int` on a 32-bit platform that sentinel is `-1`, so the code
would have compiled and then quietly misread every undefined-length sequence
and encapsulated pixel-data element. No exported signature changes, and two
`uint32(length)` casts at call sites went away
- `golibjpeg` and `goopenjpeg` are now v1.3.0, adding prebuilt libraries for
`darwin/amd64` and `windows/arm64` — six platforms each, up from four. Both
now also build everywhere else instead of failing to compile, and return an
error wrapping `ErrUnsupportedPlatform` from every entry point rather than
panicking, so importing godicom is safe on any platform Go targets
- Docs: the README documents platform support and binary size. Each binary
embeds one platform's codec libraries, never all twelve: a `linux/amd64`
`cmd/godicom` build is 11.0 MB, where embedding every library unconditionally
would make it 24.3 MB
- Docs: `CompressPixelData` listed its supported targets without HTJ2K, which
`pixels.EncodeFrame` has dispatched on for some time, and spelled JPEG-LS as
though it were one transfer syntax
- Docs: README covers the two diagnostics v0.28.0 added — `WriteOptions.OnDiagnostic`
was not mentioned at all, and the read section predated both the VR
disagreement kind and `Diagnostic.Path` naming the sequence item
Expand All @@ -18,6 +61,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
along the way; it showed the diagnostic's own message where the caller
actually gets it wrapped in `error writing dataset`

### Added
- CI cross-builds and vets `windows/386`, `linux/386`, `linux/arm` (including
`GOARM=5`), `linux/riscv64`, `linux/ppc64le`, `js/wasm` and `wasip1/wasm`, and
runs the whole test suite on `linux/386`. Compiling is not enough to catch a
width mistake — the repeater-mask bug above compiled fine and only showed when
run. `linux/mips` and `linux/mipsle` are excluded because purego does not
build for them yet
- Tests that need a native codec now skip, rather than fail, on a platform with
no prebuilt library, so a real failure is visible among them

## [0.28.0] - 2026-08-24

### Changed
Expand Down
62 changes: 62 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,68 @@ godicom readcopy <src> <dst> # read, write, re-read
| JPEG-LS | ✅ | ✅ |
| JPEG 2000 / HTJ2K | ✅ | ✅ |

JPEG, JPEG-LS, JPEG 2000 and HTJ2K are decoded and encoded by
[golibjpeg](https://github.com/godicom-dev/golibjpeg) and
[goopenjpeg](https://github.com/godicom-dev/goopenjpeg). Everything else in the
table — including RLE and Deflated — is pure Go with no native code involved.

## Platforms

godicom builds and runs anywhere Go does. The native codecs ship prebuilt
libraries for six platforms:

| Platform | JPEG / JPEG-LS / JPEG 2000 / HTJ2K |
|----------|------------------------------------|
| linux/amd64, linux/arm64 | ✅ |
| darwin/amd64, darwin/arm64 | ✅ |
| windows/amd64, windows/arm64 | ✅ |
| everything else | reading and writing work; compressed pixel data returns an error |

There is no cgo and no toolchain to install: the libraries are loaded through
[purego](https://github.com/ebitengine/purego), so a plain `go build` is all a
cross-compile takes.

On a platform without a prebuilt library, importing godicom is still safe.
Parsing, writing, the data dictionary, JSON, RLE and Deflated all work; only the
four native-codec transfer syntaxes fail, and they fail with an error rather than
a panic:

```go
if _, err := ds.PixelBytes(); errors.Is(err, golibjpeg.ErrUnsupportedPlatform) {
// No JPEG library for this GOOS/GOARCH. The dataset itself is fine.
}
```

CI builds every release for `windows/386`, `linux/386`, `linux/arm` (including
`GOARM=5`), `linux/riscv64`, `linux/ppc64le`, `js/wasm` and `wasip1/wasm`, and
runs the full test suite on 32-bit. `linux/mips` and `linux/mipsle` do not build,
because purego does not support them yet.

### Binary size

A binary carries one platform's libraries, never all six. The `//go:embed`
directives are behind per-platform build tags, so the linker only ever sees the
pair for the target you are building:

| `cmd/godicom`, `go build` | Size | Embedded libraries |
|---------------------------|------|--------------------|
| linux/amd64 | 11.0 MB | 3.7 MB |
| linux/arm64 | 10.4 MB | 3.4 MB |
| darwin/amd64 | 10.4 MB | 2.7 MB |
| darwin/arm64 | 9.8 MB | 2.3 MB |
| windows/amd64 | 10.5 MB | 2.6 MB |
| windows/arm64 | 9.8 MB | 2.4 MB |
| js/wasm | 8.3 MB | none |
| linux/386 | 6.4 MB | none |

All twelve libraries together are 17.0 MB, so embedding them unconditionally
would add about 13.3 MB to every binary — a linux/amd64 build would be 24.3 MB
instead of 11.0 MB. To confirm what your own build embeds:

```bash
go list -f '{{.EmbedFiles}}' github.com/godicom-dev/golibjpeg/native
```

## Contributing

Bug reports, fixes, and documentation improvements are welcome. Please open an
Expand Down
8 changes: 4 additions & 4 deletions deferred.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,26 @@ func dataElementOffsetToValue(isImplicit bool, vr VR) int64 {
return 8
}

func shouldDeferElement(tag Tag, length int, deferSize uint32) bool {
func shouldDeferElement(tag Tag, length uint32, deferSize uint32) bool {
if deferSize == 0 {
return false
}
// Never defer Specific Character Set (needed immediately for decoding).
if tag == TagCharset {
return false
}
return uint32(length) > deferSize
return length > deferSize
}

func markElementDeferred(
elem *Element,
valueTell int64,
length int,
length uint32,
cc codecContext,
) {
elem.Deferred = true
elem.ValueTell = valueTell
elem.ValueLength = uint32(length)
elem.ValueLength = length
elem.IsImplicitVR = cc.IsImplicitVR
elem.IsLittleEndian = cc.IsLittleEndian
elem.readCharsets = append([]string(nil), cc.Charsets...)
Expand Down
56 changes: 36 additions & 20 deletions dictionary.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,40 +110,56 @@ func dictionaryIsRetired(tag Tag) bool {
// Repeater masks: precomputed from the RepeatersDictionaryGo keys
type repeaterMask struct {
maskStr string
mask1 int
mask2 int
// A tag is 32 unsigned bits, so these are too. They were int, and int is 32
// bits wide on a 32-bit platform -- one bit short of holding a mask like
// 0xFF00FFFF. fmt.Sscanf failed there and left the field zero, which made
// (t^mask1)&mask2 == 0 true for every tag: the whole dictionary resolved
// through whichever repeater happened to be first.
value uint32 // the fixed digits, with each x as 0
mask uint32 // F where the digit is fixed, 0 where it may vary
}

var repeaterMasks []repeaterMask

func init() {
for maskStr := range RepeatersDictionaryGo {
// Convert "60xx3000" -> mask1, mask2
mask1Str := strings.ReplaceAll(maskStr, "x", "0")
mask2Str := ""
for _, c := range maskStr {
if c == 'x' {
mask2Str += "0"
} else {
mask2Str += "F"
}
}
mask1 := 0
mask2 := 0
fmt.Sscanf(mask1Str, "%x", &mask1)
fmt.Sscanf(mask2Str, "%x", &mask2)
value, mask := repeaterMaskBits(maskStr)
repeaterMasks = append(repeaterMasks, repeaterMask{
maskStr: maskStr,
mask1: mask1,
mask2: mask2,
value: value,
mask: mask,
})
}
}

// repeaterMaskBits turns a key like "60xx3000" into the pair maskMatch compares
// against. Shifting nibbles rather than building two strings and parsing them
// back means there is no error to drop on the floor and no platform-dependent
// width to overflow.
func repeaterMaskBits(maskStr string) (value, mask uint32) {
for _, c := range maskStr {
value <<= 4
mask <<= 4
if c == 'x' || c == 'X' {
continue
}
mask |= 0xF
switch {
case c >= '0' && c <= '9':
value |= uint32(c - '0')
case c >= 'a' && c <= 'f':
value |= uint32(c-'a') + 10
case c >= 'A' && c <= 'F':
value |= uint32(c-'A') + 10
}
}
return value, mask
}

func maskMatch(tag Tag) string {
t := int(tag)
t := uint32(tag)
for _, rm := range repeaterMasks {
if (t^rm.mask1)&rm.mask2 == 0 {
if (t^rm.value)&rm.mask == 0 {
return rm.maskStr
}
}
Expand Down
Loading
Loading