On 23 November 2011 23:05, Kris Thielemans <kris.f.thielem...@gmail.com> wrote:
> Hi
>
> I'm modifying octave-forge/extra/dicom/src/dicominfo.cpp to handle data
> which are arrays, i.e. VM>1. I realize that not many people will know DICOM,
> let alone GDCM, but I'd appreciate any feedback in any case. (There's some
> generic octave questions at the end)
>
> Also, it would be good if someone could test the current dicominfo on
> systems with different byteorders. I suspect that it'll only work when the
> byteorder of the dicom file is the same as the one of the machine you're
> running it on.
>
> I had a look at gdcm example code and it seems to me that we cannot really
> use memcpy to get from the dicom raw data to octave data. This presumably
> wouldn't work when byteswapping is needed (or other "transfer syntax"). In
> any case, I wouldn't know if memcpy would work with arrays (I'm not sure if
> it's guaranteed by DICOM that the subsequent members are stored
> consecutively, although it's very likely of course). In an attempt to handle
> this (and avoid horrible code repetition) I came up with the following
> modification for element2value() in dicominfo.cpp:
>
> ------------------ current code -------------------
> if (vr & gdcm::VR::US) {// unsigned short
> uint16_t usval ;
> memcpy(&usval, elem->GetByteValue()->GetPointer(), 2);
> *ov=usval;
> if(chatty) octave_stdout << '[' << usval << "]\n";
> } else ...
> // almost repeated for lots of VRs and types
> return DICOM_OK;
> -------------------- new code -----------------
> if (vr & gdcm::VR::US) {// unsigned short
> return element2intvalueHelper<gdcm::VR::US>(ov, elem, chatty);
> } else ...
> // repeated for lots of VRs
>
> ---------------------- new function definition (in dicominfo.cpp)
> ---------------------
> template <gdcm::VR::VRType vrtype>
> int element2intvalueHelper(octave_value *ov, const gdcm::DataElement * elem,
> const int chatty) {
> if( !elem->IsEmpty() ) {
> // find type that corresponds to this VR
> typedef typename gdcm::VRToType<vrtype >::Type actual_type;
> gdcm::Element<vrtype,gdcm::VM::VM1_n> el;
> el.Set( elem->GetValue() );
> intNDArray<octave_int<actual_type> > val(dim_vector (el.GetLength(),1));
> for (unsigned i=0; i<el.GetLength(); ++i)
> val(i) = el.GetValue(i);
> *ov=val;
> if(chatty) octave_stdout << '[' << val << "]\n";
> return DICOM_OK;
> }
> else
> {
> return DICOM_NOTHING_ASSIGNED;
> }
> }
> -------------------------------------------------------------------------
>
> This seems to work. It might be somewhat inefficient though, as we're
> creating a temporary array, copying to an octave_value, and then getting rid
> of it. I'm not sure how to avoid this, or even if it's an issue.
> Also, it's annoying that I need to have 2 separate code sets for ints and
> floats (as I need intNDArray<octave_int<short> > etc because of missing
> constructors in octave_value).
>
> Any suggestions for improvements?
>
> Thanks
>
> Kris Thielemans
> Algorithms and Software Consulting Ltd
> Honorary Lecturer at Imperial College London
It's really nice to see someone else working on the dicom package.
Have you timed it on your system? How much slower is it?
It did occur to me that memcpy may cause problems. Do you have a
system where it does? I have used it on a few and it always works. I
guess having someone with different types of mac to have a try might
be useful.
Do you have write acces to sf.net svn? If not please can this be
granted. Carnë, is that your remit now?
Driver issues with my linux system are still consuming my spare time,
so I have not actually tried your code yet. I would not actually
commit this yet unless;
- The timed performance degradation is tiny, or,
- We have confirmation of our suspicion that it does cause a problem
on some systems.
--
/* andy buckle */
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure
contains a definitive record of customers, application performance,
security threats, fraudulent activity, and more. Splunk takes this
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Octave-dev mailing list
Octave-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/octave-dev
