almost there

include/dicom/dicom.h

@@ -530,12 +530,12 @@ uint32_t dcm_dict_tag_from_keyword(const char *keyword);
 /**
  * Find the Value Representation for a tag.
  *
- * This will return DCM_VR_ERROR if the tag is unknown, or does not have a
- * unique Value Representation.
+ * This will return DCM_VR_ERROR if the tag is unknown. If the tag does not
+ * have a unique VR, it will pick a sensible default value.
  *
  * :param tag: Attribute Tag
  *
- * :return: the unique Value Representation for this tag, or DCM_VR_ERROR
+ * :return: the Value Representation for this tag, or DCM_VR_ERROR
  */
 DCM_EXTERN
 DcmVR dcm_vr_from_tag(uint32_t tag);

src/dicom-dict-build.c

@@ -15,8 +15,8 @@
 #include "uthash.h"
 
 #include <dicom/dicom.h>
+
 #include "pdicom.h"
-#include "dicom-dict-tables.h"
 
 #define EMPTY 0xffffffff
 #define MAX_PROBES 10

src/dicom-dict-tables.c

@@ -1,6 +1,6 @@
 #include <dicom/dicom.h>
+
 #include "pdicom.h"
-#include "dicom-dict-tables.h"
 
 /* This ordering must match the enum in dicom.h.
  *
@@ -376,6 +376,7 @@ const struct _DcmAttribute dcm_attribute_table[] = {
     {0X00089459, DCM_VR_TAG_FL, "RecommendedDisplayFrameRateInFloat"},
     {0X00089460, DCM_VR_TAG_CS, "SkipFrameRangeFlag"},
     {0X00090010, DCM_VR_TAG_LO, "PrivateCreator"},
+    {0X00091000, DCM_VR_TAG_OB, "PrivateData"},
     {0X00100010, DCM_VR_TAG_PN, "PatientName"},
     {0X00100020, DCM_VR_TAG_LO, "PatientID"},
     {0X00100021, DCM_VR_TAG_LO, "IssuerOfPatientID"},

src/dicom-dict.c

@@ -20,9 +20,9 @@
 #include "uthash.h"
 
 #include <dicom/dicom.h>
+
 #include "pdicom.h"
 #include "dicom-dict-lookup.h"
-#include "dicom-dict-tables.h"
 
 #define LOOKUP(table, field, hash, key, key_len, out) do {		\
         unsigned hash_value;						\
@@ -85,9 +85,18 @@ const char *dcm_dict_str_from_vr(DcmVR vr)
 }
 
 
-
 DcmVRClass dcm_dict_vr_class(DcmVR vr)
 {
+    DcmVRTag vr_tag = (DcmVRTag) vr;
+
+    // these are all ints of various sizes and signedness
+    if (vr == DCM_VR_TAG_OB_OW ||
+        vr == DCM_VR_TAG_US_OW ||
+        vr == DCM_VR_TAG_US_SS ||
+        vr == DCM_VR_TAG_US_SS_OW) {
+        return DCM_VR_CLASS_NUMERIC_INTEGER;
+    }
+
     if (vr >= 0 && vr < DCM_VR_LAST) {
         return dcm_vr_table[(int)vr].vr_class;
     }
@@ -137,6 +146,20 @@ static const struct _DcmAttribute *attribute_from_tag(uint32_t tag)
         tag = 0x00080000;
     }
 
+    // private creator elements are (gggg, 0010 - 00ff) where gggg is odd
+    if ((tag & (1 << 16)) &&
+        (tag & 0xffff) >= 0x0010 &&
+        (tag & 0xffff) <= 0x00ff) {
+        tag = TAG_PRIVATE_CREATOR;
+    }
+
+    // private data elements are (gggg, 1000 - ffff) where gggg is odd
+    if ((tag & (1 << 16)) &&
+        (tag & 0xffff) >= 0x1000 &&
+        (tag & 0xffff) <= 0xffff) {
+        tag = TAG_PRIVATE_DATA;
+    }
+
     LOOKUP(dcm_attribute_table, tag,
            dcm_attribute_from_tag, &tag, sizeof(tag),
            attribute);
@@ -148,12 +171,23 @@ static const struct _DcmAttribute *attribute_from_tag(uint32_t tag)
 // this will also fail for unknown or retired public tags
 bool dcm_is_public_tag(uint32_t tag)
 {
-    return attribute_from_tag(tag) != NULL;
+    return !dcm_is_private_tag(tag) &&
+        attribute_from_tag(tag) != NULL;
 }
 
 
 bool dcm_is_private_tag(uint32_t tag)
 {
+    uint16_t group_number = tag >> 16;
+
+    // the spec says groups 1, 3, 5, 7 are not allowed
+    if (group_number == 0x1 ||
+        group_number == 0x3 ||
+        group_number == 0x5 ||
+        group_number == 0x7) {
+        return false;
+    }
+
     return tag & (1 << 16);
 }
 
@@ -167,15 +201,40 @@ bool dcm_is_valid_tag(uint32_t tag)
 }
 
 
-DcmVR dcm_vr_from_tag(uint32_t tag)
+// the set of possible VRs for this tag
+DcmVRTag dcm_vr_tag_from_tag(uint32_t tag)
 {
     const struct _DcmAttribute *attribute;
 
     if (!(attribute = attribute_from_tag(tag))) {
         return DCM_VR_ERROR;
     }
 
-    return (DcmVR) attribute->vr_tag;
+    return attribute->vr_tag;
+}
+
+
+// pick a default VR for this tag
+DcmVR dcm_vr_from_tag(uint32_t tag)
+{
+    DcmVRTag vr_tag = dcm_vr_tag_from_tag(tag);
+
+    switch (vr_tag) {
+        case DCM_VR_TAG_OB_OW:
+            return DCM_VR_OB;
+
+        case DCM_VR_TAG_US_OW:
+            return DCM_VR_US;
+
+        case DCM_VR_TAG_US_SS:
+            return DCM_VR_US;
+
+        case DCM_VR_TAG_US_SS_OW:
+            return DCM_VR_US;
+
+        default:
+            return vr_tag;
+    }
 }
 
 

src/dicom-file.c

@@ -20,6 +20,7 @@
 #include "utarray.h"
 
 #include <dicom/dicom.h>
+
 #include "pdicom.h"
 
 typedef enum _DcmLayout {

src/dicom-io.c

@@ -31,6 +31,7 @@
 #include <time.h>
 
 #include <dicom/dicom.h>
+
 #include "pdicom.h"
 
 /* The size of the input buffer we use.

src/dicom-parse.c

@@ -21,6 +21,7 @@
 #include "utarray.h"
 
 #include <dicom/dicom.h>
+
 #include "pdicom.h"
 
 
@@ -253,7 +254,7 @@ static bool parse_element(DcmParseState *state,
 
 static bool parse_element_header(DcmParseState *state,
                                  uint32_t *tag,
-                                 DcmVR *vr,
+                                 DcmVRTag *vr_tag,
                                  uint32_t *length,
                                  int64_t *position)
 {
@@ -264,34 +265,40 @@ static bool parse_element_header(DcmParseState *state,
     if (state->implicit) {
         // this can be an ambiguous VR, eg. pixeldata is allowed in implicit
         // mode and has to be disambiguated later from other tags
-        *vr = dcm_vr_from_tag(*tag);
-        if (*vr == DCM_VR_ERROR) {
+        *vr_tag = dcm_vr_tag_from_tag(*tag);
+        if (*vr_tag == DCM_VR_ERROR) {
             dcm_error_set(state->error, DCM_ERROR_CODE_PARSE,
                           "reading of data element header failed",
-                          "tag %08x not allowed in implicit mode", *tag);
+                          "tag %08x not allowed in implicit mode", *vr_tag);
             return false;
         }
 
         if (!read_uint32(state, length, position)) {
             return false;
         }
     } else {
-        // Value Representation
+        // explicit mode
         char vr_str[3];
         if (!dcm_require(state, vr_str, 2, position)) {
             return false;
         }
         vr_str[2] = '\0';
-        *vr = dcm_dict_vr_from_str(vr_str);
+        DcmVR vr = dcm_dict_vr_from_str(vr_str);
+        if (vr == DCM_VR_ERROR) {
+            dcm_error_set(state->error, DCM_ERROR_CODE_PARSE,
+                          "reading of data element header failed",
+                          "unknown VR '%s'", vr_str);
+            return false;
+        }
 
-        if (!dcm_is_valid_vr_for_tag(*vr, *tag)) {
+        if (!dcm_is_valid_vr_for_tag(vr, *tag)) {
             dcm_error_set(state->error, DCM_ERROR_CODE_PARSE,
                           "reading of data element header failed",
                           "tag %08x cannot have VR '%s'", *tag, vr_str);
             return false;
         }
 
-        if (dcm_dict_vr_header_length(*vr) == 2) {
+        if (dcm_dict_vr_header_length(vr) == 2) {
             // These VRs have a short length of only two bytes
             uint16_t short_length;
             if (!read_uint16(state, &short_length, position)) {
@@ -311,10 +318,14 @@ static bool parse_element_header(DcmParseState *state,
                               "reading of data element header failed",
                               "unexpected value for reserved bytes "
                               "of data element %08x with VR '%s'",
-                              tag, vr);
+                              tag, vr_str);
                 return false;
             }
         }
+
+        // this will always be a unique VR, but we store as a VRTag for
+        // consistency with the implicit path
+        *vr_tag = (DcmVRTag) vr;
     }
 
     return true;
@@ -557,11 +568,11 @@ static bool parse_pixeldata(DcmParseState *state,
 
 static bool parse_element_body(DcmParseState *state,
                                uint32_t tag,
-                               DcmVR vr,
+                               DcmVRTag vr_tag,
                                uint32_t length,
                                int64_t *position)
 {
-    DcmVRClass vr_class = dcm_dict_vr_class(vr);
+    DcmVRClass vr_class = dcm_dict_vr_class(vr_tag);
     size_t size = dcm_dict_vr_size(vr);
     char *value;
 
@@ -574,7 +585,7 @@ static bool parse_element_body(DcmParseState *state,
     if (tag == TAG_PIXEL_DATA ||
         tag == TAG_FLOAT_PIXEL_DATA ||
         tag == TAG_DOUBLE_PIXEL_DATA) {
-        return parse_pixeldata(state, tag, vr, length, position);
+        return parse_pixeldata(state, tag, vr_tag, length, position);
     }
 
     dcm_log_debug("Read Data Element body '%08x'", tag);
@@ -632,7 +643,7 @@ static bool parse_element_body(DcmParseState *state,
                 !state->parse->element_create(state->error,
                                               state->client,
                                               tag,
-                                              vr,
+                                              vr_tag,
                                               value,
                                               length)) {
                 if (value_free != NULL) {
@@ -661,7 +672,7 @@ static bool parse_element_body(DcmParseState *state,
             int64_t seq_position = 0;
             if (!parse_element_sequence(state,
                                         tag,
-                                        vr,
+                                        vr_tag,
                                         length,
                                         &seq_position)) {
                 return false;
@@ -685,10 +696,10 @@ static bool parse_element(DcmParseState *state,
                           int64_t *position)
 {
     uint32_t tag;
-    DcmVR vr;
+    DcmVRTag vr_tag;
     uint32_t length;
-    if (!parse_element_header(state, &tag, &vr, &length, position) ||
-        !parse_element_body(state, tag, vr, length, position)) {
+    if (!parse_element_header(state, &tag, &vr_tag, &length, position) ||
+        !parse_element_body(state, tag, vr_tag, length, position)) {
         return false;
     }
 
@@ -713,10 +724,11 @@ static bool parse_toplevel_dataset(DcmParseState *state,
         }
 
         uint32_t tag;
-        DcmVR vr;
+        DcmVRTag vr_tag;
         uint32_t length;
         int64_t element_start = 0;
-        if (!parse_element_header(state, &tag, &vr, &length, &element_start)) {
+        if (!parse_element_header(state,
+                &tag, &vr_tag, &length, &element_start)) {
             return false;
         }
 
@@ -799,9 +811,9 @@ bool dcm_parse_group(DcmError **error,
     /* Groups start with (xxxx0000, UL, 4), meaning a 32-bit length value.
      */
     uint32_t tag;
