From 62d3ae01cf3fb88632285af81065a3cf2cd49397 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sun, 2 Oct 2022 17:13:30 +0100 Subject: [PATCH 01/12] [pixeldata] add transcode module - `Transcode` trait can convert objects inline between transfer syntaxes - expose `Transcode` and error/result types --- pixeldata/src/lib.rs | 2 + pixeldata/src/transcode.rs | 194 +++++++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+) create mode 100644 pixeldata/src/transcode.rs diff --git a/pixeldata/src/lib.rs b/pixeldata/src/lib.rs index 0b8ac718b..dcc9f00ee 100644 --- a/pixeldata/src/lib.rs +++ b/pixeldata/src/lib.rs @@ -132,6 +132,7 @@ pub use ndarray; mod attribute; mod lut; +mod transcode; pub mod encapsulation; pub(crate) mod transform; @@ -139,6 +140,7 @@ pub(crate) mod transform; // re-exports pub use attribute::{PhotometricInterpretation, PixelRepresentation, PlanarConfiguration}; pub use lut::{CreateLutError, Lut}; +pub use transcode::{Error as TranscodeError, Result as TranscodeResult, Transcode}; pub use transform::{Rescale, VoiLutFunction, WindowLevel, WindowLevelTransform}; #[cfg(feature = "gdcm")] diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs new file mode 100644 index 000000000..69b9ff374 --- /dev/null +++ b/pixeldata/src/transcode.rs @@ -0,0 +1,194 @@ +use dicom_core::{ + smallvec::smallvec, DataDictionary, DataElement, DicomValue, Length, PrimitiveValue, VR, +}; +use dicom_dictionary_std::tags; +use dicom_encoding::{adapters::EncodeOptions, Codec, TransferSyntax, TransferSyntaxIndex}; +use dicom_object::{mem::InMemFragment, FileDicomObject, InMemDicomObject}; +use dicom_transfer_syntax_registry::{entries::EXPLICIT_VR_LITTLE_ENDIAN, TransferSyntaxRegistry}; +use snafu::{ensure, OptionExt, ResultExt, Snafu}; + +use crate::PixelDecoder; + +#[derive(Debug, Snafu)] +pub struct Error(InnerError); + +/// An error occurred during the object transcoding process. +#[derive(Debug, Snafu)] +pub(crate) enum InnerError { + /// Unrecognized transfer syntax of receiving object ({ts}) + UnknownSrcTransferSyntax { ts: String }, + + /// Unsupported target transfer syntax + UnsupportedTransferSyntax, + + /// Could not decode pixel data of receiving object + DecodePixelData { source: crate::Error }, + + /// Could not encode pixel data to target encoding + EncodePixelData { + source: dicom_encoding::adapters::EncodeError, + }, + + /// Unsupported bits per sample ({bits_allocated}) + UnsupportedBitsAllocated { bits_allocated: u16 }, + + /// Encoding multi-frame objects is not implemented + MultiFrameEncodingNotImplemented, +} + +/// Alias for the result of transcoding a DICOM object. +pub type Result = std::result::Result; + +/// Interface for transcoding a DICOM object's pixel data +/// to comply with a different transfer syntax. +pub trait Transcode { + /// Convert the receiving object's transfer syntax + /// to the one specified in `ts`, + /// replacing one or more attributes to fit the intended transfer syntax, + /// including the meta group specifying the transfer syntax. + /// + /// If the receiving object's pixel data is encapsulated, + /// the object is first decoded into native pixel data. + /// In case of an encoding error, + /// the object may be left with this intermediate state. + fn transcode(&mut self, ts: &TransferSyntax) -> Result<()>; +} + +impl Transcode for FileDicomObject> +where + D: Clone + DataDictionary, +{ + fn transcode(&mut self, ts: &TransferSyntax) -> Result<()> { + let current_ts_uid = &self.meta().transfer_syntax; + // do nothing if the transfer syntax already matches + if current_ts_uid == ts.uid() { + return Ok(()); + } + + // inspect current object TS + let current_ts = TransferSyntaxRegistry + .get(current_ts_uid) + .with_context(|| UnknownSrcTransferSyntaxSnafu { + ts: current_ts_uid.to_string(), + })?; + + if ts.is_codec_free() { + if current_ts.is_codec_free() { + // no pixel data conversion is necessary: + // change transfer syntax and return + self.meta_mut().set_transfer_syntax(ts); + Ok(()) + } else { + // decode pixel data + let decoded_pixeldata = self.decode_pixel_data().context(DecodePixelDataSnafu)?; + + // apply change to pixel data attribute + match decoded_pixeldata.bits_allocated { + 8 => { + // 8-bit samples + let pixels = decoded_pixeldata.data().to_vec(); + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OW, + Length::defined(pixels.len() as u32), + PrimitiveValue::from(pixels), + )); + } + 16 => { + // 16-bit samples + let pixels = decoded_pixeldata.data_ow(); + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OW, + Length::defined(pixels.len() as u32 * 2), + PrimitiveValue::U16(pixels.into()), + )); + } + _ => { + return UnsupportedBitsAllocatedSnafu { + bits_allocated: decoded_pixeldata.bits_allocated, + } + .fail()? + } + }; + + // change transfer syntax and return + self.meta_mut().set_transfer_syntax(ts); + Ok(()) + } + } else { + // must decode then encode + let adapter = match ts.codec() { + Codec::PixelData(adapter) => adapter, + Codec::EncapsulatedPixelData => return UnsupportedTransferSyntaxSnafu.fail()?, + Codec::None | Codec::Unsupported | Codec::Dataset(_) => { + unreachable!("Unexpected codec from transfer syntax") + } + }; + + // decode pixel data + let decoded_pixeldata = self.decode_pixel_data().context(DecodePixelDataSnafu)?; + let bits_allocated = decoded_pixeldata.bits_allocated(); + let number_of_frames = decoded_pixeldata.number_of_frames(); + + // note: there currently not a clear way + // to encode multiple fragments, + // so we stop if the image has more than one frame + ensure!(number_of_frames == 1, MultiFrameEncodingNotImplementedSnafu); + + // apply change to pixel data attribute + match bits_allocated { + 8 => { + // 8-bit samples + let pixels = decoded_pixeldata.data().to_vec(); + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OW, + Length::defined(pixels.len() as u32), + PrimitiveValue::from(pixels), + )); + } + 16 => { + // 16-bit samples + let pixels = decoded_pixeldata.data_ow(); + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OW, + Length::defined(pixels.len() as u32 * 2), + PrimitiveValue::U16(pixels.into()), + )); + } + _ => return UnsupportedBitsAllocatedSnafu { bits_allocated }.fail()?, + }; + + // change transfer syntax to Explicit VR little endian + self.meta_mut() + .set_transfer_syntax(&EXPLICIT_VR_LITTLE_ENDIAN); + + // use RWPixel adapter API + let mut pixeldata = Vec::new(); + let options = EncodeOptions::new(); + adapter + .encode(&*self, options, &mut pixeldata) + .context(EncodePixelDataSnafu)?; + + // put everything in a single fragment + let pixel_seq = DicomValue::, InMemFragment>::PixelSequence { + offset_table: smallvec![], + fragments: smallvec![pixeldata], + }; + + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OB, + Length::UNDEFINED, + pixel_seq, + )); + + // change transfer syntax + self.meta_mut().set_transfer_syntax(ts); + + Ok(()) + } + } +} From ca5ea857e7d46009f29b460e7a89affb3c670ae2 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Wed, 5 Oct 2022 15:52:55 +0100 Subject: [PATCH 02/12] [pixeldata] update multi-frame attributes on transcode --- pixeldata/src/transcode.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 69b9ff374..27639f7a6 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -1,5 +1,5 @@ use dicom_core::{ - smallvec::smallvec, DataDictionary, DataElement, DicomValue, Length, PrimitiveValue, VR, + smallvec::smallvec, DataDictionary, DataElement, DicomValue, Length, PrimitiveValue, Tag, VR, }; use dicom_dictionary_std::tags; use dicom_encoding::{adapters::EncodeOptions, Codec, TransferSyntax, TransferSyntaxIndex}; @@ -24,6 +24,9 @@ pub(crate) enum InnerError { /// Could not decode pixel data of receiving object DecodePixelData { source: crate::Error }, + /// Could not read receiving object + ReadObject { source: dicom_object::Error }, + /// Could not encode pixel data to target encoding EncodePixelData { source: dicom_encoding::adapters::EncodeError, @@ -172,6 +175,8 @@ where .encode(&*self, options, &mut pixeldata) .context(EncodePixelDataSnafu)?; + let total_pixeldata_len = pixeldata.len() as u64; + // put everything in a single fragment let pixel_seq = DicomValue::, InMemFragment>::PixelSequence { offset_table: smallvec![], @@ -185,6 +190,26 @@ where pixel_seq, )); + self.put(DataElement::new( + tags::NUMBER_OF_FRAMES, + VR::IS, + PrimitiveValue::from("1"), + )); + + // replace Encapsulated Pixel Data Value Total Length + // if it is present + if self + .element_opt(Tag(0x7FE0, 0x0003)) + .context(ReadObjectSnafu)? + .is_some() + { + self.put(DataElement::new( + Tag(0x7FE0, 0x0003), + VR::UV, + PrimitiveValue::from(total_pixeldata_len), + )); + } + // change transfer syntax self.meta_mut().set_transfer_syntax(ts); From 98a1bc526987079e45bee9d046a51aa00cae3203 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Wed, 5 Oct 2022 15:53:06 +0100 Subject: [PATCH 03/12] [pixeldata] add some test coverage for transcode --- pixeldata/src/transcode.rs | 68 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 27639f7a6..06b8122f0 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -217,3 +217,71 @@ where } } } + +#[cfg(test)] +mod tests { + use super::*; + use dicom_object::open_file; + use dicom_test_files; + + #[test] + fn test_transcode_from_jpeg_to_native_rgb() { + let test_file = dicom_test_files::path("pydicom/SC_rgb_jpeg_gdcm.dcm").unwrap(); + let mut obj = open_file(test_file).unwrap(); + + // pre-condition check: pixel data conversion is needed here + assert_ne!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + + // transcode to explicit VR little endian + obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) + .expect("Should have transcoded successfully"); + + // check transfer syntax + assert_eq!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + + // check that the pixel data is in its native form + // and has the expected size + let pixel_data = obj.element(tags::PIXEL_DATA).unwrap(); + let pixels = pixel_data + .to_bytes() + .expect("Pixel Data should be in bytes"); + + let rows = 100; + let cols = 100; + let spp = 3; + + assert_eq!(pixels.len(), rows * cols * spp); + } + + #[test] + // Test ignored until 12-bit JPEG decoding is supported + #[ignore] + fn test_transcode_from_jpeg_to_native_16bit() { + let test_file = dicom_test_files::path("pydicom/JPEG-lossy.dcm").unwrap(); + let mut obj = open_file(test_file).unwrap(); + + // pre-condition check: pixel data conversion is needed here + assert_ne!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + + // transcode to explicit VR little endian + obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) + .expect("Should have transcoded successfully"); + + // check transfer syntax + assert_eq!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + + // check that the pixel data is in its native form + // and has the expected size + let pixel_data = obj.element(tags::PIXEL_DATA).unwrap(); + let pixels = pixel_data + .to_bytes() + .expect("Pixel Data should be in bytes"); + + let rows = 1024; + let cols = 256; + let spp = 3; + let bps = 2; + + assert_eq!(pixels.len(), rows * cols * spp * bps); + } +} From ff081be28fae591ce4df57536ec1186b9a9e368f Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Wed, 5 Oct 2022 15:59:34 +0100 Subject: [PATCH 04/12] [pixeldata] Fix transcodeError documentation --- pixeldata/src/transcode.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 06b8122f0..80db9cd5b 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -9,10 +9,10 @@ use snafu::{ensure, OptionExt, ResultExt, Snafu}; use crate::PixelDecoder; +/// An error occurred during the object transcoding process. #[derive(Debug, Snafu)] pub struct Error(InnerError); -/// An error occurred during the object transcoding process. #[derive(Debug, Snafu)] pub(crate) enum InnerError { /// Unrecognized transfer syntax of receiving object ({ts}) From 0fe0644738a08679389422f34443583b3a811aa2 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 12:19:50 +0100 Subject: [PATCH 05/12] [pixeldata] Further adjust transcoder module - add root module documentation - add `Transcode::transcode_with_options` and add default impl for `Transcode::transcode` - fix implementation according to v0.6 - add more docs to `Transcode` - add more module-level tests --- pixeldata/src/transcode.rs | 381 ++++++++++++++++++++++++++----------- 1 file changed, 270 insertions(+), 111 deletions(-) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 80db9cd5b..412066233 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -1,11 +1,20 @@ +//! DICOM Transcoder API +//! +//! This module collects the pixel data decoding and encoding capabilities +//! of `dicom_encoding` and `dicom_pixeldata` +//! to offer a convenient API for converting DICOM objects +//! to different transfer syntaxes. +//! +//! See the [`Transcode`] trait for more information. use dicom_core::{ - smallvec::smallvec, DataDictionary, DataElement, DicomValue, Length, PrimitiveValue, Tag, VR, + value::PixelFragmentSequence, DataDictionary, DataElement, + Length, PrimitiveValue, VR, ops::ApplyOp, }; use dicom_dictionary_std::tags; use dicom_encoding::{adapters::EncodeOptions, Codec, TransferSyntax, TransferSyntaxIndex}; -use dicom_object::{mem::InMemFragment, FileDicomObject, InMemDicomObject}; +use dicom_object::{FileDicomObject, InMemDicomObject}; use dicom_transfer_syntax_registry::{entries::EXPLICIT_VR_LITTLE_ENDIAN, TransferSyntaxRegistry}; -use snafu::{ensure, OptionExt, ResultExt, Snafu}; +use snafu::{OptionExt, ResultExt, Snafu}; use crate::PixelDecoder; @@ -21,11 +30,14 @@ pub(crate) enum InnerError { /// Unsupported target transfer syntax UnsupportedTransferSyntax, + /// Unsupported transcoding capability + UnsupportedTranscoding, + /// Could not decode pixel data of receiving object DecodePixelData { source: crate::Error }, /// Could not read receiving object - ReadObject { source: dicom_object::Error }, + ReadObject { source: dicom_object::ReadError }, /// Could not encode pixel data to target encoding EncodePixelData { @@ -44,24 +56,64 @@ pub type Result = std::result::Result; /// Interface for transcoding a DICOM object's pixel data /// to comply with a different transfer syntax. +/// Can be implemented by in-memory DICOM object representations +/// as well as partial or lazy DICOM object readers, +/// so that transcoding can be performed without loading the entire object. +/// +/// # Example +/// +/// A typical [file DICOM object in memory](FileDicomObject) +/// can be transcoded inline using [`transcode`](Transcode::transcode). +/// +/// ```no_run +/// # use dicom_object::open_file; +/// use dicom_pixeldata::Transcode as _; +/// +/// let mut obj = dicom_object::open_file("image.dcm").unwrap(); +/// // convert to JPEG +/// obj.transcode(&dicom_transfer_syntax_registry::entries::JPEG_BASELINE.erased())?; +/// +/// // save transcoded version to file +/// obj.write_to_file("image_jpg.dcm")?; +/// +/// Ok::<(), Box>(()) +/// ``` pub trait Transcode { /// Convert the receiving object's transfer syntax - /// to the one specified in `ts`, - /// replacing one or more attributes to fit the intended transfer syntax, + /// to the one specified in `ts` according to the given encoding options. + /// + /// This method may replace one or more attributes accordingly, + /// including the meta group specifying the transfer syntax. + /// The encoding options only apply if the pixel data needs to be re-encoded. + /// + /// If the receiving object's pixel data is encapsulated, + /// the object might be first decoded into native pixel data. + /// In case of an encoding error, + /// the object may be left in an intermediate state, + /// which should not be assumed to be consistent. + fn transcode_with_options(&mut self, ts: &TransferSyntax, options: EncodeOptions) -> Result<()>; + + /// Convert the receiving object's transfer syntax + /// to the one specified in `ts`. + /// + /// This method may replace one or more attributes accordingly, /// including the meta group specifying the transfer syntax. /// /// If the receiving object's pixel data is encapsulated, - /// the object is first decoded into native pixel data. + /// the object might be first decoded into native pixel data. /// In case of an encoding error, - /// the object may be left with this intermediate state. - fn transcode(&mut self, ts: &TransferSyntax) -> Result<()>; + /// the object may be left in an intermediate state, + /// which should not be assumed to be consistent. + fn transcode(&mut self, ts: &TransferSyntax) -> Result<()> { + self.transcode_with_options(ts, EncodeOptions::default()) + } } impl Transcode for FileDicomObject> where D: Clone + DataDictionary, { - fn transcode(&mut self, ts: &TransferSyntax) -> Result<()> { + fn transcode_with_options(&mut self, ts: &TransferSyntax, options: EncodeOptions) -> Result<()> { let current_ts_uid = &self.meta().transfer_syntax; // do nothing if the transfer syntax already matches if current_ts_uid == ts.uid() { @@ -75,13 +127,14 @@ where ts: current_ts_uid.to_string(), })?; - if ts.is_codec_free() { - if current_ts.is_codec_free() { + match (current_ts.is_codec_free(), ts.is_codec_free()) { + (true, true) => { // no pixel data conversion is necessary: // change transfer syntax and return self.meta_mut().set_transfer_syntax(ts); Ok(()) - } else { + }, + (false, true) => { // decode pixel data let decoded_pixeldata = self.decode_pixel_data().context(DecodePixelDataSnafu)?; @@ -113,107 +166,108 @@ where } .fail()? } - }; + } + + // update transfer syntax + self.meta_mut() + .set_transfer_syntax(&ts); - // change transfer syntax and return - self.meta_mut().set_transfer_syntax(ts); Ok(()) - } - } else { - // must decode then encode - let adapter = match ts.codec() { - Codec::PixelData(adapter) => adapter, - Codec::EncapsulatedPixelData => return UnsupportedTransferSyntaxSnafu.fail()?, - Codec::None | Codec::Unsupported | Codec::Dataset(_) => { - unreachable!("Unexpected codec from transfer syntax") - } - }; - - // decode pixel data - let decoded_pixeldata = self.decode_pixel_data().context(DecodePixelDataSnafu)?; - let bits_allocated = decoded_pixeldata.bits_allocated(); - let number_of_frames = decoded_pixeldata.number_of_frames(); - - // note: there currently not a clear way - // to encode multiple fragments, - // so we stop if the image has more than one frame - ensure!(number_of_frames == 1, MultiFrameEncodingNotImplementedSnafu); - - // apply change to pixel data attribute - match bits_allocated { - 8 => { - // 8-bit samples - let pixels = decoded_pixeldata.data().to_vec(); - self.put(DataElement::new_with_len( - tags::PIXEL_DATA, - VR::OW, - Length::defined(pixels.len() as u32), - PrimitiveValue::from(pixels), - )); - } - 16 => { - // 16-bit samples - let pixels = decoded_pixeldata.data_ow(); - self.put(DataElement::new_with_len( - tags::PIXEL_DATA, - VR::OW, - Length::defined(pixels.len() as u32 * 2), - PrimitiveValue::U16(pixels.into()), - )); - } - _ => return UnsupportedBitsAllocatedSnafu { bits_allocated }.fail()?, - }; - - // change transfer syntax to Explicit VR little endian - self.meta_mut() - .set_transfer_syntax(&EXPLICIT_VR_LITTLE_ENDIAN); - - // use RWPixel adapter API - let mut pixeldata = Vec::new(); - let options = EncodeOptions::new(); - adapter - .encode(&*self, options, &mut pixeldata) - .context(EncodePixelDataSnafu)?; - - let total_pixeldata_len = pixeldata.len() as u64; - - // put everything in a single fragment - let pixel_seq = DicomValue::, InMemFragment>::PixelSequence { - offset_table: smallvec![], - fragments: smallvec![pixeldata], - }; - - self.put(DataElement::new_with_len( - tags::PIXEL_DATA, - VR::OB, - Length::UNDEFINED, - pixel_seq, - )); - - self.put(DataElement::new( - tags::NUMBER_OF_FRAMES, - VR::IS, - PrimitiveValue::from("1"), - )); - - // replace Encapsulated Pixel Data Value Total Length - // if it is present - if self - .element_opt(Tag(0x7FE0, 0x0003)) - .context(ReadObjectSnafu)? - .is_some() - { + }, + (_, false) => { + // must decode then encode + let writer = match ts.codec() { + Codec::EncapsulatedPixelData(_, Some(writer)) => writer, + Codec::EncapsulatedPixelData(..) => { + return UnsupportedTransferSyntaxSnafu.fail()? + } + Codec::Dataset(None) => return UnsupportedTransferSyntaxSnafu.fail()?, + Codec::Dataset(Some(_)) => return UnsupportedTranscodingSnafu.fail()?, + Codec::None => { + // already tested in `is_codec_free` + unreachable!("Unexpected codec from transfer syntax") + } + }; + + // decode pixel data + let decoded_pixeldata = self.decode_pixel_data().context(DecodePixelDataSnafu)?; + let bits_allocated = decoded_pixeldata.bits_allocated(); + + // apply change to pixel data attribute + match bits_allocated { + 8 => { + // 8-bit samples + let pixels = decoded_pixeldata.data().to_vec(); + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OW, + Length::defined(pixels.len() as u32), + PrimitiveValue::from(pixels), + )); + } + 16 => { + // 16-bit samples + let pixels = decoded_pixeldata.data_ow(); + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OW, + Length::defined(pixels.len() as u32 * 2), + PrimitiveValue::U16(pixels.into()), + )); + } + _ => return UnsupportedBitsAllocatedSnafu { bits_allocated }.fail()?, + }; + + // change transfer syntax to Explicit VR little endian + self.meta_mut() + .set_transfer_syntax(&EXPLICIT_VR_LITTLE_ENDIAN.erased()); + + // use RWPixel adapter API + let mut offset_table = Vec::new(); + let mut fragments = Vec::new(); + + let ops = writer + .encode(&*self, options, &mut fragments, &mut offset_table) + .context(EncodePixelDataSnafu)?; + + let num_frames = offset_table.len(); + let total_pixeldata_len: u64 = fragments.iter().map(|f| f.len() as u64).sum(); + + self.put(DataElement::new_with_len( + tags::PIXEL_DATA, + VR::OB, + Length::UNDEFINED, + PixelFragmentSequence::new(offset_table, fragments), + )); + self.put(DataElement::new( - Tag(0x7FE0, 0x0003), + tags::NUMBER_OF_FRAMES, + VR::IS, + num_frames.to_string(), + )); + + // provide Encapsulated Pixel Data Value Total Length + self.put(DataElement::new( + tags::ENCAPSULATED_PIXEL_DATA_VALUE_TOTAL_LENGTH, VR::UV, PrimitiveValue::from(total_pixeldata_len), )); - } - // change transfer syntax - self.meta_mut().set_transfer_syntax(ts); + // try to apply operations + for (n, op) in ops.into_iter().enumerate() { + match self.apply(op) { + Ok(_) => (), + Err(e) => { + tracing::warn!("Could not apply transcoding step #{}: {}", n, e) + } + } + } - Ok(()) + // change transfer syntax + self.meta_mut().set_transfer_syntax(ts); + + Ok(()) + } } } } @@ -221,23 +275,25 @@ where #[cfg(test)] mod tests { use super::*; + use dicom_dictionary_std::uids; use dicom_object::open_file; use dicom_test_files; + use dicom_transfer_syntax_registry::entries::{JPEG_BASELINE, JPEG_EXTENDED}; #[test] - fn test_transcode_from_jpeg_to_native_rgb() { + fn test_transcode_from_jpeg_lossless_to_native_rgb() { let test_file = dicom_test_files::path("pydicom/SC_rgb_jpeg_gdcm.dcm").unwrap(); let mut obj = open_file(test_file).unwrap(); // pre-condition check: pixel data conversion is needed here - assert_ne!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + assert_eq!(&obj.meta().transfer_syntax, uids::JPEG_LOSSLESS_SV1); // transcode to explicit VR little endian obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) .expect("Should have transcoded successfully"); // check transfer syntax - assert_eq!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + assert_eq!(obj.meta().transfer_syntax(), EXPLICIT_VR_LITTLE_ENDIAN.uid()); // check that the pixel data is in its native form // and has the expected size @@ -254,14 +310,86 @@ mod tests { } #[test] - // Test ignored until 12-bit JPEG decoding is supported + fn test_transcode_from_native_to_jpeg_rgb() { + let test_file = dicom_test_files::path("pydicom/SC_rgb.dcm").unwrap(); + let mut obj = open_file(&test_file).unwrap(); + + // pre-condition check: pixel data is native + assert_eq!(obj.meta().transfer_syntax(), uids::EXPLICIT_VR_LITTLE_ENDIAN); + + // transcode to JPEG baseline + obj.transcode(&JPEG_BASELINE.erased()) + .expect("Should have transcoded successfully"); + + // check transfer syntax + assert_eq!(obj.meta().transfer_syntax(), JPEG_BASELINE.uid()); + + // check that the pixel data is encapsulated + // and has the expected number of fragments + let pixel_data = obj.get(tags::PIXEL_DATA).unwrap(); + let fragments = pixel_data + .fragments() + .expect("Pixel Data should be in encapsulated fragments"); + + // one frame, one fragment (as required by JPEG baseline) + assert_eq!(fragments.len(), 1); + + // check that the fragment data is in valid JPEG (magic code) + let fragment = &fragments[0]; + assert!(fragment.len() > 4); + assert_eq!(&fragment[0..2], &[0xFF, 0xD8]); + + let size_1 = fragment.len(); + + // re-encode with different options + + let mut obj = open_file(test_file).unwrap(); + + // pre-condition check: pixel data is native + assert_eq!(obj.meta().transfer_syntax(), uids::EXPLICIT_VR_LITTLE_ENDIAN); + + // transcode to JPEG baseline + let mut options = EncodeOptions::new(); + // low quality + options.quality = Some(50); + obj.transcode_with_options(&JPEG_BASELINE.erased(), options) + .expect("Should have transcoded successfully"); + + // check transfer syntax + assert_eq!(obj.meta().transfer_syntax(), JPEG_BASELINE.uid()); + + // check that the pixel data is encapsulated + // and has the expected number of fragments + let pixel_data = obj.get(tags::PIXEL_DATA).unwrap(); + let fragments = pixel_data + .fragments() + .expect("Pixel Data should be in encapsulated fragments"); + + // one frame, one fragment (as required by JPEG baseline) + assert_eq!(fragments.len(), 1); + + // check that the fragment data is in valid JPEG (magic code) + let fragment = &fragments[0]; + assert!(fragment.len() > 4); + assert_eq!(&fragment[0..2], &[0xFF, 0xD8]); + + let size_2 = fragment.len(); + + // the size of the second fragment should be smaller + // due to lower quality + assert!(size_2 < size_1, "expected smaller size for lower quality, but {} => {}", size_2, size_1); + + } + + #[test] + // Note: Test ignored until 12-bit JPEG decoding is supported #[ignore] fn test_transcode_from_jpeg_to_native_16bit() { let test_file = dicom_test_files::path("pydicom/JPEG-lossy.dcm").unwrap(); let mut obj = open_file(test_file).unwrap(); // pre-condition check: pixel data conversion is needed here - assert_ne!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + assert_eq!(&obj.meta().transfer_syntax, uids::JPEG_EXTENDED12_BIT); // transcode to explicit VR little endian obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) @@ -284,4 +412,35 @@ mod tests { assert_eq!(pixels.len(), rows * cols * spp * bps); } + + /// if the transfer syntax is the same, no transcoding should be performed + #[test] + fn test_no_transcoding_needed() { + { + let test_file = dicom_test_files::path("pydicom/SC_rgb.dcm").unwrap(); + let mut obj = open_file(test_file).unwrap(); + + // transcode to the same TS + obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) + .expect("Should have transcoded successfully"); + + assert_eq!(obj.meta().transfer_syntax(), EXPLICIT_VR_LITTLE_ENDIAN.uid()); + // pixel data is still native + let pixel_data = obj.get(tags::PIXEL_DATA).unwrap().to_bytes().unwrap(); + assert_eq!(pixel_data.len(), 100 * 100 * 3); + } + { + let test_file = dicom_test_files::path("pydicom/JPEG-lossy.dcm").unwrap(); + let mut obj = open_file(test_file).unwrap(); + + // transcode to the same TS + obj.transcode(&JPEG_EXTENDED.erased()) + .expect("Should have transcoded successfully"); + + assert_eq!(obj.meta().transfer_syntax(), JPEG_EXTENDED.uid()); + // pixel data is still encapsulated + let fragments = obj.get(tags::PIXEL_DATA).unwrap().fragments().unwrap(); + assert_eq!(fragments.len(), 1); + } + } } From 70fa0a9ae8337e4722018ef513ebd2ebc83ab507 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 12:25:26 +0100 Subject: [PATCH 06/12] [pixeldata] tweak doctest in Transcode - hide Ok at the end --- pixeldata/src/transcode.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 412066233..dfee5fc1b 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -75,8 +75,7 @@ pub type Result = std::result::Result; /// /// // save transcoded version to file /// obj.write_to_file("image_jpg.dcm")?; -/// -/// Ok::<(), Box>(()) +/// # Ok::<(), Box>(()) /// ``` pub trait Transcode { /// Convert the receiving object's transfer syntax From d7c9a6cee9072ff15dac4e99540875e82e6b7adc Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 13:41:08 +0100 Subject: [PATCH 07/12] [pixeldata] Feature gate transcode tests --- pixeldata/src/transcode.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index dfee5fc1b..43912eff3 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -277,8 +277,11 @@ mod tests { use dicom_dictionary_std::uids; use dicom_object::open_file; use dicom_test_files; - use dicom_transfer_syntax_registry::entries::{JPEG_BASELINE, JPEG_EXTENDED}; + use dicom_transfer_syntax_registry::entries::JPEG_EXTENDED; + #[cfg(feature = "native")] + use dicom_transfer_syntax_registry::entries::JPEG_BASELINE; + #[cfg(feature = "native")] #[test] fn test_transcode_from_jpeg_lossless_to_native_rgb() { let test_file = dicom_test_files::path("pydicom/SC_rgb_jpeg_gdcm.dcm").unwrap(); @@ -308,6 +311,7 @@ mod tests { assert_eq!(pixels.len(), rows * cols * spp); } + #[cfg(feature = "native")] #[test] fn test_transcode_from_native_to_jpeg_rgb() { let test_file = dicom_test_files::path("pydicom/SC_rgb.dcm").unwrap(); @@ -380,6 +384,7 @@ mod tests { } + #[cfg(feature = "native")] #[test] // Note: Test ignored until 12-bit JPEG decoding is supported #[ignore] @@ -432,11 +437,13 @@ mod tests { let test_file = dicom_test_files::path("pydicom/JPEG-lossy.dcm").unwrap(); let mut obj = open_file(test_file).unwrap(); + assert_eq!(obj.meta().transfer_syntax(), uids::JPEG_EXTENDED12_BIT); + // transcode to the same TS obj.transcode(&JPEG_EXTENDED.erased()) .expect("Should have transcoded successfully"); - assert_eq!(obj.meta().transfer_syntax(), JPEG_EXTENDED.uid()); + assert_eq!(obj.meta().transfer_syntax(), uids::JPEG_EXTENDED12_BIT); // pixel data is still encapsulated let fragments = obj.get(tags::PIXEL_DATA).unwrap().fragments().unwrap(); assert_eq!(fragments.len(), 1); From 170e2a00cdaadf4d01c667fa16a62edee2d4986a Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 15:53:17 +0100 Subject: [PATCH 08/12] [ts-registry] Fix and enhance JPEG adapter - fix frame retrieval from native pixel data - improve handling of high bit depth dat by narrowing it down to 8 bits if necessary --- transfer-syntax-registry/src/adapters/jpeg.rs | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/transfer-syntax-registry/src/adapters/jpeg.rs b/transfer-syntax-registry/src/adapters/jpeg.rs index 8ab5f3e65..39504d488 100644 --- a/transfer-syntax-registry/src/adapters/jpeg.rs +++ b/transfer-syntax-registry/src/adapters/jpeg.rs @@ -1,7 +1,7 @@ //! Support for JPG image decoding. use dicom_core::ops::{AttributeAction, AttributeOp}; -use dicom_core::Tag; +use dicom_core::{Tag, PrimitiveValue}; use dicom_encoding::adapters::{ decode_error, encode_error, DecodeResult, EncodeOptions, EncodeResult, PixelDataObject, PixelDataReader, PixelDataWriter, @@ -284,6 +284,11 @@ impl PixelDataWriter for JpegAdapter { .context(encode_error::MissingAttributeSnafu { name: "BitsAllocated", })?; + let bits_stored = src + .bits_stored() + .context(encode_error::MissingAttributeSnafu { + name: "BitsStored", + })?; ensure_whatever!( bits_allocated == 8 || bits_allocated == 16, @@ -305,20 +310,28 @@ impl PixelDataWriter for JpegAdapter { let photometric_interpretation = match samples_per_pixel { 1 => "MONOCHROME2", 3 => "RGB", - _ => unreachable!(), + _ => whatever!("Unsupported samples per pixel: {}", samples_per_pixel), }; // record dst length before encoding to know full jpeg size let len_before = dst.len(); + // identify frame data using the frame index + let pixeldata_uncompressed = &src + .raw_pixel_data() + .context(encode_error::MissingAttributeSnafu { name: "Pixel Data" })? + .fragments[0]; + + let frame_data = pixeldata_uncompressed.get(frame_size * frame as usize .. frame_size * (frame as usize + 1)) + .whatever_context("Frame index out of bounds")?; + + let frame_data = narrow_8bit(frame_data, bits_stored)?; + // Encode the data - let frame_uncompressed = src - .fragment(frame as usize) - .context(encode_error::FrameRangeOutOfBoundsSnafu)?; let mut encoder = jpeg_encoder::Encoder::new(&mut *dst, quality); encoder.set_progressive(false); encoder - .encode(&frame_uncompressed, cols, rows, color_type) + .encode(&frame_data, cols, rows, color_type) .whatever_context("JPEG encoding failed")?; let compressed_frame_size = dst.len() - len_before; @@ -328,6 +341,12 @@ impl PixelDataWriter for JpegAdapter { // provide attribute changes Ok(vec![ + // bits allocated + AttributeOp::new(Tag(0x0028, 0x0100), AttributeAction::Set(PrimitiveValue::from(8_u16))), + // bits stored + AttributeOp::new(Tag(0x0028, 0x0101), AttributeAction::Set(PrimitiveValue::from(8_u16))), + // high bit + AttributeOp::new(Tag(0x0028, 0x0102), AttributeAction::Set(PrimitiveValue::from(7_u16))), // lossy image compression AttributeOp::new(Tag(0x0028, 0x2110), AttributeAction::SetStr("01".into())), // lossy image compression ratio @@ -347,3 +366,21 @@ impl PixelDataWriter for JpegAdapter { fn next_even(l: u64) -> u64 { (l + 1) & !1 } + +/// reduce data precision to 8 bits if necessary +/// data loss is possible +fn narrow_8bit(frame_data: &[u8], bits_stored: u16) -> EncodeResult> { + debug_assert!(bits_stored >= 8); + match bits_stored { + 8 => Ok(Cow::Borrowed(frame_data)), + 9..=16 => { + let mut v = Vec::with_capacity(frame_data.len() / 2); + for chunk in frame_data.chunks(2) { + let b = u16::from(chunk[0])| u16::from(chunk[1]) << 8; + v.push((b >> (bits_stored - 8)) as u8); + } + Ok(Cow::Owned(v)) + } + b => whatever!("Unsupported Bits Stored {}", b), + } +} From 3fb1c59a1ec84500b0af84865e604239096c368e Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 15:54:49 +0100 Subject: [PATCH 09/12] [pixeldata] Add test for transcoding 2-frame data --- pixeldata/src/transcode.rs | 44 ++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 43912eff3..26176bea9 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -113,7 +113,7 @@ where D: Clone + DataDictionary, { fn transcode_with_options(&mut self, ts: &TransferSyntax, options: EncodeOptions) -> Result<()> { - let current_ts_uid = &self.meta().transfer_syntax; + let current_ts_uid = self.meta().transfer_syntax(); // do nothing if the transfer syntax already matches if current_ts_uid == ts.uid() { return Ok(()); @@ -288,7 +288,7 @@ mod tests { let mut obj = open_file(test_file).unwrap(); // pre-condition check: pixel data conversion is needed here - assert_eq!(&obj.meta().transfer_syntax, uids::JPEG_LOSSLESS_SV1); + assert_eq!(obj.meta().transfer_syntax(), uids::JPEG_LOSSLESS_SV1); // transcode to explicit VR little endian obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) @@ -393,14 +393,14 @@ mod tests { let mut obj = open_file(test_file).unwrap(); // pre-condition check: pixel data conversion is needed here - assert_eq!(&obj.meta().transfer_syntax, uids::JPEG_EXTENDED12_BIT); + assert_eq!(obj.meta().transfer_syntax(), uids::JPEG_EXTENDED12_BIT); // transcode to explicit VR little endian obj.transcode(&EXPLICIT_VR_LITTLE_ENDIAN.erased()) .expect("Should have transcoded successfully"); // check transfer syntax - assert_eq!(&obj.meta().transfer_syntax, EXPLICIT_VR_LITTLE_ENDIAN.uid()); + assert_eq!(obj.meta().transfer_syntax(), EXPLICIT_VR_LITTLE_ENDIAN.uid()); // check that the pixel data is in its native form // and has the expected size @@ -417,6 +417,42 @@ mod tests { assert_eq!(pixels.len(), rows * cols * spp * bps); } + + /// can transcode native multi-frame pixel data + #[test] + fn test_transcode_2frames_to_jpeg() { + let test_file = dicom_test_files::path("pydicom/SC_rgb_2frame.dcm").unwrap(); + let mut obj = open_file(test_file).unwrap(); + + // pre-condition check: pixel data conversion is needed here + assert_eq!(obj.meta().transfer_syntax(), uids::EXPLICIT_VR_LITTLE_ENDIAN); + + // transcode to JPEG baseline + obj.transcode(&JPEG_BASELINE.erased()) + .expect("Should have transcoded successfully"); + + // check transfer syntax + assert_eq!(obj.meta().transfer_syntax(), JPEG_BASELINE.uid()); + + // check that the number of frames stayed the same + let num_frames = obj.get(tags::NUMBER_OF_FRAMES).unwrap(); + assert_eq!(num_frames.to_int::().unwrap(), 2); + + // check that the pixel data is encapsulated + let pixel_data = obj.element(tags::PIXEL_DATA).unwrap(); + + let fragments = pixel_data + .fragments() + .expect("Pixel Data should be in encapsulated fragments"); + + // two frames, two fragments (as required by JPEG baseline) + assert_eq!(fragments.len(), 2); + + // each frame has some data + assert!(fragments[0].len() > 4); + assert!(fragments[1].len() > 4); + } + /// if the transfer syntax is the same, no transcoding should be performed #[test] fn test_no_transcoding_needed() { From dd1d2192951a9d781e360613a87c85916c07a83d Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 16:40:41 +0100 Subject: [PATCH 10/12] [pixeldata] Add binary dicom-transcode - CLI tool for transcoding files, gated behind feature "cli" - include "cli" feature in CI --- .github/workflows/rust.yml | 2 +- Cargo.lock | 16 +-- pixeldata/Cargo.toml | 16 +++ pixeldata/src/bin/dicom-transcode.rs | 169 +++++++++++++++++++++++++++ 4 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 pixeldata/src/bin/dicom-transcode.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 354bd9728..983ef8525 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -23,7 +23,7 @@ jobs: toolchain: ${{ matrix.rust }} cache: true # test project with default + extra features - - run: cargo test --features image,ndarray,sop-class,rle + - run: cargo test --features image,ndarray,sop-class,rle,cli # test dicom-pixeldata with gdcm-rs - run: cargo test -p dicom-pixeldata --features gdcm # test dicom-pixeldata without default features diff --git a/Cargo.lock b/Cargo.lock index 4d2230d99..4adf24199 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -199,33 +199,31 @@ dependencies = [ [[package]] name = "clap" -version = "4.4.0" +version = "4.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d5f1946157a96594eb2d2c10eb7ad9a2b27518cb3000209dec700c35df9197d" +checksum = "6a13b88d2c62ff462f88e4a121f17a82c1af05693a2f192b5c38d14de73c19f6" dependencies = [ "clap_builder", "clap_derive", - "once_cell", ] [[package]] name = "clap_builder" -version = "4.4.0" +version = "4.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78116e32a042dd73c2901f0dc30790d20ff3447f3e3472fad359e8c3d282bcd6" +checksum = "2bb9faaa7c2ef94b2743a21f5a29e6f0010dff4caa69ac8e9d6cf8b6fa74da08" dependencies = [ "anstream", "anstyle", "clap_lex", - "once_cell", "strsim", ] [[package]] name = "clap_derive" -version = "4.4.0" +version = "4.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9fd1a5729c4548118d7d70ff234a44868d00489a4b6597b0b020918a0e91a1a" +checksum = "0862016ff20d69b84ef8247369fabf5c008a7417002411897d40ee1f4532b873" dependencies = [ "heck", "proc-macro2", @@ -525,6 +523,7 @@ name = "dicom-pixeldata" version = "0.2.0" dependencies = [ "byteorder", + "clap", "dicom-core", "dicom-dictionary-std", "dicom-encoding", @@ -539,6 +538,7 @@ dependencies = [ "rstest", "snafu", "tracing", + "tracing-subscriber", ] [[package]] diff --git a/pixeldata/Cargo.toml b/pixeldata/Cargo.toml index 565c2fd14..981c62e81 100644 --- a/pixeldata/Cargo.toml +++ b/pixeldata/Cargo.toml @@ -10,6 +10,11 @@ categories = ["multimedia::images"] keywords = ["dicom"] readme = "README.md" +[[bin]] +name = "dicom-transcode" +path = "src/bin/dicom-transcode.rs" +required-features = ["cli"] + [dependencies] dicom-object = { path = "../object", version = "0.6.1" } dicom-core = { path = "../core", version = "0.6.1" } @@ -30,6 +35,15 @@ default-features = false features = ["jpeg", "png", "pnm", "tiff", "webp", "bmp", "openexr"] optional = true +[dependencies.clap] +version = "4.4.2" +optional = true +features = ["cargo", "derive"] + +[dependencies.tracing-subscriber] +version = "0.3.17" +optional = true + [dev-dependencies] rstest = "0.18.1" dicom-test-files = "0.2.1" @@ -44,5 +58,7 @@ native = ["dicom-transfer-syntax-registry/native"] gdcm = ["gdcm-rs"] rayon = ["dep:rayon", "image?/jpeg_rayon", "dicom-transfer-syntax-registry/rayon"] +cli = ["dep:clap", "dep:tracing-subscriber"] + [package.metadata.docs.rs] features = ["image", "ndarray"] diff --git a/pixeldata/src/bin/dicom-transcode.rs b/pixeldata/src/bin/dicom-transcode.rs new file mode 100644 index 000000000..e67f34194 --- /dev/null +++ b/pixeldata/src/bin/dicom-transcode.rs @@ -0,0 +1,169 @@ +//! A CLI tool for transcoding a DICOM file +//! to another transfer syntax. +use clap::Parser; +use dicom_dictionary_std::uids; +use dicom_encoding::{TransferSyntaxIndex, TransferSyntax}; +use dicom_encoding::adapters::EncodeOptions; +use dicom_object::open_file; +use dicom_transfer_syntax_registry::TransferSyntaxRegistry; +use snafu::{Report, Whatever, OptionExt}; +use tracing::Level; +use std::path::PathBuf; +use dicom_pixeldata::Transcode; + +/// Exit code for when an error emerged while reading the DICOM file. +const ERROR_READ: i32 = -2; +/// Exit code for when an error emerged while transcoding the file. +const ERROR_TRANSCODE: i32 = -3; +/// Exit code for when an error emerged while writing the file. +const ERROR_WRITE: i32 = -4; +/// Exit code for when an error emerged while writing the file. +const ERROR_OTHER: i32 = -128; + +/// Transcode a DICOM file +#[derive(Debug, Parser)] +#[command(version)] +struct App { + file: PathBuf, + /// The output file (default is to change the extension to .new.dcm) + #[clap(short = 'o', long = "output")] + output: Option, + + /// The encoding quality (from 0 to 100) + #[clap(long = "quality")] + quality: Option, + /// The encoding effort (from 0 to 100) + #[clap(long = "effort")] + effort: Option, + + /// Target transfer syntax + #[clap(flatten)] + target_ts: TargetTransferSyntax, + + /// Verbose mode + #[clap(short = 'v', long = "verbose")] + verbose: bool, +} + +/// Specifier for the target transfer syntax +#[derive(Debug, Parser)] + +#[group(required = true, multiple = false)] +struct TargetTransferSyntax { + /// Transcode to the Transfer Syntax indicated by UID + #[clap(long = "ts")] + ts: Option, + + /// Transcode to Explicit VR Little Endian + #[clap(long = "expl-vr-le")] + explicit_vr_le: bool, + + /// Transcode to Implicit VR Little Endian + #[clap(long = "impl-vr-le")] + implicit_vr_le: bool, + + /// Transcode to JPEG baseline (8-bit) + #[clap(long = "jpeg-baseline")] + jpeg_baseline: bool, +} + +impl TargetTransferSyntax { + fn resolve(&self) -> Result<&'static TransferSyntax, Whatever> { + + // explicit VR little endian + if self.explicit_vr_le { + return Ok(TransferSyntaxRegistry.get(uids::EXPLICIT_VR_LITTLE_ENDIAN) + .expect("Explicit VR Little Endian is missing???")); + } + + // implicit VR little endian + if self.implicit_vr_le { + return Ok(TransferSyntaxRegistry.get(uids::IMPLICIT_VR_LITTLE_ENDIAN) + .expect("Implicit VR Little Endian is missing???")); + } + + // JPEG baseline + if self.jpeg_baseline { + return TransferSyntaxRegistry.get(uids::JPEG_BASELINE8_BIT) + .whatever_context("Missing specifier for JPEG Baseline (8-bit)"); + } + + // by TS UID + let Some(ts) = &self.ts else { + snafu::whatever!("No target transfer syntax specified"); + }; + + TransferSyntaxRegistry.get(ts) + .whatever_context("Unknown transfer syntax") + } +} + + +fn main() { + run().unwrap_or_else(|e| { + eprintln!("{}", Report::from_error(e)); + std::process::exit(ERROR_OTHER); + }); +} + +fn run() -> Result<(), Whatever> { + let App { + file, + output, + quality, + effort, + target_ts, + verbose, + } = App::parse(); + + tracing::subscriber::set_global_default( + tracing_subscriber::FmtSubscriber::builder() + .with_max_level(if verbose { Level::DEBUG } else { Level::INFO }) + .finish(), + ) + .unwrap_or_else(|e| { + eprintln!("{}", snafu::Report::from_error(e)); + }); + + let output = output.unwrap_or_else(|| { + let mut file = file.clone(); + file.set_extension("new.dcm"); + file + }); + + let mut obj = open_file(file).unwrap_or_else(|e| { + eprintln!("{}", Report::from_error(e)); + std::process::exit(ERROR_READ); + }); + + // lookup transfer syntax + let ts = target_ts.resolve()?; + + let mut options = EncodeOptions::default(); + options.quality = quality; + options.effort = effort; + + obj.transcode_with_options(ts, options).unwrap_or_else(|e| { + eprintln!("{}", Report::from_error(e)); + std::process::exit(ERROR_TRANSCODE); + }); + + // write to file + obj.write_to_file(output).unwrap_or_else(|e| { + eprintln!("{}", Report::from_error(e)); + std::process::exit(ERROR_WRITE); + }); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use crate::App; + use clap::CommandFactory; + + #[test] + fn verify_cli() { + App::command().debug_assert(); + } +} From 48c694eb92546688346ffca813428423ea4b3d71 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 2 Sep 2023 16:49:04 +0100 Subject: [PATCH 11/12] [pixeldata] Feature-gate extra test --- pixeldata/src/transcode.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pixeldata/src/transcode.rs b/pixeldata/src/transcode.rs index 26176bea9..50a33aab1 100644 --- a/pixeldata/src/transcode.rs +++ b/pixeldata/src/transcode.rs @@ -419,6 +419,7 @@ mod tests { /// can transcode native multi-frame pixel data + #[cfg(feature = "native")] #[test] fn test_transcode_2frames_to_jpeg() { let test_file = dicom_test_files::path("pydicom/SC_rgb_2frame.dcm").unwrap(); From 88d3edf46ca8f094c9688babdd7562de894656b0 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 23 Sep 2023 14:48:22 +0100 Subject: [PATCH 12/12] [storescu] Add transcoding when required by SCP - new Cargo feature "transcode" - feature-gate dicom-pixeldata on "transcode" - if enabled, try to decode the file to explicit VR LE when the SCP does not accept the original TS - add option never_transcode, to support retaining the previous behavior --- Cargo.lock | 1 + storescu/Cargo.toml | 12 +++- storescu/src/main.rs | 152 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 137 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4adf24199..4a3dbe147 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -578,6 +578,7 @@ dependencies = [ "dicom-dictionary-std", "dicom-encoding", "dicom-object", + "dicom-pixeldata", "dicom-transfer-syntax-registry", "dicom-ul", "indicatif", diff --git a/storescu/Cargo.toml b/storescu/Cargo.toml index bcd94bcde..3d5b12ec5 100644 --- a/storescu/Cargo.toml +++ b/storescu/Cargo.toml @@ -10,14 +10,20 @@ categories = ["command-line-utilities"] keywords = ["dicom"] readme = "README.md" +[features] +default = ["transcode"] +# support DICOM transcoding +transcode = ["dep:dicom-pixeldata"] + [dependencies] clap = { version = "4.0.18", features = ["derive"] } dicom-core = { path = '../core', version = "0.6.1" } -dicom-ul = { path = '../ul', version = "0.5.0" } -dicom-object = { path = '../object', version = "0.6.1" } -dicom-encoding = { path = "../encoding/", version = "0.6.0" } dicom-dictionary-std = { path = "../dictionary-std/", version = "0.6.0" } +dicom-encoding = { path = "../encoding/", version = "0.6.0" } +dicom-object = { path = '../object', version = "0.6.1" } +dicom-pixeldata = { version = "0.2.0", path = "../pixeldata", optional = true } dicom-transfer-syntax-registry = { path = "../transfer-syntax-registry/", version = "0.6.0" } +dicom-ul = { path = '../ul', version = "0.5.0" } walkdir = "2.3.2" indicatif = "0.17.0" tracing = "0.1.34" diff --git a/storescu/src/main.rs b/storescu/src/main.rs index 0d2727a7d..ca29e91df 100644 --- a/storescu/src/main.rs +++ b/storescu/src/main.rs @@ -1,8 +1,9 @@ use clap::Parser; use dicom_core::{dicom_value, header::Tag, DataElement, VR}; -use dicom_dictionary_std::tags; +use dicom_dictionary_std::{tags, uids}; use dicom_encoding::transfer_syntax; -use dicom_object::{mem::InMemDicomObject, open_file, StandardDataDictionary}; +use dicom_encoding::TransferSyntax; +use dicom_object::{mem::InMemDicomObject, open_file, DefaultDicomObject, StandardDataDictionary}; use dicom_transfer_syntax_registry::TransferSyntaxRegistry; use dicom_ul::{ association::ClientAssociationOptions, @@ -50,6 +51,11 @@ struct App { /// fail if not all DICOM files can be transferred #[arg(long = "fail-first")] fail_first: bool, + /// fail file transfer if it cannot be done without transcoding + #[arg(long("never-transcode"))] + // hide option if transcoding is disabled + #[cfg_attr(not(feature = "transcode"), arg(hide(true)))] + never_transcode: bool, } struct DicomFile { @@ -77,6 +83,9 @@ enum Error { /// Could not construct DICOM command CreateCommand { source: dicom_object::WriteError }, + /// Unsupported file transfer syntax {uid} + UnsupportedFileTransferSyntax { uid: std::borrow::Cow<'static, str> }, + #[snafu(whatever, display("{}", message))] Other { message: String, @@ -102,8 +111,14 @@ fn run() -> Result<(), Error> { called_ae_title, max_pdu_length, fail_first, + mut never_transcode, } = App::parse(); + // never transcode if the feature is disabled + if cfg!(not(feature = "transcode")) { + never_transcode = true; + } + tracing::subscriber::set_global_default( tracing_subscriber::FmtSubscriber::builder() .with_max_level(if verbose { Level::DEBUG } else { Level::INFO }) @@ -143,6 +158,21 @@ fn run() -> Result<(), Error> { dicom_file.sop_class_uid.to_string(), dicom_file.file_transfer_syntax.clone(), )); + + // also accept uncompressed transfer syntaxes + // as mandated by the standard + // (though it might not always be able to fulfill this) + if !never_transcode { + presentation_contexts.insert(( + dicom_file.sop_class_uid.to_string(), + uids::EXPLICIT_VR_LITTLE_ENDIAN.to_string(), + )); + presentation_contexts.insert(( + dicom_file.sop_class_uid.to_string(), + uids::IMPLICIT_VR_LITTLE_ENDIAN.to_string(), + )); + } + dicom_files.push(dicom_file); } Err(_) => { @@ -179,11 +209,19 @@ fn run() -> Result<(), Error> { } for file in &mut dicom_files { - // TODO(#106) transfer syntax conversion is currently not supported - let r: Result<_, Error> = check_presentation_contexts(file, scu.presentation_contexts()) - .whatever_context::<_, _>("Could not choose a transfer syntax"); + // identify the right transfer syntax to use + let r: Result<_, Error> = + check_presentation_contexts(file, scu.presentation_contexts(), never_transcode) + .whatever_context::<_, _>("Could not choose a transfer syntax"); match r { Ok((pc, ts)) => { + if verbose { + debug!( + "{}: Selected presentation context: {:?}", + file.file.display(), + pc + ); + } file.pc_selected = Some(pc); file.ts_selected = Some(ts); } @@ -231,7 +269,11 @@ fn run() -> Result<(), Error> { open_file(&file.file).whatever_context("Could not open listed DICOM file")?; let ts_selected = TransferSyntaxRegistry .get(&ts_uid_selected) - .whatever_context("Unsupported file transfer syntax")?; + .with_context(|| UnsupportedFileTransferSyntaxSnafu { uid: ts_uid_selected.to_string() })?; + + // transcode file if necessary + let dicom_file = into_ts(dicom_file, ts_selected, verbose)?; + dicom_file .write_dataset_with_ts(&mut object_data, ts_selected) .whatever_context("Could not write object dataset")?; @@ -440,7 +482,7 @@ fn check_file(file: &Path) -> Result { let transfer_syntax_uid = &meta.transfer_syntax.trim_end_matches('\0'); let ts = TransferSyntaxRegistry .get(transfer_syntax_uid) - .whatever_context("Unsupported file transfer syntax")?; + .with_context(|| UnsupportedFileTransferSyntaxSnafu { uid: transfer_syntax_uid.to_string() })?; Ok(DicomFile { file: file.to_path_buf(), sop_class_uid: storage_sop_class_uid.to_string(), @@ -454,27 +496,47 @@ fn check_file(file: &Path) -> Result { fn check_presentation_contexts( file: &DicomFile, pcs: &[dicom_ul::pdu::PresentationContextResult], + never_transcode: bool, ) -> Result<(dicom_ul::pdu::PresentationContextResult, String), Error> { let file_ts = TransferSyntaxRegistry .get(&file.file_transfer_syntax) - .whatever_context("Unsupported file transfer syntax")?; - // TODO(#106) transfer syntax conversion is currently not supported - let pc = pcs - .iter() - .find(|pc| { - // Check support for this transfer syntax. - // If it is the same as the file, we're good. - // Otherwise, uncompressed data set encoding - // and native pixel data is required on both ends. - let ts = &pc.transfer_syntax; - ts == file_ts.uid() - || TransferSyntaxRegistry - .get(&pc.transfer_syntax) - .filter(|ts| file_ts.is_codec_free() && ts.is_codec_free()) - .map(|_| true) - .unwrap_or(false) - }) - .whatever_context("No presentation context accepted")?; + .with_context(|| UnsupportedFileTransferSyntaxSnafu { uid: file.file_transfer_syntax.to_string() })?; + // if destination does not support original file TS, + // check whether we can transcode to explicit VR LE + + let pc = pcs.iter().find(|pc| { + // Check support for this transfer syntax. + // If it is the same as the file, we're good. + // Otherwise, uncompressed data set encoding + // and native pixel data is required on both ends. + let ts = &pc.transfer_syntax; + ts == file_ts.uid() + || TransferSyntaxRegistry + .get(&pc.transfer_syntax) + .filter(|ts| file_ts.is_codec_free() && ts.is_codec_free()) + .map(|_| true) + .unwrap_or(false) + }); + + let pc = match pc { + Some(pc) => pc, + None => { + if never_transcode || !file_ts.can_decode_all() { + whatever!("No presentation context acceptable"); + } + + // Else, if transcoding is possible, we go for it. + pcs.iter() + // accept explicit VR little endian + .find(|pc| pc.transfer_syntax == uids::EXPLICIT_VR_LITTLE_ENDIAN) + .or_else(|| + // accept implicit VR little endian + pcs.iter() + .find(|pc| pc.transfer_syntax == uids::IMPLICIT_VR_LITTLE_ENDIAN)) + // welp + .whatever_context("No presentation context acceptable")? + } + }; let ts = TransferSyntaxRegistry .get(&pc.transfer_syntax) .whatever_context("Poorly negotiated transfer syntax")?; @@ -482,6 +544,46 @@ fn check_presentation_contexts( Ok((pc.clone(), String::from(ts.uid()))) } + +// transcoding functions + +#[cfg(feature = "transcode")] +fn into_ts( + dicom_file: DefaultDicomObject, + ts_selected: &TransferSyntax, + verbose: bool, +) -> Result { + if ts_selected.uid() != dicom_file.meta().transfer_syntax() { + use dicom_pixeldata::Transcode; + let mut file = dicom_file; + if verbose { + info!( + "Transcoding file from {} to {}", + file.meta().transfer_syntax(), + ts_selected.uid() + ); + } + file.transcode(ts_selected) + .whatever_context("Failed to transcode file")?; + Ok(file) + } else { + Ok(dicom_file) + } +} + +#[cfg(not(feature = "transcode"))] +fn into_ts( + dicom_file: DefaultDicomObject, + ts_selected: &TransferSyntax, + _verbose: bool, +) -> Result { + if ts_selected.uid() != dicom_file.meta().transfer_syntax() { + panic!("Transcoding feature is disabled, should not have tried to transcode") + } else { + Ok(dicom_file) + } +} + #[cfg(test)] mod tests { use crate::App;