Skip to content

Commit

Permalink
Merge pull request #20 from exini/bugfix/item-indices-should-not-be-t…
Browse files Browse the repository at this point in the history
…racked-during-parsing

Removed item index tracking from parsing as this cannot be tracked re…
  • Loading branch information
karl-exini committed Feb 1, 2021
2 parents 62688ab + 507a559 commit b2e0abe
Show file tree
Hide file tree
Showing 15 changed files with 272 additions and 233 deletions.
44 changes: 20 additions & 24 deletions src/main/scala/com/exini/dicom/data/DicomElements.scala
Expand Up @@ -110,32 +110,32 @@ object DicomElements {
s"FragmentsElement(${tagToString(tag)} $vr # ${Lookup.keywordOf(tag).getOrElse("")})"
}

case class FragmentElement(index: Int, length: Long, value: Value, bigEndian: Boolean = false) extends Element {
case class FragmentElement(length: Long, value: Value, bigEndian: Boolean = false) extends Element {
override def toBytes: ByteString = toParts.map(_.bytes).reduce(_ ++ _)
override def toParts: List[DicomPart] =
if (value.length > 0)
ItemElement(index, value.length, bigEndian).toParts ::: ValueChunk(bigEndian, value.bytes, last = true) :: Nil
ItemElement(value.length, bigEndian).toParts ::: ValueChunk(bigEndian, value.bytes, last = true) :: Nil
else
ItemElement(index, value.length, bigEndian).toParts ::: Nil
override def toString: String = s"FragmentElement(index = $index, length = $length)"
ItemElement(value.length, bigEndian).toParts ::: Nil
override def toString: String = s"FragmentElement(length = $length)"
}

object FragmentElement {
def empty(index: Int, length: Long, bigEndian: Boolean = false): FragmentElement =
FragmentElement(index, length, Value.empty, bigEndian)
def empty(length: Long, bigEndian: Boolean = false): FragmentElement =
FragmentElement(length, Value.empty, bigEndian)
}

case class ItemElement(index: Int, length: Long, bigEndian: Boolean = false) extends Element {
case class ItemElement(length: Long, bigEndian: Boolean = false) extends Element {
def indeterminate: Boolean = length == indeterminateLength
override def toBytes: ByteString = tagToBytes(Tag.Item, bigEndian) ++ intToBytes(length.toInt, bigEndian)
override def toParts: List[DicomPart] = ItemPart(index, length, bigEndian, toBytes) :: Nil
override def toString: String = s"ItemElement(index = $index, length = $length)"
override def toParts: List[DicomPart] = ItemPart(length, bigEndian, toBytes) :: Nil
override def toString: String = s"ItemElement(length = $length)"
}

case class ItemDelimitationElement(index: Int, bigEndian: Boolean = false) extends Element {
case class ItemDelimitationElement(bigEndian: Boolean = false) extends Element {
override def toBytes: ByteString = tagToBytes(Tag.ItemDelimitationItem, bigEndian) ++ ByteString(0, 0, 0, 0)
override def toParts: List[DicomPart] = ItemDelimitationPart(index, bigEndian, toBytes) :: Nil
override def toString: String = s"ItemDelimitationElement(index = $index)"
override def toParts: List[DicomPart] = ItemDelimitationPart(bigEndian, toBytes) :: Nil
override def toString: String = s"ItemDelimitationElement"
}

case class SequenceDelimitationElement(bigEndian: Boolean = false) extends Element {
Expand Down Expand Up @@ -166,10 +166,7 @@ object DicomElements {
override def toBytes: ByteString = toElements.map(_.toBytes).reduce(_ ++ _)
override def toElements: List[Element] =
SequenceElement(tag, length, bigEndian, explicitVR) ::
items.zipWithIndex.flatMap {
case (item, index) => item.toElements(index + 1)
} :::
(if (indeterminate) SequenceDelimitationElement(bigEndian) :: Nil else Nil)
items.flatMap(_.toElements) ::: (if (indeterminate) SequenceDelimitationElement(bigEndian) :: Nil else Nil)
def size: Int = items.length
def setItem(index: Int, item: Item): Sequence = copy(items = items.updated(index - 1, item))
override def toString: String =
Expand Down Expand Up @@ -210,10 +207,10 @@ object DicomElements {

case class Item(elements: Elements, length: Long = indeterminateLength, bigEndian: Boolean = false) {
val indeterminate: Boolean = length == indeterminateLength
def toElements(index: Int): List[Element] =
ItemElement(index, length, bigEndian) :: elements.toElements(false) :::
(if (indeterminate) ItemDelimitationElement(index, bigEndian) :: Nil else Nil)
def toBytes: ByteString = toElements(1).map(_.toBytes).reduce(_ ++ _)
def toElements: List[Element] =
ItemElement(length, bigEndian) :: elements.toElements(false) :::
(if (indeterminate) ItemDelimitationElement(bigEndian) :: Nil else Nil)
def toBytes: ByteString = toElements.map(_.toBytes).reduce(_ ++ _)
def setElements(elements: Elements): Item = {
val newLength = if (this.indeterminate) indeterminateLength else elements.toBytes(withPreamble = false).length
copy(elements = elements, length = newLength)
Expand All @@ -230,8 +227,8 @@ object DicomElements {
}

case class Fragment(length: Long, value: Value, bigEndian: Boolean = false) {
def toElement(index: Int): FragmentElement = FragmentElement(index, length, value, bigEndian)
override def toString: String = s"Fragment(length = $length, value length = ${value.length})"
def toElement: FragmentElement = FragmentElement(length, value, bigEndian)
override def toString: String = s"Fragment(length = $length, value length = ${value.length})"
}

object Fragment {
Expand Down Expand Up @@ -313,7 +310,6 @@ object DicomElements {
offsets
.map(o =>
FragmentElement(
1,
4L * o.length,
Value(
o.map(offset => truncate(4, longToBytes(offset, bigEndian), bigEndian))
Expand All @@ -323,7 +319,7 @@ object DicomElements {
) :: Nil
)
.getOrElse(Nil) :::
fragments.zipWithIndex.map { case (fragment, index) => fragment.toElement(index + 2) } :::
fragments.zipWithIndex.map { case (fragment, index) => fragment.toElement } :::
SequenceDelimitationElement(bigEndian) :: Nil
def setFragment(index: Int, fragment: Fragment): Fragments =
copy(fragments = fragments.updated(index - 1, fragment))
Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/com/exini/dicom/data/DicomParts.scala
Expand Up @@ -120,12 +120,12 @@ object DicomParts {

case class DeflatedChunk(bigEndian: Boolean, bytes: ByteString, nowrap: Boolean) extends DicomPart

case class ItemPart(index: Int, length: Long, bigEndian: Boolean, bytes: ByteString) extends LengthPart {
case class ItemPart(length: Long, bigEndian: Boolean, bytes: ByteString) extends LengthPart {
override def toString =
s"${getClass.getSimpleName} index = $index length = $length ${if (bigEndian) "(big endian) " else ""}$bytes"
s"${getClass.getSimpleName} length = $length ${if (bigEndian) "(big endian) " else ""}$bytes"
}

case class ItemDelimitationPart(index: Int, bigEndian: Boolean, bytes: ByteString) extends DicomPart
case class ItemDelimitationPart(bigEndian: Boolean, bytes: ByteString) extends DicomPart

case class SequencePart(tag: Int, length: Long, bigEndian: Boolean, explicitVR: Boolean, bytes: ByteString)
extends DicomPart
Expand Down
10 changes: 5 additions & 5 deletions src/main/scala/com/exini/dicom/streams/CollectFlow.scala
Expand Up @@ -122,7 +122,7 @@ object CollectFlow {
part match {
case ValueChunkMarker =>
case SequenceDelimitationPartMarker =>
case _: ItemDelimitationPartMarker =>
case ItemDelimitationPartMarker =>
case _ =>
buffer = buffer :+ part
currentBufferSize = currentBufferSize + part.bytes.size
Expand All @@ -139,7 +139,7 @@ object CollectFlow {
Nil

case item: ItemPart if inFragments =>
currentFragment = Option(FragmentElement.empty(item.index, item.length, item.bigEndian))
currentFragment = Option(FragmentElement.empty(item.length, item.bigEndian))
bytes = ByteString.empty
Nil

Expand All @@ -162,12 +162,12 @@ object CollectFlow {
maybeAdd(FragmentsElement(fragments.tag, fragments.vr, fragments.bigEndian, fragments.explicitVR))
Nil
case item: ItemPart =>
maybeAdd(ItemElement(item.index, item.length, item.bigEndian))
maybeAdd(ItemElement(item.length, item.bigEndian))
Nil
case _: ItemDelimitationPartMarker =>
case ItemDelimitationPartMarker =>
Nil
case itemDelimitation: ItemDelimitationPart =>
maybeAdd(ItemDelimitationElement(itemDelimitation.index, itemDelimitation.bigEndian))
maybeAdd(ItemDelimitationElement(itemDelimitation.bigEndian))
Nil
case SequenceDelimitationPartMarker =>
Nil
Expand Down
15 changes: 10 additions & 5 deletions src/main/scala/com/exini/dicom/streams/DicomFlow.scala
Expand Up @@ -248,7 +248,7 @@ trait GuaranteedValueEvent[Out] extends DicomFlow[Out] with InFragments[Out] {

object SequenceDelimitationPartMarker extends SequenceDelimitationPart(bigEndian = false, ByteString.empty)

class ItemDelimitationPartMarker(item: Int) extends ItemDelimitationPart(item, bigEndian = false, ByteString.empty)
object ItemDelimitationPartMarker extends ItemDelimitationPart(bigEndian = false, ByteString.empty)

/**
* By mixing in this trait, sequences and items with determinate length will be concluded by delimitation events, just
Expand All @@ -269,7 +269,7 @@ trait GuaranteedDelimitationEvents[Out] extends DicomFlow[Out] with InFragments[
val (inactive, active) = partStack.span { case (p, b) => !p.indeterminate && b <= 0 }
partStack = active // only keep items and sequences with bytes left to subtract
inactive.flatMap { // call events, any items will be inserted in stream
case (item: ItemPart, _) => onItemDelimitation(new ItemDelimitationPartMarker(item.index))
case (item: ItemPart, _) => onItemDelimitation(ItemDelimitationPartMarker)
case _ => onSequenceDelimitation(SequenceDelimitationPartMarker)
}
}
Expand Down Expand Up @@ -299,11 +299,11 @@ trait GuaranteedDelimitationEvents[Out] extends DicomFlow[Out] with InFragments[
}

abstract override def onItemDelimitation(part: ItemDelimitationPart): List[Out] = {
if (partStack.nonEmpty && !part.isInstanceOf[ItemDelimitationPartMarker])
if (partStack.nonEmpty && part != ItemDelimitationPartMarker)
partStack = partStack.tail
subtractAndEmit(
part,
(p: ItemDelimitationPart) => super.onItemDelimitation(p).filterNot(_.isInstanceOf[ItemDelimitationPartMarker])
(p: ItemDelimitationPart) => super.onItemDelimitation(p).filterNot(_ == ItemDelimitationPartMarker)
)
}

Expand Down Expand Up @@ -356,7 +356,12 @@ trait TagPathTracking[Out]
}

abstract override def onItem(part: ItemPart): List[Out] = {
if (!inFragments) tagPath = tagPath.previous.thenItem(tagPath.tag, part.index)
if (!inFragments)
tagPath = tagPath match {
case t: TagPathItemEnd =>
t.previous.thenItem(t.tag, t.item + 1)
case t => t.previous.thenItem(t.tag, 1)
}
super.onItem(part)
}

Expand Down
29 changes: 10 additions & 19 deletions src/main/scala/com/exini/dicom/streams/DicomFlows.scala
Expand Up @@ -16,8 +16,6 @@

package com.exini.dicom.streams

import java.util.zip.Deflater

import akka.NotUsed
import akka.stream.Attributes
import akka.stream.scaladsl.{ Flow, Source }
Expand All @@ -31,6 +29,8 @@ import com.exini.dicom.data._
import com.exini.dicom.streams.CollectFlow._
import com.exini.dicom.streams.ModifyFlow._

import java.util.zip.Deflater

/**
* Various flows for transforming data of <code>DicomPart</code>s.
*/
Expand Down Expand Up @@ -363,7 +363,7 @@ object DicomFlows {
}

override def createLogic(inheritedAttributes: Attributes): GraphStageLogic = {
var discarding = false
var discarding = false
var isFragments = false // inFragments does not work for DeferToPartFlow in seq delim

new DeferToPartLogic with InSequenceLogic with GroupLengthWarningsLogic {
Expand Down Expand Up @@ -501,14 +501,11 @@ object DicomFlows {
}

override def onItemDelimitation(part: ItemDelimitationPart): List[DicomPart] =
super.onItemDelimitation(part) ::: (if (part.bytes.isEmpty)
ItemDelimitationPart(
part.index,
part.bigEndian,
itemDelimitation(part.bigEndian)
) :: Nil
else
Nil)
super.onItemDelimitation(part) :::
(if (part.bytes.isEmpty)
ItemDelimitationPart(part.bigEndian, itemDelimitation(part.bigEndian)) :: Nil
else
Nil)
}
})

Expand Down Expand Up @@ -653,16 +650,10 @@ object DicomFlows {
) :: Nil

case i: ItemPart =>
ItemPart(
i.index,
i.length,
bigEndian = false,
tagToBytesLE(Tag.Item) ++ i.bytes.takeRight(4).reverse
) :: Nil
ItemPart(i.length, bigEndian = false, tagToBytesLE(Tag.Item) ++ i.bytes.takeRight(4).reverse) :: Nil

case i: ItemDelimitationPart =>
case _: ItemDelimitationPart =>
ItemDelimitationPart(
i.index,
bigEndian = false,
tagToBytesLE(Tag.ItemDelimitationItem) ++ ByteString(0, 0, 0, 0)
) :: Nil
Expand Down
13 changes: 9 additions & 4 deletions src/main/scala/com/exini/dicom/streams/ElementFlows.scala
Expand Up @@ -53,7 +53,7 @@ object ElementFlows {
bytes = ByteString.empty
Nil
case item: ItemPart if inFragments =>
currentFragment = Option(FragmentElement.empty(item.index, item.length, item.bigEndian))
currentFragment = Option(FragmentElement.empty(item.length, item.bigEndian))
bytes = ByteString.empty
Nil

Expand All @@ -74,9 +74,9 @@ object ElementFlows {
case fragments: FragmentsPart =>
FragmentsElement(fragments.tag, fragments.vr, fragments.bigEndian, fragments.explicitVR) :: Nil
case item: ItemPart =>
ItemElement(item.index, item.length, item.bigEndian) :: Nil
ItemElement(item.length, item.bigEndian) :: Nil
case itemDelimitation: ItemDelimitationPart =>
ItemDelimitationElement(itemDelimitation.index, itemDelimitation.bigEndian) :: Nil
ItemDelimitationElement(itemDelimitation.bigEndian) :: Nil
case sequenceDelimitation: SequenceDelimitationPart =>
SequenceDelimitationElement(sequenceDelimitation.bigEndian) :: Nil

Expand Down Expand Up @@ -116,7 +116,12 @@ object ElementFlows {
inFragments = false
(tagPath, e) :: Nil
case e: ItemElement =>
if (!inFragments) tagPath = tagPath.previous.thenItem(tagPath.tag, e.index)
if (!inFragments)
tagPath = tagPath match {
case t: TagPathItemEnd =>
t.previous.thenItem(t.tag, t.item + 1)
case t => t.previous.thenItem(t.tag, 1)
}
(tagPath, e) :: Nil
case e: ItemDelimitationElement =>
tagPath = tagPath match {
Expand Down

0 comments on commit b2e0abe

Please sign in to comment.