Skip to content
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.

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

Lock during frame index parsing #178

Merged
merged 6 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Check for supported transfer syntax when DICOMWeb client returns an iterable of frames.
- Fix for order of transfer syntax to check.
- Use `ImageType` `DERIVED` instead of `ORIGINAL` when saving resampled volume instances.
- Thread-safe parsing of pixel data frame index.

## [0.21.4] - 2024-10-30

Expand Down
42 changes: 25 additions & 17 deletions tests/file/io/frame_index/test_frame_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@
JPEGBaseline8Bit,
)

from wsidicom.file.io.frame_index.bot import Bot
from wsidicom.file.io.frame_index.empty_bot import EmptyBot
from wsidicom.file.io.frame_index.eot import Eot
from wsidicom.file.io.frame_index.native_pixel_data_frame import NativePixelData
from wsidicom.file.io.frame_index.basic import BasicOffsetTableFrameIndexParser
from wsidicom.file.io.frame_index.extended import ExtendedOffsetFrameIndexParser
from wsidicom.file.io.frame_index.native_pixel_data import (
NativePixelDataFrameIndexParser,
)
from wsidicom.file.io.frame_index.pixel_data import PixelDataFrameIndexParser
from wsidicom.file.io.wsidicom_io import WsiDicomIO
from wsidicom.geometry import Size
from wsidicom.tags import (
Expand Down Expand Up @@ -60,13 +62,13 @@ def tiles(bits: int):
]


class TestFrameIndex:
class TestFrameIndexParser:
@pytest.mark.parametrize(
"transfer_syntax",
[ImplicitVRLittleEndian, ExplicitVRLittleEndian, ExplicitVRBigEndian],
)
@pytest.mark.parametrize("bits", [8, 16])
def test_read_native_pixel_data(
def test_read_native_pixel_data_offset_table_frame_positions(
self, buffer: WsiDicomIO, tiles: List[bytes], transfer_syntax: UID, bits: int
):
# Arrange
Expand All @@ -79,13 +81,16 @@ def test_read_native_pixel_data(
buffer.write(tile)

# Act
frame_index = NativePixelData(buffer, 0, len(tiles), Size(1, 1), 1, bits)
parser = NativePixelDataFrameIndexParser(
buffer, 0, len(tiles), Size(1, 1), 1, bits
)
frame_index = parser.parse_frame_index()

# Assert
assert frame_index.index == expected_frame_index
assert frame_index == expected_frame_index

@pytest.mark.parametrize("bits", [8, 16])
def test_empty_bot(self, buffer: WsiDicomIO, tiles: List[bytes]):
def test_pixel_data_offset_table(self, buffer: WsiDicomIO, tiles: List[bytes]):
# Arrange
EMPTY_BOT = 16
ITEM_TAG_AND_LENGTH = 8
Expand All @@ -104,13 +109,14 @@ def test_empty_bot(self, buffer: WsiDicomIO, tiles: List[bytes]):
]

# Act
frame_index = EmptyBot(buffer, 0, len(tiles))
parser = PixelDataFrameIndexParser(buffer, 0, len(tiles))
frame_index = parser.parse_frame_index()

# Assert
assert frame_index.index == expected_frame_index
assert frame_index == expected_frame_index

@pytest.mark.parametrize("bits", [8, 16])
def test_bot(self, buffer: WsiDicomIO, tiles: List[bytes]):
def test_basic_offset_table(self, buffer: WsiDicomIO, tiles: List[bytes]):
# Arrange
BOT = 16 + len(tiles) * 4
ITEM_TAG_AND_LENGTH = 8
Expand All @@ -128,13 +134,14 @@ def test_bot(self, buffer: WsiDicomIO, tiles: List[bytes]):
]

# Act
frame_index = Bot(buffer, 0, len(tiles))
parser = BasicOffsetTableFrameIndexParser(buffer, 0, len(tiles))
frame_index = parser.parse_frame_index()

# Assert
assert frame_index.index == expected_frame_index
assert frame_index == expected_frame_index

