Skip to content

Fix encapsulated pixeldata handling #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix encapsulated pixeldata handling #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
87c0436
remove restriction for bits stored, because its value can be modality…
Jun 26, 2025
835ed96
fix offset reading and frame reading for encapsulated pixel data
Jun 26, 2025
d190231
fix offset calculation for encapsualted fragments: length was essenti…
Jun 30, 2025
0ceb9cc
split regular and encapsulated frame parsing
Jul 1, 2025
c02f192
fix offset calculation for non-encapsulated frames: bits allocated wa…
Jul 3, 2025
7a08c78
Merge branch 'main' into main
weanti Sep 30, 2025
ab8b9f0
coding style, code documentation, review fixes: simplification, error…
Oct 2, 2025
ac71ff4
functional tests for encapsulated pixel data handling
weanti Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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]

Expand Down
Binary file added data/test_files/generated_encapsulated_defined_bot_1_to_1.dcm
View file
Open in desktop
Binary file not shown.
Binary file added data/test_files/generated_encapsulated_defined_bot_2_to_1.dcm
View file
Open in desktop
Binary file not shown.
Binary file added data/test_files/generated_encapsulated_defined_bot_2_to_2.dcm
View file
Open in desktop
Binary file not shown.
Binary file added data/test_files/generated_encapsulated_empty_bot_1_to_1.dcm
View file
Open in desktop
Binary file not shown.
Binary file added data/test_files/generated_encapsulated_empty_bot_2_to_1.dcm
View file
Open in desktop
Binary file not shown.
7 changes: 0 additions & 7 deletions src/dicom-data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
25 changes: 19 additions & 6 deletions src/dicom-file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
157 changes: 111 additions & 46 deletions src/dicom-parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,61 +954,75 @@ bool dcm_parse_pixeldata_offsets(DcmError **error,
// the BOT is missing, we must scan pixeldata to find the position of
// each frame

// we could use our generic parser above ^^ but we have a special loop
// here as an optimisation (we can skip over the pixel data itself)

dcm_log_info("building Offset Table from Pixel Data");

// 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)) {
return false;
// each frame may consist of several fragments, so we need to scan each fragment to find the next frame
if (!read_tag(&state, &tag, &position)) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
"building BasicOffsetTable failed",
"failed to read tag");
return false;
}
// all fragments belong to the only frame
if ( num_frames == 1 ) {
// according to the standard the first frame's offset is 0
offsets[0] = 0;
} else {
// 1 fragment shall contain 1 frame
int fragment_idx = 0;
offsets[fragment_idx] = 0; // by definition the first fragment is at offset 0
fragment_idx++;
while( tag != TAG_SQ_DELIM ) {
if ( tag != TAG_ITEM || !read_uint32(&state, &length, &position)) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
"building BasicOffsetTable failed",
"failed to read fragment");
return false;
}
// skip actual content to find next offset
dcm_seekcur(&state, length, &position);
if (!read_tag(&state, &tag, &position)) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
"building BasicOffsetTable failed",
"failed to read tag");
return false;
}
if ( fragment_idx < num_frames ) {
offsets[fragment_idx] = offsets[fragment_idx-1] + 8/*tag and length field size*/ + length;
Copy link
Collaborator

@jcupitt jcupitt Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. I think you can have many fragments in each frame, so don't you need two loops here?

for (i = 0; i < num_frames; i++) {
    record position as start of frame i 
    while (frame has fragments)
        skip fragment
}

Copy link
Author

@weanti weanti Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is a kind of an assumption. It may be according to the standard, but the standard is not an easy reading.
This part covers the case where there is no Basic Offset Table (so the frame offsets are absent). In this case we need to find the frame offsets ourselves.
Here is the assumption: if there is no BOT then I assume that each frame consists of only 1 fragment.
Why do I assume this: If each frame could have arbitrary number of fragments then it would be impossible to find out which fagment belongs to which frame. This means that each frame shall have the same amount of fragments.
We could use a different assumption here: number of fragments = C * number of frames, where C >= 1 and C is an integer. This would complicate things a bit. I think encapsulation is used mainly for compressed Transfer Syntaxes and there is no guarantee that each frame can be compressed in a way that they span across exactly C fragment.
Feel free to ask or propose a different approach to assign fragments to frames.

}
fragment_idx++;
}

if (tag == TAG_SQ_DELIM) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
"reading BasicOffsetTable failed",
"too few frames in PixelData");
// fragment_idx shall equal to num_frames+1 at the end
fragment_idx--;
if ( fragment_idx > num_frames ) {
dcm_error_set(error, DCM_ERROR_CODE_INVALID,
"building BasicOffsetTable failed",
"Too many fragments" );
return false;
}

if (tag != TAG_ITEM) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
if ( num_frames < fragment_idx ) {
dcm_error_set(error, DCM_ERROR_CODE_INVALID,
"building BasicOffsetTable failed",
"frame Item #%d has wrong tag '%08x'",
i + 1,
tag);
"Too many frames" );
return false;
}

// step back to the start of the item for this frame
offsets[i] = position - 8;

// and seek forward over the value
if (!dcm_seekcur(&state, length, &position)) {
if ( !read_uint32(&state, &length, &position ) || length != 0 ) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
"building BasicOffsetTable failed",
"Sequence Delimiter Tag failure" );
return false;
}
}

// the next thing should be the end of sequence tag
if (!read_tag(&state, &tag, &position)) {
return false;
}
if (tag != TAG_SQ_DELIM) {
dcm_error_set(error, DCM_ERROR_CODE_PARSE,
"reading BasicOffsetTable failed",
"too many frames in PixelData");
return false;
}
}

return true;
}


char *dcm_parse_frame(DcmError **error,
DcmIO *io,
bool implicit,
Expand All @@ -1022,33 +1036,84 @@ 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;

// 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);
*length += fragment_length;
}

char *value = DCM_MALLOC(error, *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;
}

return value;
Expand Down
6 changes: 6 additions & 0 deletions src/pdicom.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Loading