Fix encapsulated pixeldata handling

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]
 

src/dicom-data.c

@@ -1813,13 +1813,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",

src/dicom-file.c

@@ -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;
     }

src/dicom-parse.c

@@ -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;
 }

src/pdicom.h

@@ -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);

tests/check_dicom.c

@@ -807,6 +807,121 @@ START_TEST(test_file_ct_brain_single)
 }
 END_TEST
 
+
+START_TEST(test_encapsulated_empty_BOT_1_to_1)
+{
+    char *file_path = fixture_path("data/test_files/generated_encapsulated_empty_bot_1_to_1.dcm");
+    DcmFilehandle *filehandle = 
+        dcm_filehandle_create_from_file(NULL, file_path);
+    free( file_path );
+    ck_assert_ptr_nonnull(filehandle);
+    DcmFrame* frame = dcm_filehandle_read_frame(NULL,
+                            filehandle,
+                            1);
+    ck_assert_ptr_nonnull( frame );
+    uint32_t frame_length = dcm_frame_get_length( frame );
+    ck_assert_uint_eq( frame_length, 8 );
+    const char* data = dcm_frame_get_value( frame );
+    const char expected_data[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7 };
+    ck_assert_mem_eq( expected_data, data, sizeof(expected_data) );
+}
+END_TEST
+
+
+START_TEST(test_encapsulated_empty_BOT_2_to_1)
+{
+    char *file_path = fixture_path("data/test_files/generated_encapsulated_empty_bot_2_to_1.dcm");
+    DcmFilehandle *filehandle = 
+        dcm_filehandle_create_from_file(NULL, file_path);
+    free( file_path );
+    ck_assert_ptr_nonnull(filehandle);
+    DcmFrame* frame = dcm_filehandle_read_frame(NULL,
+                            filehandle,
+                            1);
+    ck_assert_ptr_nonnull( frame );
+    uint32_t frame_length = dcm_frame_get_length( frame );
+    ck_assert_uint_eq( frame_length, 16 );
+    const char* data = dcm_frame_get_value( frame );
+    const char expected_data[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8,
+                    0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf };
+    ck_assert_mem_eq( expected_data, data, sizeof(expected_data) );
+}
+END_TEST
+
+
+START_TEST(test_encapsulated_defined_BOT_1_to_1)
+{
+    char *file_path = fixture_path("data/test_files/generated_encapsulated_defined_bot_1_to_1.dcm");
+    DcmFilehandle *filehandle = 
+        dcm_filehandle_create_from_file(NULL, file_path);
+    free( file_path );
+    ck_assert_ptr_nonnull(filehandle);
+    DcmFrame* frame = dcm_filehandle_read_frame(NULL,
+                            filehandle,
+                            1);
+    ck_assert_ptr_nonnull( frame );
+    uint32_t frame_length = dcm_frame_get_length( frame );
+    ck_assert_uint_eq( frame_length, 8 );
+    const char* data = dcm_frame_get_value( frame );
+    const char expected_data[] = 
+        { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7 };
+    ck_assert_mem_eq( expected_data, data, sizeof(expected_data) );
+}
+END_TEST
+
+
+START_TEST(test_encapsulated_defined_BOT_2_to_1)
+{
+    char *file_path = fixture_path("data/test_files/generated_encapsulated_defined_bot_2_to_1.dcm");
+    DcmFilehandle *filehandle = 
+        dcm_filehandle_create_from_file(NULL, file_path);
+    free( file_path );
+    ck_assert_ptr_nonnull(filehandle);
+    DcmFrame* frame = dcm_filehandle_read_frame(NULL,
+                            filehandle,
+                            1);
+    ck_assert_ptr_nonnull( frame );
+    uint32_t frame_length = dcm_frame_get_length( frame );
+    ck_assert_uint_eq( frame_length, 16 );
+    const char* data = dcm_frame_get_value( frame );
+    const char expected_data[] = 
+        { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf };
+    ck_assert_mem_eq( expected_data, data, sizeof(expected_data) );
+}
+END_TEST
+
+
+START_TEST(test_encapsulated_defined_BOT_2_to_2)
+{
+    char *file_path = fixture_path("data/test_files/generated_encapsulated_defined_bot_2_to_2.dcm");
+    DcmFilehandle *filehandle = 
+        dcm_filehandle_create_from_file(NULL, file_path);
+    free( file_path );
+    ck_assert_ptr_nonnull(filehandle);
+    DcmFrame* frame = dcm_filehandle_read_frame(NULL,
+                            filehandle,
+                            1);
+    ck_assert_ptr_nonnull( frame );
+    uint32_t frame_length = dcm_frame_get_length( frame );
+    ck_assert_uint_eq( frame_length, 8 );
+    const char* data1 = dcm_frame_get_value( frame );
+    const char expected_data1[] = 
+        { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7 };
+    ck_assert_mem_eq( expected_data1, data1, sizeof(expected_data1) );
+    frame = dcm_filehandle_read_frame(NULL,
+                            filehandle,
+                            2);
+    ck_assert_ptr_nonnull( frame );
+    frame_length = dcm_frame_get_length( frame );
+    ck_assert_uint_eq( frame_length, 8 );
+    const char* data2 = dcm_frame_get_value( frame );
+    const char expected_data2[] = 
+        { 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf };
+    ck_assert_mem_eq( expected_data2, data2, sizeof(expected_data2) );
+}
+END_TEST
+
+
 static Suite *create_main_suite(void)
 {
     Suite *suite = suite_create("main");
@@ -896,13 +1011,40 @@ static Suite *create_single_frame_suite(void)
     return suite;
 }
 
+static Suite *create_parse_suite(void)
+{
+    Suite *suite = suite_create("parse");
+
+    TCase *encapsulated_case1 = tcase_create("empty_BOT_1_to_1");
+    tcase_add_test(encapsulated_case1, test_encapsulated_empty_BOT_1_to_1);
+    suite_add_tcase(suite, encapsulated_case1);
+
+    TCase *encapsulated_case2 = tcase_create("empty_BOT_2_to_1");
+    tcase_add_test(encapsulated_case2, test_encapsulated_empty_BOT_2_to_1);
+    suite_add_tcase(suite, encapsulated_case2);
+
+    TCase *encapsulated_case3 = tcase_create("defined_BOT_1_to_1");
+    tcase_add_test(encapsulated_case3, test_encapsulated_defined_BOT_1_to_1);
+    suite_add_tcase(suite, encapsulated_case3);
+
+    TCase *encapsulated_case4 = tcase_create("defined_BOT_2_to_1");
+    tcase_add_test(encapsulated_case4, test_encapsulated_defined_BOT_2_to_1);
+    suite_add_tcase(suite, encapsulated_case4);
+
+    TCase *encapsulated_case5 = tcase_create("defined_BOT_2_to_2");
+    tcase_add_test(encapsulated_case5, test_encapsulated_defined_BOT_2_to_2);
+    suite_add_tcase(suite, encapsulated_case5);
+
+    return suite;
+}
 
 int main(void)
 {
     SRunner *runner = srunner_create(create_main_suite());
     srunner_add_suite(runner, create_data_suite());
     srunner_add_suite(runner, create_file_suite());
     srunner_add_suite(runner, create_single_frame_suite());
+    srunner_add_suite(runner, create_parse_suite());
     srunner_run_all(runner, CK_VERBOSE);
     int number_failed = srunner_ntests_failed(runner);
     srunner_free(runner);