Non-sequential sectors fix (#1175)

* Accounts for inclusive ends Similar issue to trying to use arr[arr.size] to index the last element in an array, the actual last element is at arr[arr.size - 1]. So start = scratchAudio.totalFrames is correct if I want to access one past the last sector of audio. * Corrects the end value of a sector. Before, we would have verseAudio.totalFrames = 2. The sectors for this should be [0 .. 3], since 2 frames 4 bytes, however, it was [0 .. 4]. Adding 1 after converting the frames to bytes corrects this issue, and removes "dead" space between sectors. * Ensures no dead space on new verse and verse edit actions * Updated chapter edit and chapter import fix Based on meeting with Joe. Uses until (-1) instead of (+1), because using (+1) does not correctly account for varying frame sizes * Fixed incorrect start for record again action * Updated return value in finalizeVerse, since we are not finalizing sectors with until * Updated tests
Bible-Translation-Tools · Aug 6, 2024 · 4c3e493 · 4c3e493
1 parent b9d7e09
commit 4c3e493
Show file tree

Hide file tree

Showing 7 changed files with 162 additions and 153 deletions.
diff --git a/...main/kotlin/org/wycliffeassociates/otter/common/domain/narration/ChapterRepresentation.kt b/...main/kotlin/org/wycliffeassociates/otter/common/domain/narration/ChapterRepresentation.kt
@@ -155,11 +155,11 @@ internal class ChapterRepresentation(
     }
 
     fun finalizeVerse(verseIndex: Int, history: NarrationHistory? = null): Int {
-        val endIndex = frameToIndex(scratchAudio.totalFrames)
+        val endIndex = frameToIndex(scratchAudio.totalFrames) - 1
 
         history?.finalizeVerse(endIndex, totalVerses)
         onVersesUpdated()
-        return endIndex - 1 // subtract 1 due to the sector being finalized with `until`
+        return endIndex
     }
 
     fun onVersesUpdated() {
@@ -346,7 +346,7 @@ internal class ChapterRepresentation(
         verses
             .find { it.marker.label == verse.label }
             ?.let { _verse ->
-                return indexToFrame(_verse.firstIndex()) .. indexToFrame(_verse.lastIndex())
+                return indexToFrame(_verse.firstIndex())..indexToFrame(_verse.lastIndex())
             }
         return null
     }

diff --git a/common/src/main/kotlin/org/wycliffeassociates/otter/common/domain/narration/Narration.kt b/common/src/main/kotlin/org/wycliffeassociates/otter/common/domain/narration/Narration.kt
@@ -310,9 +310,10 @@ class Narration @AssistedInject constructor(
 
         return Completable.fromAction {
             val scratchAudio = chapterRepresentation.scratchAudio
-            val start = if (scratchAudio.totalFrames == 0) 0 else scratchAudio.totalFrames + 1
+            val start = if (scratchAudio.totalFrames == 0) 0 else scratchAudio.totalFrames
             audioFileUtils.appendFile(chapterRepresentation.scratchAudio, editedFile)
-            val end = chapterRepresentation.scratchAudio.totalFrames
+            val verseAudio = AudioFile(editedFile)
+            val end = max(start + verseAudio.totalFrames, 0)
 
             /* When a new verse recorded with an EXTERNAL plugin comes back empty,
             {start} could be greater than {end} by 1, which is invalid and may cause a crash.
@@ -538,22 +539,21 @@ class Narration @AssistedInject constructor(
             }
 
         val scratchAudio = chapterRepresentation.scratchAudio
-        var start = if (scratchAudio.totalFrames == 0) 0 else scratchAudio.totalFrames + 1
+        var start = if (scratchAudio.totalFrames == 0) 0 else scratchAudio.totalFrames
         var end: Int
         val frameSizeInBytes = chapterRepresentation.frameSizeInBytes
 
         segments.forEach { (marker, file) ->
             val verseAudio = AudioFile(file)
-            end = max(start + verseAudio.totalFrames - 1, 0)
+            end = max(start + verseAudio.totalFrames, 0)
 
             val node = VerseNode(
                 true,
                 marker,
                 mutableListOf(start * frameSizeInBytes until end * frameSizeInBytes)
             )
             nodes.add(node)
-
-            start = end + 1
+            start = end
         }
 
         return nodes.sortedBy { it.marker.sort } // sort order of book-chapter-verse

diff --git a/.../src/main/kotlin/org/wycliffeassociates/otter/common/domain/narration/NarrationActions.kt b/.../src/main/kotlin/org/wycliffeassociates/otter/common/domain/narration/NarrationActions.kt
@@ -53,7 +53,7 @@ internal class NewVerseAction(
     override fun execute(
         totalVerses: MutableList<VerseNode>, workingAudio: AudioFile
     ) {
-        val start = (if (workingAudio.totalFrames == 0) 0 else workingAudio.totalFrames + 1) * frameSizeInBytes
+        val start = (if (workingAudio.totalFrames == 0) 0 else workingAudio.totalFrames) * frameSizeInBytes
         val end = start
 
         logger.info("New marker added: ${totalVerses[verseIndex].marker.formattedLabel} at $start")
@@ -105,7 +105,7 @@ internal class RecordAgainAction(
         logger.info("Recording again for: ${totalVerses[verseIndex].marker.formattedLabel}")
         previous = totalVerses[verseIndex].copy()
 
-        val start = (if (workingAudio.totalFrames == 0) 0 else workingAudio.totalFrames + 1) * frameSizeInBytes
+        val start = (if (workingAudio.totalFrames == 0) 0 else workingAudio.totalFrames) * frameSizeInBytes
         val end = start
 
         node = VerseNode(

diff --git a/common/src/main/kotlin/org/wycliffeassociates/otter/common/domain/narration/VerseNode.kt b/common/src/main/kotlin/org/wycliffeassociates/otter/common/domain/narration/VerseNode.kt
@@ -63,7 +63,7 @@ internal data class VerseNode(
         if (sectors.isNotEmpty()) {
             val last = sectors.last()
             sectors.removeLast()
-            sectors.add(last.first until end)
+            sectors.add(last.first..end)
         } else {
             throw IllegalStateException("Tried to finalize VerseNode ${marker.label} that was not started!")
         }

diff --git a/...rc/test/kotlin/org/wycliffeassociates/otter/common/domain/narration/NewVerseActionTest.kt b/...rc/test/kotlin/org/wycliffeassociates/otter/common/domain/narration/NewVerseActionTest.kt
@@ -49,8 +49,8 @@ class NewVerseActionTest {
     }
 
     fun initializeTotalVerses() {
-        for(i in 0 until numTestVerses){
-            val verseMarker = VerseMarker((i+1), (i+1), 0)
+        for (i in 0 until numTestVerses) {
+            val verseMarker = VerseMarker((i + 1), (i + 1), 0)
             val sectors = mutableListOf<IntRange>()
             val verseNode = VerseNode(false, verseMarker, sectors)
             totalVerses.add(verseNode)
@@ -95,7 +95,8 @@ class NewVerseActionTest {
         newVerseAction.execute(totalVerses, workingAudioFile)
 
         // verify that NewVerseAction.node is valid
-        val expectedIndexRange = (totalFramesInTestAudio + 1) * frameSizeInBytes..(totalFramesInTestAudio + 1) * frameSizeInBytes
+        val expectedIndexRange =
+            (totalFramesInTestAudio) * frameSizeInBytes..(totalFramesInTestAudio) * frameSizeInBytes
         Assert.assertEquals(expectedIndexRange, newVerseAction.node?.sectors?.last())
         Assert.assertEquals(true, newVerseAction?.node?.placed)
 
@@ -112,7 +113,7 @@ class NewVerseActionTest {
         try {
             newVerseAction.undo(totalVerses)
             Assert.fail("expecting IndexOutOfBoundsException")
-        } catch (illegalIndex: IndexOutOfBoundsException){
+        } catch (illegalIndex: IndexOutOfBoundsException) {
             // Success: expecting IndexOutOfBoundsException
         }
     }

diff --git a/...test/kotlin/org/wycliffeassociates/otter/common/domain/narration/RecordAgainActionTest.kt b/...test/kotlin/org/wycliffeassociates/otter/common/domain/narration/RecordAgainActionTest.kt
@@ -47,8 +47,8 @@ class RecordAgainActionTest {
     }
 
     fun initializeTotalVerses() {
-        for(i in 0 until numTestVerses){
-            val verseMarker = VerseMarker((i+1), (i+1), 0)
+        for (i in 0 until numTestVerses) {
+            val verseMarker = VerseMarker((i + 1), (i + 1), 0)
             val sectors = mutableListOf<IntRange>()
             val verseNode = VerseNode(false, verseMarker, sectors)
             totalVerses.add(verseNode)
@@ -74,7 +74,7 @@ class RecordAgainActionTest {
         recordAgainAction.execute(totalVerses, workingAudioFile)
 
         // verify that RecordAgainAction.node is valid
-        val expectedEnd = workingAudioFile.totalFrames + 1
+        val expectedEnd = workingAudioFile.totalFrames
         Assert.assertEquals(
             expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes,
             recordAgainAction.node?.sectors?.last()
@@ -112,7 +112,7 @@ class RecordAgainActionTest {
         recordAgainAction.execute(totalVerses, workingAudioFile)
 
         // verify that RecordAgainAction.node is valid
-        val expectedEnd = workingAudioFile.totalFrames + 1
+        val expectedEnd = workingAudioFile.totalFrames
         Assert.assertEquals(
             expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes,
             recordAgainAction.node?.sectors?.last()
@@ -140,7 +140,7 @@ class RecordAgainActionTest {
         try {
             recordAgainAction.execute(totalVerses, workingAudioFile)
             Assert.fail("expecting IndexOutOfBoundsException")
-        } catch (illegalIndex: IndexOutOfBoundsException){
+        } catch (illegalIndex: IndexOutOfBoundsException) {
             // Success: expecting IndexOutOfBoundsException
         }
     }
@@ -172,7 +172,7 @@ class RecordAgainActionTest {
         recordAgainAction.execute(totalVerses, workingAudioFile)
 
         // verify that totalVerses[verseIndex] is valid
-        val expectedEnd = workingAudioFile.totalFrames + 1
+        val expectedEnd = workingAudioFile.totalFrames
         Assert.assertEquals(
             expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes,
             totalVerses[verseIndex].sectors.last()
@@ -198,7 +198,7 @@ class RecordAgainActionTest {
         recordAgainAction.execute(totalVerses, workingAudioFile)
 
         // verify that totalVerses[verseIndex] is valid
-        val expectedEnd = workingAudioFile.totalFrames + 1
+        val expectedEnd = workingAudioFile.totalFrames
         Assert.assertEquals(
             expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes,
             totalVerses[verseIndex].sectors.last()
@@ -215,8 +215,7 @@ class RecordAgainActionTest {
 
         // verify that totalVerses[verseIndex] is redone
         Assert.assertEquals(
-            expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes
-            , totalVerses[verseIndex].sectors.last()
+            expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes, totalVerses[verseIndex].sectors.last()
         )
         Assert.assertTrue(totalVerses[verseIndex].placed)
     }
@@ -251,7 +250,7 @@ class RecordAgainActionTest {
         recordAgainAction.execute(totalVerses, workingAudioFile)
 
         // Verify that totalVerse[verseIndex] has been updated
-        val expectedEnd = workingAudioFile.totalFrames + 1
+        val expectedEnd = workingAudioFile.totalFrames
         Assert.assertEquals(
             expectedEnd * frameSizeInBytes..expectedEnd * frameSizeInBytes,
             totalVerses[verseIndex].sectors.last()
@@ -262,7 +261,7 @@ class RecordAgainActionTest {
 
         // Verify that totalVerse[verseIndex] has been finalized
         Assert.assertEquals(
-            expectedEnd * frameSizeInBytes .. 882000 * frameSizeInBytes,
+            expectedEnd * frameSizeInBytes..882000 * frameSizeInBytes,
             totalVerses[verseIndex].sectors.last()
         )
         Assert.assertTrue(previousNode.placed)