Closed 9enki closed 4 months ago
Enet4
commented
7 months ago Thank you for bringing this up. It appears that the current iteration of openjp2 does not work on either Windows or macOS. #432
Please check whether the same mitigation strategy in #432 works for you.
cargo build --no-default-features --features=default_windowsor
cargo build --no-default-features --features=default_windows,openjpeg-sys
9enki
commented
7 months ago @Enet4 Thank you for your reply. I have updated Cargo.toml in the sample program as follows.
[package]
name = "dicom-rs-0-6-3-openjp2-test"
version = "0.1.0"
edition = "2021"
[dependencies]
dicom = { version = "0.6.3", default-features = false, features = ["default_windows", "openjpeg-sys"] }However, when I cargo build it, I get the following error.
error: failed to select a version for `dicom`.
... required by package `dicom-rs-0-6-3-openjp2-test v0.1.0 (/home/gnk/dicom-rs-0-6-3-openjp2-test)`
versions that meet the requirements `^0.6.3` (locked to 0.6.3) are: 0.6.3
the package `dicom-rs-0-6-3-openjp2-test` depends on `dicom`, with features: `default_windows` but `dicom` does not have these features.
failed to select a version for `dicom` which could resolve this conflictWould this work?
dicom = { version = "0.6.3", default-features = false, features = ["dicom-pixeldata/default_windows", "dicom-pixeldata/openjpeg-sys"] }Otherwise, the subdependencies can be added separately.
dicom-pixeldata = { version = "0.2.2", default-features = false, features = ["default_windows", "openjpeg-sys"] }In any case, the plan for DICOM-rs 0.7 is to make JPEG 2000 support opt-in regardless of platform, until there is a reliable cross-platform implementation.
alanwilter
commented
6 months ago @9enki I didn't have any problems to compile you dummy repo on my mac (M1).
However @Enet4, my project which is very simple, I don't even need pixeldata, although compiles fine in my Mac, it is failing on Windows (directly or via cross-compiling: same error):
Mine current Cargo.toml:
[[bin]]
name = "open-sight"
path = "src/main.rs"
[dependencies]
serde = { version = "1.0.193", features = ["derive"] }
csv = "1.3.0"
dicom = "0.6.3"
rayon = "1.8.0"
walkdir = "2.4.0"
chrono = "0.4.31"
clap = { version = "4.4.11", features = ["derive"] }@Enet4 I tried your suggestions but (I must say I'm very new to rust):
# Cargo.toml
dicom = { version = "0.6.3", default-features = false, features = ["dicom-pixeldata/default_windows", "dicom-pixeldata/openjpeg-sys"] }
$ cargo build --target x86_64-pc-windows-gnu --no-default-features --features=default_windows
error: failed to parse manifest at `/mnt/data/awilter/OpenSight/Cargo.toml`
Caused by:
feature `dicom-pixeldata/default_windows` in dependency `dicom` is not allowed to contain slashes
If you want to enable features of a transitive dependency, the direct dependency needs to re-export those features from the `[features]` table.The only piece of my code that uses dicom-rs library:
use dicom::dictionary_std::tags;
use dicom::object::OpenFileOptions;
...
fn extract_dicom_data(path: &Path) -> Result<DicomData, Box<dyn std::error::Error>> {
let obj = OpenFileOptions::new()
.read_until(tags::PIXEL_DATA)
.open_file(path)?;
let patient_id = obj.element_by_name("PatientID")?.to_str()?;
let image_laterality = obj.element_by_name("ImageLaterality")?.to_str()?;
let patient_sex = obj.element_by_name("PatientSex")?.to_str()?;
let patient_dob = obj.element_by_name("PatientBirthDate")?.to_str()?;
let content_date = obj.element_by_name("ContentDate")?.to_str()?;
let modality = obj.element_by_name("Modality")?.to_str()?;
let manufacturer = obj.element_by_name("Manufacturer")?.to_str()?;
let formatted_patient_dob = format_dicom_date(&patient_dob)?;
let formatted_content_date = format_dicom_date(&content_date)?;
let file_path = path
.canonicalize()?
.to_str()
.ok_or("Invalid file path")?
.to_string();
Ok(DicomData {
patient_id: patient_id.to_string(),
image_laterality: image_laterality.to_string(),
patient_sex: patient_sex.to_string(),
patient_dob: formatted_patient_dob,
content_date: formatted_content_date,
modality: modality.to_string(),
manufacturer: manufacturer.to_string(),
file_path,
})
}Anyway, the only way I got Windows to work was:
# Cargo.toml
dicom = { version = "0.6.3", default-features = false }
$ cargo build --target x86_64-pc-windows-gnucargo build --no-default-features alone is not working as I expected, it's like if Cargo.toml had priority of command arguments and this is not right IMO.
Do you have a nightly pre-build of dicom-rs v0.7 so I can test?
Enet4
commented
6 months ago @alanwilter In your case you might be better served grabbing the individual library components that you need.
dicom_object = "0.6"
dicom_dictionary_std = "0.6"Then in your code:
use dicom_dictionary_std::tags;
use dicom_object::OpenFileOptions;Do you have a nightly pre-build of dicom-rs v0.7 so I can test?
There is no such build yet. All upstream contributions can be seen in this repository. I can follow up on this once I have some spare time and energy. In the meantime, one could contribute with a pull request to exclude openjp2 by default.
alanwilter
commented
6 months ago Thanks @Enet4 that worked nicely.
BTW, I'm a python guy and sorry if I'm using the wrong channel (if you know so, please tell me), but how do I do to change extract_dicom_data() function so if element is not found (for PatientID, ImageLaterality, etc.) I still want to assign a empty string value in the end. For ImageLaterality actually, I search for Laterality as well and only defaulting to "" if neither were found (this is how I do with pydicom).
Now that I put my code to run I'm seeing lots of No such data element ImageLaterality (with tag (0020,0062)) but I still want to fill my output CSV files with empty results if needed.
Neopallium
commented
3 months ago FYI, I released 0.3.1 of openjp2 that should fix that compile error for non-linux platforms. Not sure why I didn't get the same error when compiling on Linux.
Update the openjp2 crate using:
cargo update -p openjp2
Enet4
commented
2 months ago @Neopallium Thank you for the update to openjp2! If I may ask, are there any other plans for long-term maintainability of the crate? Without an issue tracker at the corresponding fork it may be hard to follow up on future concerns, such as updating transitive dependencies or keeping the port updated with upstream OpenJPEG.
Neopallium
commented
2 months ago @Enet4 I hadn't thought about the repo not having an issue tracker. I just created a new repo for openjp2: https://github.com/Neopallium/openjp2
There is still a lot of work to refactor all the unsafe Rust in openjp2 (this will be a slow long-term process). But I am happy to fix any compile issues.
Build of
0.6.3fails on Mac M1.Environment
Mac Book Pro Apple M1
How to reproduce
We have created a repository with the minimum configuration to reproduce the defect. https://github.com/ikneg/dicom-rs-0-6-3-openjp2-test
On wsl2 ubuntu, the
docker compose buildsucceeds, but fails with the above Mac configuration.Error log