Compression support

[ThalesMMS]

No description provided.

[chatgpt-codex-connector]

Codex Review: Here are some suggestions.

Reply with @codex fix comments to fix any unresolved comments.

About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

[chatgpt-codex-connector]

[P0] Build encapsulated frame index using wrong BOT representation

The new FrameIndex.buildEncapsulatedFrameIndex treats pixelSequence.basicOffsetTable() as a raw Data buffer, dividing bot.count by 4 and accessing withUnsafeBytes. However the same commit changes PixelSequence.basicOffsetTable() to return [Int] offsets (one per frame). This mismatch prevents the project from compiling and, even if fixed by force-casting, would compute a quarter of the real frame count and use bot.count (the number of offsets) as the last frame’s byte length, yielding zero or negative sizes. The method should iterate the returned offset array directly and derive the last frame length from the fragment data size, not from the number of offsets.

Useful? React with 👍 / 👎.

[ThalesMMS]

@codex fix comments

[chatgpt-codex-connector]

Summary

• Updated FrameIndex.buildEncapsulatedFrameIndex to iterate the [Int] BOT offsets, clamp each frame to the concatenated fragment data, and continue supporting the single-frame fallback when the BOT is empty. Sources/DcmSwift/Data/FrameIndex.swiftL94-L127

Testing

• ⚠️ swift test (fails: platform lacks the os.signpost module needed by graphics utilities)

View task →

[ThalesMMS]

@claude

[claude]

Code Review: Compression Support PR #31

Thank you for this comprehensive implementation of compression support for DcmSwift! This is a substantial contribution that adds critical functionality for handling compressed DICOM files. Here's my detailed review:

🎯 Overall Assessment

This PR successfully implements support for multiple compression formats (RLE, JPEG-LS, JPEG 2000) with good test coverage. The implementation is well-structured with clear separation of concerns between different decoders.

✅ Strengths

1. Well-organized architecture: Each compression format has its own decoder module with clear responsibilities
2. Comprehensive test coverage: Good unit tests for individual components and integration tests
3. Performance optimizations: Memory-mapped I/O for large files, adaptive prefetching, and Metal acceleration
4. Error handling: Proper error types and propagation throughout the decoders

🔍 Code Quality & Best Practices

Positive Observations:

• Clean separation between different compression formats
• Good use of Swift's type system with dedicated error types
• Efficient memory management with reserved capacity for buffers

Areas for Improvement:

1. Force Unwrapping in RLEDecoder (Sources/DcmSwift/Graphics/RLEDecoder.swift:95)

   let control = Int(Int8(bitPattern: data[i])); i += 1

   Consider bounds checking before accessing data[i] to prevent potential crashes.

2. Magic Numbers: Several magic numbers could be extracted as named constants:

   • File size threshold (10MB) for memory mapping
   • Buffer sizes in various decoders
   • JPEG-LS context limits

3. Documentation: While code is generally clear, some complex algorithms (especially in JPEG-LS decoder) would benefit from more detailed comments explaining the mathematical operations.

🐛 Potential Issues

1. Memory Usage: The memory-mapped file optimization is great, but consider adding a configuration option to control the threshold (currently hardcoded at 10MB).

2. Thread Safety: The CompressedPixelRouter and related components don't appear to have explicit thread safety guarantees. Consider documenting thread safety requirements or adding synchronization if needed.

3. Error Recovery: In JPEGLSDecoder.ScanDecoder, errors during decoding could leave the decoder in an inconsistent state. Consider adding cleanup logic.

⚡ Performance Considerations

1. Good optimizations:

   • Memory-mapped I/O for large files
   • Cached parsed values for frequently accessed tags
   • Adaptive prefetching based on scroll velocity

2. Suggestions:

   • Consider lazy initialization for decoder instances to reduce startup overhead
   • The frequent tags cache could benefit from LRU eviction for very large DICOM files
   • Profile the JPEG-LS decoder - the bit-by-bit operations might benefit from buffering

🔒 Security Concerns

1. Input Validation: Good bounds checking in most places, but ensure all user-controllable inputs are validated:

   • Maximum segment counts in RLE
   • Image dimensions to prevent integer overflow in buffer allocations
   • Compressed data sizes to prevent excessive memory allocation

2. Resource Limits: Consider adding configurable limits for:

   • Maximum decompressed image size
   • Maximum number of frames to cache
   • Timeout for decompression operations

🧪 Test Coverage

The test coverage looks comprehensive with both unit and integration tests. Suggestions:

1. Add edge case tests for:

   • Corrupted compressed data
   • Maximum allowed image dimensions
   • Memory pressure scenarios

2. Consider adding performance benchmarks to track regression in decompression speed.

📝 Minor Issues

1. Inconsistent error messages: Some use print(), others use proper logging. Consider standardizing on a logging approach.

2. TODO comments: Several TODO comments should be tracked as issues:

   • JPEG-LS color support
   • HTJ2K implementation
   • Deflate support

3. Code style: Generally consistent, but some lines exceed reasonable length (especially in tests).

💡 Suggestions for Future Improvements

1. Progressive decoding: For large compressed images, consider implementing progressive decoding to show partial results quickly.

2. Caching strategy: The current byte-based cache limit is good, but consider adding time-based eviction for long-running processes.

3. Metrics: Add performance metrics/telemetry for compression format usage and decode times.

✅ Recommendation

This PR is ready to merge with the understanding that the minor issues identified can be addressed in follow-up PRs. The core functionality is solid, well-tested, and adds significant value to the library.

Consider creating GitHub issues for:

• Thread safety documentation/improvements
• Resource limit configuration
• Progressive decoding implementation
• Remaining compression format support (Deflate)

Great work on this substantial feature addition! 🎉

---

Review generated with assistance from Claude Code