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

Improved installation support for DICOM [Native*] libraries #14

Closed
anders9ustafsson opened this issue May 28, 2015 · 6 comments
Closed
Assignees
Milestone

Comments

@anders9ustafsson
Copy link
Contributor

One of the most recurring issues on the Google Groups forum is that the DICOM [Native*] (codec) libraries are not properly recognized when invoking the DICOM library.

I don't have any clear answers on what needs to be done (or even if something needs to be done), but I think it will be good for the user experience of fo-dicom if the usage of the DICOM [Native*] libraries would be more fail-safe.

Perhaps installation via the NuGet package can be improved? It is not entirely straightforward, but it is possible to have conditional installment of NuGet packages based on CPU architecture, see here for example.

Please share your thoughts on this.

@IanYates
Copy link
Contributor

I've noticed it also matters if the app in question is a web application versus a regular command line / desktop application (binaries are within the /bin folder for a web app, etc).

I think in my old local copy I had modified the loader code so that I could pass in a path. I also had it do some extra logging around what was found so I could debug it in production. Also, at that time, I had the loader specifically choose to examine one DLL or the other based on the bitness of the current process. That allowed for easy deployments in x86 or x64 environments (or x64 but with the IIS app in 32-bit mode) without having to worry about anything.

@maikel87
Copy link

maikel87 commented Jun 6, 2015

I was doing a WADO service and had trouble reading the DICOM codecs, the solution to this was, in the class "DicomTranscoder", method "LoadCodecs", change this line "Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)" by "Path.GetDirectoryName(new Uri(Assembly.GetExecutingAssembly().CodeBase).LocalPath)", this solution work fine in any app(console/desktop/web)

@anders9ustafsson
Copy link
Contributor Author

@maikel87 Thanks for your input!

@hdesouky
Copy link
Contributor

Also there is too many complaints about VC runtime, there is no direct instructions for devs to pre-install VC runtime (x86 or x64)
I don't know if Nuget supports bundling x86 and x64 on the same package or we should have 2?

@IanYates
Copy link
Contributor

Thinking about this, could we embed the x64 and x86 DLLs as resources in the dicom.dll assembly? At runtime, when they're first required (which may be never, depending on the app), fo-dicom checks the current process' bitness, extracts the x86 or x64 DLL and loads it.
See here http://blogs.msdn.com/b/microsoft_press/archive/2010/02/03/jeffrey-richter-excerpt-2-from-clr-via-c-third-edition.aspx
One commenter asks about using this for libraries (like fo-DICOM). We just need to have the fo-DICOM methods that might call into those DLLs first make a call to some thread-safe init() function.

Also, to round out the reading...
http://stackoverflow.com/questions/9228423/how-to-use-an-dll-load-from-embed-resource
And
http://stackoverflow.com/questions/6893060/load-an-assembly-from-an-embedded-resource

@anders9ustafsson
Copy link
Contributor Author

@IanYates I would argue that embedding the DLLs as resources would make the DICOM.dll unnecessarily big in those cases when image codecs are not requested.

I have looked a little closer into this issue this morning. I created a unit test for the DicomTranscoder.GetCodec method, and it is then clear that even in this simple unit test scenario native assembly loading fails. As @maikel87 hinted, the problem appears to lie in the use of Assembly.Location. Assembly.Location refers to the location from which the assembly is loaded, which may very well be a shadow-copy location (which is often the case for unit testing within Visual Studio or ASP.NET projects, if I am not mistaken). Assembly.CodeBase on the other hand refers to the original location of the assembly, which is really what the DicomTranscoder.LoadCodecs method should be looking for when the path argument is null.

I am preparing a PR where the path setting in LoadCodecs is changed to the following:

path = Path.GetDirectoryName(new Uri(Assembly.GetExecutingAssembly().EscapedCodeBase).LocalPath);

This is basically what @maikel87 suggested, except that I am using EscapedCodeBase instead of CodeBase, since the escaped version should be more fail-safe in the URI context.

This fix did the trick for the unit test. It would be really good if someone could also verify that it improves the situation for web projects.

@anders9ustafsson anders9ustafsson self-assigned this Jun 24, 2015
@anders9ustafsson anders9ustafsson added this to the 1.1.0 milestone Jun 24, 2015
anders9ustafsson added a commit to anders9ustafsson/fo-dicom that referenced this issue Jun 25, 2015
anders9ustafsson added a commit that referenced this issue Jun 25, 2015
Fail-safer search for DICOM directory location. Connected to #14
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