Fix encapsulated pixeldata handling (#103)

* remove restriction for bits stored, because its value can be modality specific e.g. in case of CT it can be 12,13,14,15,16

* fix offset reading and frame reading for encapsulated pixel data

* fix offset calculation for encapsualted fragments: length was essentially not read, undefined length was assumed
use more intuitive offset calculation: previous offset + tag and length size + current length

* split regular and encapsulated frame parsing
fix indexing problem when creating offsets table
fix frame size calculation: bits allocated was not considered

* fix offset calculation for non-encapsulated frames: bits allocated was not considered

* coding style, code documentation, review fixes: simplification, error handling, etc

* functional tests for encapsulated pixel data handling

* simplify BOT construction
avoid overflow for length value in dcm_parse_encapsulated_frame

* simplify first BOT offset calculation
restore some error handling

* review fixesL coding style, comments, error messages

* indentation

---------

Co-authored-by: Antal Ispanovity <aispanovity@workstation>

Author: weanti

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## main
 
+* add support for encapsulated pixel data reading [weanti]
 * fix one-byte overread into struct padding [bgilbert]
 * support single-frame DICOM images and allow BitsStored > 8 [tokyovigilante]
 * fix error handling for string values over max length [arngaillard]

data/test_files/generated_encapsulated_defined_bot_1_to_1.dcm

@@ -1811,13 +1811,6 @@ DcmFrame *dcm_frame_create(DcmError **error,
         return NULL;
     }
 
-    if (bits_stored != 1 && bits_stored % 8 != 0) {
-        dcm_error_set(error, DCM_ERROR_CODE_INVALID,
-                      "constructing frame item failed",
-                      "wrong number of bits stored");
-        return NULL;
-    }
-
     if (pixel_representation != 0 && pixel_representation != 1) {
         dcm_error_set(error, DCM_ERROR_CODE_INVALID,
                       "constructing frame item failed",

data/test_files/generated_encapsulated_defined_bot_2_to_1.dcm

@@ -1365,12 +1365,25 @@ DcmFrame *dcm_filehandle_read_frame(DcmError **error,
         return NULL;
     }
 
-    uint32_t length;
-    char *frame_data = dcm_parse_frame(error,
-                                       filehandle->io,
-                                       filehandle->implicit,
-                                       &filehandle->desc,
-                                       &length);
+    const char *syntax = dcm_filehandle_get_transfer_syntax_uid(filehandle);
+    uint32_t length = 0;
+    char* frame_data = NULL;
+    if (dcm_is_encapsulated_transfer_syntax(syntax)) {
+        int64_t frame_end_offset = frame_number < filehandle->num_frames ? 
+                                    filehandle->offset_table[i + 1] : 0xFFFFFFFF;
+        frame_data = dcm_parse_encapsulated_frame(error,
+                                                  filehandle->io,
+                                                  filehandle->implicit,
+                                                  frame_end_offset,
+                                                  &length );
+    } else {
+        frame_data = dcm_parse_frame(error,
+                                     filehandle->io,
+                                     filehandle->implicit,
+                                     &filehandle->desc,
+                                     &length);
+    }
+
     if (frame_data == NULL) {
         return NULL;
     }

data/test_files/generated_encapsulated_defined_bot_2_to_2.dcm

@@ -961,8 +961,6 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
 
         // 0 in the BOT is the offset to the start of frame 1, ie. here
         *first_frame_offset = position;
-
-        position = 0;
         for (int i = 0; i < num_frames; i++) {
             if (!read_tag(&state, &tag, &position) ||
                 !read_uint32(&state, &length, &position)) {
@@ -975,7 +973,7 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
                               "too few frames in PixelData");
                 return false;
             }
-
+         
             if (tag != TAG_ITEM) {
                 dcm_error_set(error, DCM_ERROR_CODE_PARSE,
                               "building BasicOffsetTable failed",
@@ -984,21 +982,22 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
                               tag);
                 return false;
             }
-
+            
             // step back to the start of the item for this frame
-            offsets[i] = position - 8;
-
+            offsets[i] = position - *first_frame_offset - 8;
+            
             // and seek forward over the value
             if (!dcm_seekcur(&state, length, &position)) {
                 return false;
             }
         }
 
-        // the next thing should be the end of sequence tag
+        // in case multiple frames 1:1 frame to fragment mapping is assumed,
+        // therefore the next thing should be the end of sequence tag
         if (!read_tag(&state, &tag, &position)) {
             return false;
         }
-        if (tag != TAG_SQ_DELIM) {
+        if (num_frames != 1 && tag != TAG_SQ_DELIM) {
             dcm_error_set(error, DCM_ERROR_CODE_PARSE,
                           "reading BasicOffsetTable failed",
                           "too many frames in PixelData");
@@ -1009,6 +1008,7 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
     return true;
 }
 
+
 char *dcm_parse_frame(DcmError **error,
                       DcmIO *io,
                       bool implicit,
@@ -1022,34 +1022,95 @@ char *dcm_parse_frame(DcmError **error,
         .big_endian = is_big_endian(),
     };
 
+    *length =   desc->rows *
+                desc->columns *
+                desc->samples_per_pixel *
+                (desc->bits_allocated / 8);
+
+    char *value = DCM_MALLOC(error, *length);
+    if (value == NULL) {
+        return NULL;
+    }
     int64_t position = 0;
+    if (!dcm_require(&state, value, *length, &position)) {
+        free(value);
+        return NULL;
+    }
 
-    if (dcm_is_encapsulated_transfer_syntax(desc->transfer_syntax_uid)) {
-        uint32_t tag;
-        if (!read_tag(&state, &tag, &position) ||
-            !read_uint32(&state, length, &position)) {
+    return value;
+}
+
+/* Read encapsulated frame. Return NULL in case of error.
+*/
+char *dcm_parse_encapsulated_frame(DcmError **error,
+                                   DcmIO *io,
+                                   bool implicit,
+                                   int64_t frame_end_offset,
+                                   uint32_t* length)
+{
+    DcmParseState state = {
+        .error = error,
+        .io = io,
+        .implicit = implicit,
+        .big_endian = is_big_endian(),
+    };
+
+    int64_t position = 0;
+    *length = 0;
+    uint32_t tag;
+    uint32_t fragment_length = 0;
+    uint64_t frame_length = 0;
+
+    // first determine the total length of bytes to be read
+    while (position < frame_end_offset) {
+        if (!read_tag(&state, &tag, &position)) {
             return NULL;
         }
-
+        if (tag == TAG_SQ_DELIM) {
+            break;
+        }
         if (tag != TAG_ITEM) {
             dcm_error_set(error, DCM_ERROR_CODE_PARSE,
                           "reading frame item failed",
                           "no item tag found for frame item");
             return NULL;
         }
-    } else {
-        *length = desc->rows * desc->columns * desc->samples_per_pixel *
-            (desc->bits_allocated / 8);
+        if (!read_uint32(&state, &fragment_length, &position)) {
+            return NULL;
+        }
+        dcm_seekcur(&state, fragment_length, &position);
+        frame_length += fragment_length;
+    }
+    if (frame_length > 0xFFFFFFFF) {
+        dcm_error_set(error, DCM_ERROR_CODE_PARSE,
+                      "invalid frame size",
+                      "frame size exceeds 4GB" );
+        return NULL;
     }
 
-    char *value = DCM_MALLOC(error, *length);
+    char *value = DCM_MALLOC(error, frame_length);
     if (value == NULL) {
         return NULL;
     }
-    if (!dcm_require(&state, value, *length, &position)) {
-        free(value);
-        return NULL;
+    // if frame end is unknown/undefined then update it
+    if (frame_end_offset == 0xFFFFFFFF) {
+        frame_end_offset = position;
     }
-
+    // reposition to the beginning of encapsulated pixel data
+    dcm_seekcur(&state, -position, &position);
+
+    fragment_length = 0;
+    char* fragment = value;
+    position = 0;
+    while (position < frame_end_offset) {
+        read_tag(&state, &tag, &position);
+        read_uint32(&state, &fragment_length, &position);
+        if (!dcm_require(&state, fragment, fragment_length, &position)) {
+            free(value);
+            return NULL;
+        }
+        fragment += fragment_length;
+    }
+    *length = (uint32_t) frame_length;
     return value;
 }

data/test_files/generated_encapsulated_empty_bot_1_to_1.dcm

@@ -167,3 +167,9 @@ char *dcm_parse_frame(DcmError **error,
                       bool implicit,
                       struct PixelDescription *desc,
                       uint32_t *length);
+
+char *dcm_parse_encapsulated_frame(DcmError **error,
+                                   DcmIO *io,
+                                   bool implicit,
+                                   int64_t frame_end_offset,
+                                   uint32_t* length);