Data Element constructor - DICOM

This is a discussion on Data Element constructor - DICOM ; Hello again, I'm doing a class that models a Data Element in C# (as a part of a project to do a small DICOM library in C# ) and I need your feedback. I'm not sure if I fully understand ...

+ Reply to Thread
Results 1 to 10 of 10

Thread: Data Element constructor

  1. Data Element constructor

    Hello again,

    I'm doing a class that models a Data Element in C# (as a part of a
    project to do a small DICOM library in C# ) and I need your feedback.
    I'm not sure if I fully understand all the intricancies of a Data
    Element and of DICOM ...
    So my code is the following:


    // I have a base class called ByteFragment that stored a byte array
    and DataElement extends this class.
    class DataElement:ByteFragment
    {
    //
    ************************************************** ***************
    // Constructor
    // It takes a Group Number, an Element
    // Number , the VR , The Value and whether the VR
    // is implicit or explicit and whether
    // ByteEncoding is LittleEndian or BigEndian
    //
    ************************************************** ***************

    public DataElement(ByteFragment GroupNumber, ByteFragment
    ElementNumber, ByteFragment ValueRepresentation, ByteFragment
    Value,VR v, ByteEncoding b) : base()
    {

    // According to the DICOM protocol, the Group Number in a
    tag is always stored
    // in the "Little Endian" format so I have to reverse it.
    // E.g if the user gives a group number 0x4008 , it should
    be sent
    // as "08 40"
    GroupNumber.Reverse();

    Append(GroupNumber); // Append the Group Number to the
    Data Element

    // According to the DICOM protocol, the Element Number in
    a tag is always stored
    // in the "Little Endian" format.

    ElementNumber.Reverse();
    Append(ElementNumber); // Append the Element Number to
    the Data Element


    if (v == VR.Explicit)
    {
    // if the VR is Explicit then we should append the
    ValueRepresentation field to
    // the DataElement
    Append(ValueRepresentation);

    SetLengthFragment(7, 8);
    // This function is defined in the base class.
    // If we are in Explicit VR mode bytes 7 - 8 are going
    to represent the value
    // length.

    }
    else
    {
    if v == VR.Implicit)
    {
    SetLengthFragment(5,8);
    // This function is defined in the base class.
    // but If we are in Explicit VR mode bytes 5 - 8
    are going to represent the value
    // length.
    }
    }



    if (ValueRepresentation.Equals("US"))
    {
    // If the value is a character string the characters
    should be encoded in the order of
    // occurence in the string ( left to right ). So we leave
    the Value as it is.
    }
    else
    {
    if (b == ByteEncoding.LittleEndian)
    {
    // Reverse bytes.
    Value.Reverse();
    }
    }

    Append(Value);
    // Append the value to the data fragment.

    UpdateLength(b);
    // This function is defined in the base class.
    // Updates the "Length fragment" according to the
    ByteEncoding b.

    }
    }


  2. Re: Data Element constructor

    Hi sebastian,
    small library you say ...Looks like you have to implement almost a
    whole toolkit
    Anyway it's not correct you need to change it according to my last
    post.

    Thomas


  3. Re: Data Element constructor

    On Aug 13, 3:38 pm, sebastian.ha...@gmail.com wrote:
    > Hello again,
    >
    > I'm doing a class that models a Data Element in C# (as a part of a
    > project to do a small DICOM library in C# ) and I need your feedback.
    > I'm not sure if I fully understand all the intricancies of a Data
    > Element and of DICOM ...
    > So my code is the following:
    >
    > // I have a base class called ByteFragment that stored a byte array
    > and DataElement extends this class.
    > class DataElement:ByteFragment
    > {
    > //
    > ************************************************** ***************
    > // Constructor
    > // It takes a Group Number, an Element
    > // Number , the VR , The Value and whether the VR
    > // is implicit or explicit and whether
    > // ByteEncoding is LittleEndian or BigEndian
    > //
    > ************************************************** ***************
    >
    > public DataElement(ByteFragment GroupNumber, ByteFragment
    > ElementNumber, ByteFragment ValueRepresentation, ByteFragment
    > Value,VR v, ByteEncoding b) : base()
    > {
    >
    > // According to the DICOM protocol, the Group Number in a
    > tag is always stored
    > // in the "Little Endian" format so I have to reverse it.
    > // E.g if the user gives a group number 0x4008 , it should
    > be sent
    > // as "08 40"
    > GroupNumber.Reverse();
    >
    > Append(GroupNumber); // Append the Group Number to the
    > Data Element
    >
    > // According to the DICOM protocol, the Element Number in
    > a tag is always stored
    > // in the "Little Endian" format.
    >
    > ElementNumber.Reverse();
    > Append(ElementNumber); // Append the Element Number to
    > the Data Element
    >
    > if (v == VR.Explicit)
    > {
    > // if the VR is Explicit then we should append the
    > ValueRepresentation field to
    > // the DataElement
    > Append(ValueRepresentation);
    >
    > SetLengthFragment(7, 8);
    > // This function is defined in the base class.
    > // If we are in Explicit VR mode bytes 7 - 8 are going
    > to represent the value
    > // length.
    >
    > }
    > else
    > {
    > if v == VR.Implicit)
    > {
    > SetLengthFragment(5,8);
    > // This function is defined in the base class.
    > // but If we are in Explicit VR mode bytes 5 - 8
    > are going to represent the value
    > // length.
    > }
    > }
    >
    > if (ValueRepresentation.Equals("US"))
    > {
    > // If the value is a character string the characters
    > should be encoded in the order of
    > // occurence in the string ( left to right ). So we leave
    > the Value as it is.
    > }
    > else
    > {
    > if (b == ByteEncoding.LittleEndian)
    > {
    > // Reverse bytes.
    > Value.Reverse();
    > }
    > }
    >
    > Append(Value);
    > // Append the value to the data fragment.
    >
    > UpdateLength(b);
    > // This function is defined in the base class.
    > // Updates the "Length fragment" according to the
    > ByteEncoding b.
    >
    > }
    > }


    Sebastian,

    you should also consider the following:
    the tag's length is represented in different ways when the tag's VR is
    implicit, explicit, explicit but "long" (e,g,; OB)
    the tag's length is always even: you should add a padding byte at the
    end of the tag if it has an odd length. The padding byte depends from
    the VR.

    I suggest you to build a table of VRs containing:
    - the VR's word length (1 byte, 2 bytes or 4 bytes) useful for byte
    swapping
    - the padding byte needed for each VR
    - a boolean value used specify how the length should be encoded
    Then use that table during then encoding and the decoding.

    If your app is for an international audience, you should also consider
    the problem of dealing with different charsets (unicode, ansi, and so
    on).

    Paolo






  4. Re: Data Element constructor

    On Aug 13, 3:38 pm, sebastian.ha...@gmail.com wrote:
    > public DataElement(ByteFragment GroupNumber, ByteFragment
    > ElementNumber, ByteFragment ValueRepresentation, ByteFragment
    > Value,VR v, ByteEncoding b) : base()


    That's a bad design. You cannot mix representation on disk and
    representation in memory. You should separate data from filter (eg:
    reader). So your data store the group/element and the reader is in
    charge of decoding correctly.

    > {
    >
    > // According to the DICOM protocol, the Group Number in a
    > tag is always stored
    > // in the "Little Endian" format so I have to reverse it.


    No ! You did not read correctly the standard. Meta Info is -ideally-
    stored in Explicit Little Endian, but your dataset can be anything.

    > if (ValueRepresentation.Equals("US"))


    Create a mapping instead of hardcoding it (esp in a cstor).

    > if (b == ByteEncoding.LittleEndian)
    > {
    > // Reverse bytes.
    > Value.Reverse();
    > }
    > }


    I am not sure I understand what you are doing here...

    >
    > }
    > }


    HTH
    -Mathieu


  5. Re: Data Element constructor

    On Aug 14, 12:23 pm, paolo.brand...@gmail.com wrote:
    > On Aug 13, 3:38 pm, sebastian.ha...@gmail.com wrote:
    >
    >
    >
    > > Hello again,

    >
    > > I'm doing a class that models a Data Element in C# (as a part of a
    > > project to do a small DICOM library in C# ) and I need your feedback.
    > > I'm not sure if I fully understand all the intricancies of a Data
    > > Element and of DICOM ...
    > > So my code is the following:

    >
    > > // I have a base class called ByteFragment that stored a byte array
    > > and DataElement extends this class.
    > > class DataElement:ByteFragment
    > > {
    > > //
    > > ************************************************** ***************
    > > // Constructor
    > > // It takes a Group Number, an Element
    > > // Number , the VR , The Value and whether the VR
    > > // is implicit or explicit and whether
    > > // ByteEncoding is LittleEndian or BigEndian
    > > //
    > > ************************************************** ***************

    >
    > > public DataElement(ByteFragment GroupNumber, ByteFragment
    > > ElementNumber, ByteFragment ValueRepresentation, ByteFragment
    > > Value,VR v, ByteEncoding b) : base()
    > > {

    >
    > > // According to the DICOM protocol, the Group Number in a
    > > tag is always stored
    > > // in the "Little Endian" format so I have to reverse it.
    > > // E.g if the user gives a group number 0x4008 , it should
    > > be sent
    > > // as "08 40"
    > > GroupNumber.Reverse();

    >
    > > Append(GroupNumber); // Append the Group Number to the
    > > Data Element

    >
    > > // According to the DICOM protocol, the Element Number in
    > > a tag is always stored
    > > // in the "Little Endian" format.

    >
    > > ElementNumber.Reverse();
    > > Append(ElementNumber); // Append the Element Number to
    > > the Data Element

    >
    > > if (v == VR.Explicit)
    > > {
    > > // if the VR is Explicit then we should append the
    > > ValueRepresentation field to
    > > // the DataElement
    > > Append(ValueRepresentation);

    >
    > > SetLengthFragment(7, 8);
    > > // This function is defined in the base class.
    > > // If we are in Explicit VR mode bytes 7 - 8 are going
    > > to represent the value
    > > // length.

    >
    > > }
    > > else
    > > {
    > > if v == VR.Implicit)
    > > {
    > > SetLengthFragment(5,8);
    > > // This function is defined in the base class.
    > > // but If we are in Explicit VR mode bytes 5 - 8
    > > are going to represent the value
    > > // length.
    > > }
    > > }

    >
    > > if (ValueRepresentation.Equals("US"))
    > > {
    > > // If the value is a character string the characters
    > > should be encoded in the order of
    > > // occurence in the string ( left to right ). So we leave
    > > the Value as it is.
    > > }
    > > else
    > > {
    > > if (b == ByteEncoding.LittleEndian)
    > > {
    > > // Reverse bytes.
    > > Value.Reverse();
    > > }
    > > }

    >
    > > Append(Value);
    > > // Append the value to the data fragment.

    >
    > > UpdateLength(b);
    > > // This function is defined in the base class.
    > > // Updates the "Length fragment" according to the
    > > ByteEncoding b.

    >
    > > }
    > > }

    >
    > Sebastian,
    >
    > you should also consider the following:
    > the tag's length is represented in different ways when the tag's VR is
    > implicit, explicit, explicit but "long" (e,g,; OB)
    > the tag's length is always even: you should add a padding byte at the
    > end of the tag if it has an odd length. The padding byte depends from
    > the VR.
    >


    Yup. You should definitely reread PS 3.5-2007. Tables are pretty
    clear.

    > I suggest you to build a table of VRs containing:
    > - the VR's word length (1 byte, 2 bytes or 4 bytes) useful for byte
    > swapping
    > - the padding byte needed for each VR


    Not required for a reader, and often implementation do not respect it.

    > - a boolean value used specify how the length should be encoded


    I do not understand that ? VR in Explicit should be enough to know how
    to encode the VL.

    > Then use that table during then encoding and the decoding.
    >
    > If your app is for an international audience, you should also consider
    > the problem of dealing with different charsets (unicode, ansi, and so
    > on).


    That's a problem of interpreting the data, not reading. Array of bytes
    should be fine.

    2 cents,
    -Mathieu


  6. Re: Data Element constructor

    Mathieu,

    > > I suggest you to build a table of VRs containing:
    > > - the VR's word length (1 byte, 2 bytes or 4 bytes) useful for byte
    > > swapping
    > > - the padding byte needed for each VR

    >
    > Not required for a reader, and often implementation do not respect it.
    >


    Personally I think that building a table that specifies each VR's
    attributes is better than a list of "if ifelse ifelse ifelse...." in
    the code. Then the company calls a contractor to fix it and the code
    ends up in http://worsethanfailure.com/
    With a little code you could cover all the VRs: byte swapping if
    necessary and also the padding of odd lengths if have to build a Dicom
    Stream

    > > - a boolean value used specify how the length should be encoded

    >
    > I do not understand that ? VR in Explicit should be enough to know how
    > to encode the VL.
    >


    Even when the VR is explicit, some VRs require that their length is
    encoded using 4 bytes and not the usual 2.

    Paolo


  7. Re: Data Element constructor

    On Aug 14, 2:27 pm, paolo.brand...@gmail.com wrote:
    > Mathieu,
    >
    > > > I suggest you to build a table of VRs containing:
    > > > - the VR's word length (1 byte, 2 bytes or 4 bytes) useful for byte
    > > > swapping
    > > > - the padding byte needed for each VR

    >
    > > Not required for a reader, and often implementation do not respect it.

    >
    > Personally I think that building a table that specifies each VR's
    > attributes is better than a list of "if ifelse ifelse ifelse...." in
    > the code. Then the company calls a contractor to fix it and the code
    > ends up inhttp://worsethanfailure.com/
    > With a little code you could cover all the VRs: byte swapping if
    > necessary and also the padding of odd lengths if have to build a Dicom
    > Stream


    Yes, I also suggested a table to the original poster.
    The only issue with a table is the linear lookup. In gdcm, I decided
    to redo the design of the VR to have a O(1) lookup.

    > > > - a boolean value used specify how the length should be encoded

    >
    > > I do not understand that ? VR in Explicit should be enough to know how
    > > to encode the VL.

    >
    > Even when the VR is explicit, some VRs require that their length is
    > encoded using 4 bytes and not the usual 2.


    s/Even/Only/g (In Implicit there is no such thing as 2bytes VL).

    Anyway I misunderstood your original comment, and yes indeed your
    table need to have this boolean value. Sorry for the misunderstanding.

    -Mathieu



  8. Re: Data Element constructor

    Thanks for the help guys ....
    Ok .... So what about this in pseudo - code....

    constructor DataElement(Group, Element, Value )
    begin

    CASE 1: (endianness == LittleEndian)
    {
    swap Group bytes;
    swap Element bytes;
    if ( VR Implicit)
    {
    Set Length (Bytes 5 - 8 ) and Compute the Length Value in Little
    Endian Format
    }
    else
    {
    Lookup Group Number and Element Number in the Dictionary , Get VR
    Copy VR to Bytes 5 - 6
    Set Length (Bytes 7 - 8) and Compute the Length Value in Little
    Endian Format
    }

    if (VR = "CS")
    {
    Leave the value as it is because it's an ordered character string
    }
    else
    {
    Reverse Value
    }
    }

    CASE 2: (Endiannes = Big Endian)
    {
    Do not swap group number, element number
    if ( VR Implicit)
    {
    Set Length (Bytes 5 - 8 ) and Compute the Length Value in Big
    Endian Format
    }
    else
    {
    Lookup Group Number and Element Number in the Dictionary , Get VR
    Copy VR to Bytes 5 - 6
    Set Length (Bytes 7 - 8) and Compute the Length Value in Big
    Endian Format
    }
    Do not swap value
    }
    end

    so is this correct now ?? (except for the fact that it does not deal
    with some exceptions )







  9. Re: Data Element constructor

    (continued)

    DataElement( 0x0010, 0x0010, "JOHN") should produce the following
    byte fragments:

    using Explicit VR, Big Endian
    00 10 00 10 50 4E 00 04 4A 4F 48 4E

    using Explicit VR, Little Endian
    10 00 10 00 50 4E 04 00 4A 4F 48 4E

    using Implicit VR, Big Endian :
    00 10 00 10 00 00 00 04 4A 4F 48 4E

    using Implicit VR , Little Endian
    10 00 10 00 04 00 00 00 4A 4F 48 4E

    is this correct ?




  10. Re: Data Element constructor

    The byte fragments shown are correct. There is no Implicit VR Big
    Endian used as standard DICOM transfer syntax. So you can skip this
    one.

    Thomas


+ Reply to Thread