Lock during frame index parsing (#178)

* Lock stream during frame index parsing

* Update changelog

CHANGELOG.md

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

tests/file/io/frame_index/test_frame_index.py

@@ -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 (
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

tests/file/io/frame_index/test_offset_table_writer.py

@@ -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,
@@ -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
@@ -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):

tests/file/io/test_wsidicom_writer.py

@@ -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
@@ -37,6 +38,7 @@
     WsiDicomReader,
     WsiDicomWriter,
 )
+from wsidicom.file.io.frame_index.parser import FrameIndexParser
 from wsidicom.file.io.wsidicom_writer import (
     WsiDicomEncapsulatedWriter,
     WsiDicomNativeWriter,
@@ -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:
@@ -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 = [
@@ -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

wsidicom/file/io/frame_index/__init__.py

@@ -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",
 ]

wsidicom/file/io/frame_index/bot.py → wsidicom/file/io/frame_index/basic.py

@@ -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
@@ -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]:
@@ -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)

wsidicom/file/io/frame_index/encapsulated_pixel_data.py

@@ -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.

wsidicom/file/io/frame_index/eot.py → wsidicom/file/io/frame_index/extended.py

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

wsidicom/file/io/frame_index/native_pixel_data_frame.py → wsidicom/file/io/frame_index/native_pixel_data.py

@@ -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,

wsidicom/file/io/frame_index/offset_table.py

@@ -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:

wsidicom/file/io/frame_index/frame_index.py → wsidicom/file/io/frame_index/parser.py

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

wsidicom/file/io/frame_index/empty_bot.py → wsidicom/file/io/frame_index/pixel_data.py

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

wsidicom/file/io/frame_index/tiff_table.py → wsidicom/file/io/frame_index/tiff.py

@@ -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
 
 
@@ -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."""