-    DcmVR vr;
+    DcmVRTag vr_tag;
     uint32_t length;
-    if (!parse_element_header(&state, &tag, &vr, &length, &position)) {
+    if (!parse_element_header(&state, &tag, &vr_tag, &length, &position)) {
         return false;
     }
     uint16_t element_number = tag & 0xffff;
@@ -825,7 +837,8 @@ bool dcm_parse_group(DcmError **error,
 
     while (position < group_length) {
         int64_t element_start = 0;
-        if (!parse_element_header(&state, &tag, &vr, &length, &element_start)) {
+        if (!parse_element_header(&state,
+                &tag, &vr_tag, &length, &element_start)) {
             return false;
         }
 
@@ -883,10 +896,10 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
     dcm_log_debug("parsing PixelData");
 
     uint32_t tag;
-    DcmVR vr;
+    DcmVRTag vr_tag;
     uint32_t length;
     uint32_t value;
-    if (!parse_element_header(&state, &tag, &vr, &length, &position)) {
+    if (!parse_element_header(&state, &tag, &vr_tag, &length, &position)) {
         return false;
     }
 

src/pdicom.h

@@ -6,6 +6,8 @@
 typedef SSIZE_T ssize_t;
 #endif
 
+#include "dicom-dict-tables.h"
+
 #ifndef NDEBUG
 #  define DCM_DEBUG_ONLY( ... ) __VA_ARGS__
 #else
@@ -44,6 +46,8 @@ typedef SSIZE_T ssize_t;
 #define USED(x) (void)(x)
 
 #define TAG_TRANSFER_SYNTAX_UID                     0x00020010
+#define TAG_PRIVATE_CREATOR                         0x00090010
+#define TAG_PRIVATE_DATA                            0x00091000
 #define TAG_DIMENSION_INDEX_VALUES                  0x00209157
 #define TAG_REFERENCED_IMAGE_NAVIGATION_SEQUENCE    0x00480200
 #define TAG_PLANE_POSITION_SLIDE_SEQUENCE           0x0048021a
@@ -68,6 +72,7 @@ void dcm_free_string_array(char **strings, int n);
 size_t dcm_dict_vr_size(DcmVR vr);
 uint32_t dcm_dict_vr_capacity(DcmVR vr);
 int dcm_dict_vr_header_length(DcmVR vr);
+DcmVRTag dcm_vr_tag_from_tag(uint32_t tag);
 
 #define DCM_SWITCH_NUMERIC(VR, OPERATION) \
     switch (VR) { \