@pytest.mark.parametrize("bits", [8, 16])
def test_eot(self, buffer: WsiDicomIO, tiles: List[bytes]):
def test_extended_offset_table(self, buffer: WsiDicomIO, tiles: List[bytes]):
# Arrange
EMPTY_BOT = 16
ITEM_TAG_AND_LENGTH = 8
Expand Down Expand Up @@ -169,7 +176,8 @@ def ensure_tile_is_even_length(tile: bytes) -> bytes:
]

# Act
frame_index = Eot(buffer, 0, len(tiles))
parser = ExtendedOffsetFrameIndexParser(buffer, 0, len(tiles))
frame_index = parser.parse_frame_index()

# Assert
assert frame_index.index == expected_frame_index
assert frame_index == expected_frame_index
8 changes: 4 additions & 4 deletions tests/file/io/frame_index/test_offset_table_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
)

from wsidicom.file.io.frame_index import BotWriter, EotWriter
from wsidicom.file.io.frame_index.bot import Bot
from wsidicom.file.io.frame_index.eot import Eot
from wsidicom.file.io.frame_index.basic import BasicOffsetTableFrameIndexParser
from wsidicom.file.io.frame_index.extended import ExtendedOffsetFrameIndexParser
from wsidicom.file.io.wsidicom_io import WsiDicomIO
from wsidicom.tags import (
ExtendedOffsetTableLengthsTag,
Expand Down Expand Up @@ -113,7 +113,7 @@ def test_write_bot(self, buffer: WsiDicomIO, positions: Sequence[int]):

# Assert
buffer.seek(0)
Bot(buffer, 0, len(positions))
BasicOffsetTableFrameIndexParser(buffer, 0, len(positions))

def test_write_eot(self, buffer: WsiDicomIO, positions: Sequence[int]):
# Arrange
Expand All @@ -129,7 +129,7 @@ def test_write_eot(self, buffer: WsiDicomIO, positions: Sequence[int]):

# Assert
buffer.seek(0)
Eot(buffer, 0, len(positions))
ExtendedOffsetFrameIndexParser(buffer, 0, len(positions))

@staticmethod
def assertEndOfFile(file: WsiDicomIO):
Expand Down
12 changes: 8 additions & 4 deletions tests/file/io/test_wsidicom_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@

import math
import os
import threading
from pathlib import Path
from typing import List, Optional, OrderedDict, Sequence
from typing import List, Optional, OrderedDict, Sequence, Tuple

import pytest
from PIL.Image import Image
Expand All @@ -37,6 +38,7 @@
WsiDicomReader,
WsiDicomWriter,
)
from wsidicom.file.io.frame_index.parser import FrameIndexParser
from wsidicom.file.io.wsidicom_writer import (
WsiDicomEncapsulatedWriter,
WsiDicomNativeWriter,
Expand Down Expand Up @@ -75,8 +77,10 @@ def __init__(
dataset.SamplesPerPixel = samples_per_pixel
dataset.NumberOfFrames = frame_count
dataset.ImageType = ["ORIGINAL", "PRIMARY", "VOLUME", "NONE"]

self._dataset = WsiDataset(dataset)
self._frame_index_parser: Optional[FrameIndexParser] = None
self._frame_index: Optional[List[Tuple[int, int]]] = None
self._lock = threading.Lock()

@property
def frame_count(self) -> int:
Expand Down Expand Up @@ -250,7 +254,7 @@ def test_write_encapsulated_pixel_data(
filepath, transfer_syntax, frame_count, bits, tile_size, samples_per_pixel
) as reader:
read_table_type = reader.offset_table_type
read_frame_positions = reader.frame_index.index
read_frame_positions = reader.frame_index
TAG_BYTES = 4
LENGTH_BYTES = 4
frame_offsets = [
Expand Down Expand Up @@ -305,7 +309,7 @@ def test_write_unencapsulated_pixel_data(
filepath, transfer_syntax, frame_count, bits, tile_size, samples_per_pixel
) as read_file:
read_table_type = read_file.offset_table_type
read_frame_positions = read_file.frame_index.index
read_frame_positions = read_file.frame_index
assert read_table_type == OffsetTableType.NONE
if transfer_syntax == ImplicitVRLittleEndian:
offset = 8
Expand Down
27 changes: 16 additions & 11 deletions wsidicom/file/io/frame_index/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,32 @@
# limitations under the License.


from wsidicom.file.io.frame_index.bot import Bot, EmptyBotException
from wsidicom.file.io.frame_index.empty_bot import EmptyBot
from wsidicom.file.io.frame_index.eot import Eot
from wsidicom.file.io.frame_index.frame_index import FrameIndex
from wsidicom.file.io.frame_index.native_pixel_data_frame import NativePixelData
from wsidicom.file.io.frame_index.basic import (
BasicOffsetTableFrameIndexParser,
EmptyBasicTableOffsetException,
)
from wsidicom.file.io.frame_index.extended import ExtendedOffsetFrameIndexParser
from wsidicom.file.io.frame_index.native_pixel_data import (
NativePixelDataFrameIndexParser,
)
from wsidicom.file.io.frame_index.offset_table_type import OffsetTableType
from wsidicom.file.io.frame_index.offset_table_writer import (
BotWriter,
EotWriter,
OffsetTableWriter,
)
from wsidicom.file.io.frame_index.parser import FrameIndexParser
from wsidicom.file.io.frame_index.pixel_data import PixelDataFrameIndexParser

__all__ = [
"Bot",
"EmptyBot",
"Eot",
"FrameIndex",
"NativePixelData",
"BasicOffsetTableFrameIndexParser",
"PixelDataFrameIndexParser",
"ExtendedOffsetFrameIndexParser",
"FrameIndexParser",
"NativePixelDataFrameIndexParser",
"OffsetTableType",
"BotWriter",
"EotWriter",
"OffsetTableWriter",
"EmptyBotException",
"EmptyBasicTableOffsetException",
]
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@

from typing import List, Optional, Tuple

from wsidicom.file.io.frame_index.offset_table import OffsetTable
from wsidicom.file.io.frame_index.offset_table import OffsetTableFrameIndexParser
from wsidicom.file.io.frame_index.offset_table_type import OffsetTableType


class EmptyBotException(Exception):
class EmptyBasicTableOffsetException(Exception):
"""Exception raised when BOT was empty."""

pass


class Bot(OffsetTable):
class BasicOffsetTableFrameIndexParser(OffsetTableFrameIndexParser):
@property
def offset_table_type(self) -> OffsetTableType:
return OffsetTableType.BASIC
Expand All @@ -51,7 +51,7 @@ def _get_pixels_start(self) -> int:
self._validate_pixel_data_start()
bot_length = self._read_bot_length()
if bot_length is None:
raise EmptyBotException()
raise EmptyBasicTableOffsetException()
return self._file.tell()

def _read_table(self) -> Optional[bytes]:
Expand All @@ -64,5 +64,5 @@ def _read_table(self) -> Optional[bytes]:
"""
bot_length = self._read_bot_length()
if bot_length is None:
raise EmptyBotException()
raise EmptyBasicTableOffsetException()
return self._file.read(bot_length, need_exact_length=True)
4 changes: 2 additions & 2 deletions wsidicom/file/io/frame_index/encapsulated_pixel_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
from pydicom.tag import ItemTag

from wsidicom.errors import WsiDicomFileError
from wsidicom.file.io.frame_index.frame_index import FrameIndex
from wsidicom.file.io.frame_index.parser import FrameIndexParser


class EncapsulatedPixelData(FrameIndex):
class EncapsulatedPixelDataFrameIndexParser(FrameIndexParser):
def _validate_pixel_data_start(self):
"""Check that pixel data tag is present and that the tag length is
set as undefined. Raises WsiDicomFileError otherwise.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
from pydicom.tag import Tag

from wsidicom.errors import WsiDicomFileError
from wsidicom.file.io.frame_index.offset_table import OffsetTable
from wsidicom.file.io.frame_index.offset_table import OffsetTableFrameIndexParser
from wsidicom.file.io.frame_index.offset_table_type import OffsetTableType
from wsidicom.tags import ExtendedOffsetTableLengthsTag, ExtendedOffsetTableTag


class Eot(OffsetTable):
class ExtendedOffsetFrameIndexParser(OffsetTableFrameIndexParser):
@property
def offset_table_type(self) -> OffsetTableType:
return OffsetTableType.EXTENDED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
import math
from typing import List, Tuple

from wsidicom.file.io.frame_index.frame_index import FrameIndex
from wsidicom.file.io.frame_index.offset_table_type import OffsetTableType
from wsidicom.file.io.frame_index.parser import FrameIndexParser
from wsidicom.file.io.wsidicom_io import WsiDicomIO
from wsidicom.geometry import Size


class NativePixelData(FrameIndex):
class NativePixelDataFrameIndexParser(FrameIndexParser):
def __init__(
self,
file: WsiDicomIO,
Expand Down
6 changes: 4 additions & 2 deletions wsidicom/file/io/frame_index/offset_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
from pydicom.tag import ItemTag

from wsidicom.errors import WsiDicomFileError
from wsidicom.file.io.frame_index.encapsulated_pixel_data import EncapsulatedPixelData
from wsidicom.file.io.frame_index.encapsulated_pixel_data import (
EncapsulatedPixelDataFrameIndexParser,
)


class OffsetTable(EncapsulatedPixelData):
class OffsetTableFrameIndexParser(EncapsulatedPixelDataFrameIndexParser):
@property
@abstractmethod
def bytes_per_item(self) -> int:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
"""Index for frame positions and length in image data."""

from abc import abstractmethod
from functools import cached_property
from typing import List, Optional, Tuple

from wsidicom.errors import WsiDicomFileError
Expand All @@ -24,17 +23,15 @@
from wsidicom.tags import PixelDataTag


class FrameIndex:
class FrameIndexParser:
def __init__(self, file: WsiDicomIO, pixel_data_start: int, frame_count: int):
self._file = file
self._frame_count = frame_count
self._pixel_data_start = pixel_data_start
self._file.seek(self._pixel_data_start)
self._pixels_start = self._get_pixels_start()

@cached_property
def index(self) -> List[Tuple[int, int]]:
"""Return a list of frame positions and lengths."""
def parse_frame_index(self) -> List[Tuple[int, int]]:
self._file.seek(self._pixel_data_start)
index = self._get_index()
self._validate_frame_index(index)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
from pydicom.tag import ItemTag

from wsidicom.errors import WsiDicomFileError
from wsidicom.file.io.frame_index.bot import EmptyBotException
from wsidicom.file.io.frame_index.encapsulated_pixel_data import EncapsulatedPixelData
from wsidicom.file.io.frame_index.basic import EmptyBasicTableOffsetException
from wsidicom.file.io.frame_index.encapsulated_pixel_data import (
EncapsulatedPixelDataFrameIndexParser,
)
from wsidicom.file.io.frame_index.offset_table_type import OffsetTableType


class EmptyBot(EncapsulatedPixelData):
class PixelDataFrameIndexParser(EncapsulatedPixelDataFrameIndexParser):
"""Frame index parsed from reading the sequence of pixel data delimeters."""

@property
Expand Down Expand Up @@ -68,5 +70,5 @@ def _get_pixels_start(self) -> int:
self._validate_pixel_data_start()
bot_length = self._read_bot_length()
if bot_length is not None:
raise EmptyBotException()
raise EmptyBasicTableOffsetException()
return self._file.tell()
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
from PIL import Image as Pillow
from PIL import UnidentifiedImageError

from wsidicom.file.io.frame_index.empty_bot import EmptyBot
from wsidicom.file.io.frame_index.offset_table_type import OffsetTableType
from wsidicom.file.io.frame_index.pixel_data import PixelDataFrameIndexParser
from wsidicom.file.io.wsidicom_io import WsiDicomIO


Expand All @@ -20,7 +20,7 @@ class TiffTags(Enum):
TILEBYTECOUNTS = 325


class TiffTable(EmptyBot):
class TiffFrameIndexParser(PixelDataFrameIndexParser):
"""Frame index for TIFF, parsing the index from `TileOffsets`and TileByteCounts`
if present. Only works with `DICOM-TIFF dual files."""

Expand Down
Loading
Loading