Reduce seeks on file io (#114)

* Reduce seeks on file io

* add changelog entry

Author: rvause

CHANGELOG.md

@@ -5,6 +5,7 @@
 * support single-frame DICOM images and allow BitsStored > 8 [tokyovigilante]
 * fix error handling for string values over max length [arngaillard]
 * add `-w` (show warnings) to dcm-dump and dcm-getframe
+* reduce seeks on file io [rvause]
 
 ## 1.2.0, 09/04/2025
 
@@ -55,4 +56,3 @@
 ## 1.0.0, 2/10/23
 
 * first release!
-

src/dicom-file.c

@@ -1370,7 +1370,8 @@ DcmFrame *dcm_filehandle_read_frame(DcmError **error,
     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;
+                                   (filehandle->offset_table[i + 1] -
+                                    filehandle->offset_table[i]) : 0xFFFFFFFF;
         frame_data = dcm_parse_encapsulated_frame(error,
                                                   filehandle->io,
                                                   filehandle->implicit,

src/dicom-io.c

@@ -46,6 +46,7 @@ typedef struct _DcmIOFile {
     char input_buffer[BUFFER_SIZE];
     int64_t bytes_in_buffer;
     int64_t read_point;
+    int64_t offset;
 } DcmIOFile;
 
 
@@ -154,6 +155,7 @@ static int64_t refill(DcmError **error, DcmIOFile *file)
 
     file->read_point = 0;
     file->bytes_in_buffer = bytes_read;
+    file->offset += bytes_read;
 
     return bytes_read;
 }
@@ -202,42 +204,62 @@ static int64_t dcm_io_seek_file(DcmError **error, DcmIO *io,
 {
     DcmIOFile *file = (DcmIOFile *) io;
 
-    /* We've read ahead by some number of buffered bytes, so first undo that,
-     * then do the seek from the true position.
+    /* Translate the request to an absolute target. The logical position the
+     * caller perceives is offset - bytes_in_buffer + read_point.
      */
-    int64_t new_offset;
+    int64_t logical_pos = file->offset - file->bytes_in_buffer +
+                          file->read_point;
+    int64_t target;
 
-    int64_t bytes_ahead = file->bytes_in_buffer - file->read_point;
-    if (bytes_ahead > 0) {
-#ifdef _WIN32
-        new_offset = _lseeki64(file->fd, -bytes_ahead, SEEK_CUR);
-#else
-        new_offset = lseek(file->fd, -bytes_ahead, SEEK_CUR);
-#endif
+    switch (whence) {
+        case SEEK_SET:
+            target = offset;
+            break;
+        case SEEK_CUR:
+            target = logical_pos + offset;
+            break;
+        case SEEK_END:
+        default:
+            target = -1;
+            break;
+    }
 
-        if (new_offset < 0) {
-            dcm_error_set(error, DCM_ERROR_CODE_IO,
-                "unable to seek file",
-                "unable to seek %s - %s", file->filename, strerror(errno));
+    /* If we know the absolute target and it falls within the
+     * currently-buffered window (offset - bytes_in_buffer, offset), we can
+     * skip the seek entirely.
+     */
+    if (target >= 0) {
+        int64_t buffer_base = file->offset - file->bytes_in_buffer;
+        if (target >= buffer_base && target <= file->offset) {
+            file->read_point = target - buffer_base;
+            return target;
         }
     }
 
+    /* For SEEK_SET / SEEK_CUR we always use SEEK_SET with the resulting
+     * absolute target.
+     */
+    int64_t new_offset;
+    int seek_whence = (target >= 0) ? SEEK_SET : whence;
+    int64_t seek_offset = (target >= 0) ? target : offset;
 #ifdef _WIN32
-    new_offset = _lseeki64(file->fd, offset, whence);
+    new_offset = _lseeki64(file->fd, seek_offset, seek_whence);
 #else
-    new_offset = lseek(file->fd, offset, whence);
+    new_offset = lseek(file->fd, seek_offset, seek_whence);
 #endif
 
     if (new_offset < 0) {
         dcm_error_set(error, DCM_ERROR_CODE_IO,
             "unable to seek file",
             "unable to seek %s - %s", file->filename, strerror(errno));
+        return new_offset;
     }
 
-    /* Empty the buffer, since we may now be at a different position.
+    /* Empty the buffer; the next read will refill it from the new position.
      */
     file->bytes_in_buffer = 0;
     file->read_point = 0;
+    file->offset = new_offset;
 
     return new_offset;
 }
@@ -399,4 +421,3 @@ int64_t dcm_io_seek(DcmError **error,
 {
     return io->methods->seek(error, io, offset, whence);
 }
-

src/dicom-parse.c

@@ -1060,10 +1060,11 @@ char *dcm_parse_encapsulated_frame(DcmError **error,
     uint32_t tag;
     uint32_t fragment_length = 0;
     uint64_t frame_length = 0;
+    char *value = NULL;
 
-    // first determine the total length of bytes to be read
     while (position < frame_end_offset) {
         if (!read_tag(&state, &tag, &position)) {
+            free(value);
             return NULL;
         }
         if (tag == TAG_SQ_DELIM) {
@@ -1073,49 +1074,37 @@ char *dcm_parse_encapsulated_frame(DcmError **error,
             dcm_error_set(error, DCM_ERROR_CODE_PARSE,
                           "reading frame item failed",
                           "no item tag found for frame item");
+            free(value);
             return NULL;
         }
         if (!read_uint32(&state, &fragment_length, &position)) {
+            free(value);
             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, frame_length);
-    if (value == NULL) {
-        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) {
-        if (!read_tag(&state, &tag, &position)) {
+        if (frame_length + fragment_length > 0xFFFFFFFF) {
+            dcm_error_set(error, DCM_ERROR_CODE_PARSE,
+                          "invalid frame size",
+                          "frame size exceeds 4GB");
+            free(value);
             return NULL;
         }
-        if (tag == TAG_SQ_DELIM) {
-            break;
+
+        char *new_value = (char *) dcm_realloc(error, value,
+                                               frame_length + fragment_length);
+        if (new_value == NULL) {
+            free(value);
+            return NULL;
         }
-        if (!read_uint32(&state, &fragment_length, &position) ||
-            !dcm_require(&state, fragment, fragment_length, &position)) {
+        value = new_value;
+
+        if (!dcm_require(&state, value + frame_length, fragment_length,
+                         &position)) {
             free(value);
             return NULL;
         }
-        fragment += fragment_length;
+        frame_length += fragment_length;
     }
+
     *length = (uint32_t) frame_length;
 
     return value;