Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Color-flow support #94

Open
forderud opened this issue Aug 28, 2018 · 6 comments
Open

API: Color-flow support #94

forderud opened this issue Aug 28, 2018 · 6 comments
Milestone

Comments

@forderud
Copy link
Member

forderud commented Aug 28, 2018

Leverage DICOM standard to the extent possible.

API: Add color-flow support (GE proposal): #140

@forderud forderud added this to the 3.x milestone Aug 28, 2018
@forderud forderud modified the milestones: 3.x, 2.x Aug 28, 2018
@SteveKauffman
Copy link
Member

I've discussed this with Joe Luszcz. The color-flow and echo/color blending that is defined in the DICOM Enhanced 3D standard was then also generalized into the DICOM Presentation State object, which is intended for use with ultrasound and other modalities' images. He suggests that Open4D may want to simply use the DICOM presentation state object to define the blending (and also use it for definition of the various MPR slice views).

@forderud forderud modified the milestones: 2.x, 3.x Apr 18, 2019
@forderud forderud modified the milestones: 3.x, 2.x Jun 22, 2019
@ravimanaguli
Copy link

[1] TestViewer and DummyLoader are essential for detail investigation and proper interoperability of loader DLL. Difficult to understand usage of changed interface classes without them.

[2] Is ColorMapType TYPE_FLOW_ARB color map used for only slice image or used for VR image as well?

[3] Please clarify IMAGE_FORMAT_FREQ8POW8 data format.

  1. Is velocity data (signed 8bit) assigned LSB side (bit0..bit7)?
  2. Is velocity data 2's complement expression? (0..127-->0..127, 128..255-->-128..-1)
  3. Is range of velocity data -127..127? Is value -128 not used?
  4. Is input of TYPE_FLOW_COLOR type color map IMAGE_FORMAT_FREQ8POW8 type value?"

[4]
Please combine CF support IImage3d.idl and PS support as one.

[5]
The interface class IImage3dSource has been changed to support CF.
Is it necessary to support both old and new IImage3dSource?
It is difficult to handle new and old Loader DLLs at the same time. If the methods are different with the same class name, the DLL design becomes complicated.

[6]
If there no need to support the old interface and only support the new interface, it's wasteful to release a Loader DLL with only tissue data and the old interface.
Would require rework of both loader DLL and Viewer. And it is used only for a short time.
DLL developers and Viewer developers will be confused, if they develop release version before the interface is settled.

[7]
If it is necessary to support both old and new IImage3dSource, please propose interface specifications which have lower compatibility.

[8]
Is there no need to provide 32it loader DLL? Only 64bit DLL is required?

@forderud
Copy link
Member Author

I've discussed this with Joe Luszcz. The color-flow and echo/color blending that is defined in the DICOM Enhanced 3D standard was then also generalized into the DICOM Presentation State object, which is intended for use with ultrasound and other modalities' images. He suggests that Open4D may want to simply use the DICOM presentation state object to define the blending (and also use it for definition of the various MPR slice views).

@SteveKauffman This is probably a good idea, but the API implications are a bit unclear to me. Would it be possible for you to investigate the implications, and come up with an alternative API proposal that is based on the DICOM presentation state object?

@ravimanaguli
Copy link

[2] Is ColorMapType TYPE_FLOW_ARB color map used for only slice image or used for VR image as well?

In [2], VR refers to volume rendered image.

@forderud
Copy link
Member Author

[1] TestViewer and DummyLoader are essential for detail investigation and proper interoperability of loader DLL. Difficult to understand usage of changed interface classes without them.

Agree. I promise to update the test code after we've agreed on the spec.

[2] Is ColorMapType TYPE_FLOW_ARB color map used for only slice image or used for VR image as well?

TYPE_FLOW_ARB will be relevant for all code that want to convert color-flow data from IMAGE_FORMAT_FREQ8POW8 format to RGB(A) format. This includes 3D volume slicing and volume rendering algorithms.

[3] Please clarify IMAGE_FORMAT_FREQ8POW8 data format.

  1. Is velocity data (signed 8bit) assigned LSB side (bit0..bit7)?
  2. Is velocity data 2's complement expression? (0..127-->0..127, 128..255-->-128..-1)
  3. Is range of velocity data -127..127? Is value -128 not used?
  4. Is input of TYPE_FLOW_COLOR type color map IMAGE_FORMAT_FREQ8POW8 type value?"

I've now updated the PR to clarify that the formats are documented in byte order. Integrators won't need answers to the other questions in order to integrate CF support in their PACS, so I would prefer to leave velocity range etc. unspecified.

[4] Please combine CF support IImage3d.idl and PS support as one.

I would prefer to keep the proposals separate, so that they can be reviewed & merged independently.

[5] The interface class IImage3dSource has been changed to support CF.
Is it necessary to support both old and new IImage3dSource?
It is difficult to handle new and old Loader DLLs at the same time. If the methods are different with the same class name, the DLL design becomes complicated.

Loaders will only support one version of the interface, whereas integrators can chose to support both in parallel. However, things become simpler if all stakeholders agree to focus on one version at a time.

[6] If there no need to support the old interface and only support the new interface, it's wasteful to release a Loader DLL with only tissue data and the old interface.
Would require rework of both loader DLL and Viewer. And it is used only for a short time.
DLL developers and Viewer developers will be confused, if they develop release version before the interface is settled.

This is not a technical question. Please address that through other channels.

[7] If it is necessary to support both old and new IImage3dSource, please propose interface specifications which have lower compatibility.

I don't understand your comment. We currently have version 1.2 or the API which is B-mode-only. After this extension, we'll probably get a version 2.0 (or similar) that will also support CF. Integrators will be able to chose which version(s) of the API they want to support.

[8] Is there no need to provide 32it loader DLL? Only 64bit DLL is required?

The CF API proposal will not affect "bitness" requirements for loader binaries. In GE, we're currently only releasing 64bit loader DLLs. This loader is, however, also compatible with 32bit PACS systems.

@lassoan
Copy link

lassoan commented Jul 27, 2020

What is the current status of this?

I've tried to load 3D CF image using GE_CVUS_Loader-2.25.0 and GE_CVUS_Loader-3.9.0 but LoadFile fails (the same code works well for B-mode). Is this expected or am I doing something wrong?

I've seen "Open4D" mentioned here, but I could not find any information about it online (other than just a JASE paper mentioning it). Does it exist? How it relates to Image3dAPI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants