add options for -xerror and other global params to EncodingProperties. Added environment variable ENCORE_TMPDIR to reliably configure temp directory.

fhermansson · John Laurin (jola10) · commit 6998b8387a2b · 2024-04-29T14:20:21.000+02:00
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/config/EncodingProperties.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/config/EncodingProperties.kt
@@ -12,5 +12,7 @@ data class EncodingProperties(
     val audioMixPresets: Map<String, AudioMixPreset> = mapOf("default" to AudioMixPreset()),
     @NestedConfigurationProperty
     val defaultChannelLayouts: Map<Int, ChannelLayout> = emptyMap(),
-    val flipWidthHeightIfPortrait: Boolean = true
+    val flipWidthHeightIfPortrait: Boolean = true,
+    val exitOnError: Boolean = true,
+    val globalParams: LinkedHashMap<String, Any?> = linkedMapOf(),
 )
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/model/mediafile/Extensions.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/model/mediafile/Extensions.kt
@@ -83,4 +83,5 @@ fun AudioFile.selectAudioStream(index: Int?): AudioFile {
 fun Map<String, Any?>.toParams(): List<String> =
     flatMap { entry ->
         listOfNotNull("-${entry.key}", entry.value?.let { "$it" })
+            .filterNot { it.isEmpty() }
     }
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/model/profile/ThumbnailMapEncode.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/model/profile/ThumbnailMapEncode.kt
@@ -14,8 +14,8 @@ import se.svt.oss.encore.model.input.videoInput
 import se.svt.oss.encore.model.mediafile.toParams
 import se.svt.oss.encore.model.output.Output
 import se.svt.oss.encore.model.output.VideoStreamEncode
+import se.svt.oss.encore.process.createTempDir
 import se.svt.oss.mediaanalyzer.file.stringValue
-import kotlin.io.path.createTempDirectory
 
 data class ThumbnailMapEncode(
     val tileWidth: Int = 160,
@@ -53,7 +53,7 @@ data class ThumbnailMapEncode(
             ?.let { "gte(t\\,$it)*(isnan(prev_selected_t)+gt(floor((t-$it)/$interval)\\,floor((prev_selected_t-$it)/$interval)))" }
             ?: "isnan(prev_selected_t)+gt(floor(t/$interval)\\,floor(prev_selected_t/$interval))"
 
-        val tempFolder = createTempDirectory(suffix).toFile()
+        val tempFolder = createTempDir(suffix).toFile()
         tempFolder.deleteOnExit()
 
         val pad = "aspect=${Fraction(tileWidth, tileHeight).stringValue()}:x=(ow-iw)/2:y=(oh-ih)/2"
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/process/CommandBuilder.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/process/CommandBuilder.kt
@@ -15,6 +15,7 @@ import se.svt.oss.encore.model.input.inputParams
 import se.svt.oss.encore.model.mediafile.AudioLayout
 import se.svt.oss.encore.model.mediafile.audioLayout
 import se.svt.oss.encore.model.mediafile.channelLayout
+import se.svt.oss.encore.model.mediafile.toParams
 import se.svt.oss.encore.model.output.AudioStreamEncode
 import se.svt.oss.encore.model.output.Output
 import se.svt.oss.encore.model.output.VideoStreamEncode
@@ -165,13 +166,15 @@ class CommandBuilder(
         val readDuration = encoreJob.duration?.let {
             it + (encoreJob.seekTo ?: 0.0)
         }
-        return listOf(
-            "ffmpeg",
-            "-hide_banner",
-            "-loglevel",
-            "+level",
-            "-y"
-        ) + inputs.inputParams(readDuration)
+        return buildList {
+            add("ffmpeg")
+            if (encodingProperties.exitOnError) {
+                add("-xerror")
+            }
+            addAll(encodingProperties.globalParams.toParams())
+            addAll(listOf("-hide_banner", "-loglevel", "+level", "-y"))
+            addAll(inputs.inputParams(readDuration))
+        }
     }
 
     private fun globalVideoFilters(input: VideoIn, videoFile: VideoFile): List<String> {
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/process/TempDir.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/process/TempDir.kt
@@ -0,0 +1,10 @@
+package se.svt.oss.encore.process
+
+import java.nio.file.Path
+import kotlin.io.path.createTempDirectory
+
+fun createTempDir(prefix: String): Path {
+    val tmpdir = System.getenv("ENCORE_TMPDIR")
+        ?: System.getProperty("java.io.tmpdir")
+    return createTempDirectory(tmpdir?.let { Path.of(it) }, prefix)
+}
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/service/FfmpegExecutor.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/service/FfmpegExecutor.kt
@@ -14,11 +14,11 @@ import se.svt.oss.encore.model.EncoreJob
 import se.svt.oss.encore.model.input.maxDuration
 import se.svt.oss.encore.model.mediafile.toParams
 import se.svt.oss.encore.process.CommandBuilder
+import se.svt.oss.encore.process.createTempDir
 import se.svt.oss.encore.service.profile.ProfileService
 import se.svt.oss.mediaanalyzer.MediaAnalyzer
 import se.svt.oss.mediaanalyzer.file.MediaFile
 import java.io.File
-import java.nio.file.Files
 import java.util.concurrent.TimeUnit
 import kotlin.math.min
 import kotlin.math.round
@@ -57,7 +57,7 @@ class FfmpegExecutor(
         val commands =
             CommandBuilder(encoreJob, profile, outputFolder, encoreProperties.encoding).buildCommands(outputs)
         log.info { "Start encoding ${encoreJob.baseName}..." }
-        val workDir = Files.createTempDirectory("encore_").toFile()
+        val workDir = createTempDir("encore_").toFile()
         val duration = encoreJob.duration ?: encoreJob.inputs.maxDuration()
         return try {
             File(outputFolder).mkdirs()
diff --git a/encore-common/src/main/kotlin/se/svt/oss/encore/service/localencode/LocalEncodeService.kt b/encore-common/src/main/kotlin/se/svt/oss/encore/service/localencode/LocalEncodeService.kt
@@ -6,12 +6,13 @@ package se.svt.oss.encore.service.localencode
 
 import mu.KotlinLogging
 import org.springframework.stereotype.Service
+import se.svt.oss.encore.config.EncoreProperties
+import se.svt.oss.encore.model.EncoreJob
+import se.svt.oss.encore.process.createTempDir
 import se.svt.oss.mediaanalyzer.file.AudioFile
 import se.svt.oss.mediaanalyzer.file.ImageFile
 import se.svt.oss.mediaanalyzer.file.MediaFile
 import se.svt.oss.mediaanalyzer.file.VideoFile
-import se.svt.oss.encore.config.EncoreProperties
-import se.svt.oss.encore.model.EncoreJob
 import java.io.File
 import java.nio.file.Files
 import java.nio.file.Path
@@ -28,7 +29,7 @@ class LocalEncodeService(
         encoreJob: EncoreJob
     ): String {
         return if (encoreProperties.localTemporaryEncode) {
-            Files.createTempDirectory("job_${encoreJob.id}").toString()
+            createTempDir("job_${encoreJob.id}").toString()
         } else {
             encoreJob.outputFolder
         }
@@ -58,12 +59,7 @@ class LocalEncodeService(
     }
 
     private fun moveTempLocalFiles(destination: File, tempDirectory: String) {
-        val filesBeforeMove = File(tempDirectory).listFiles()
-        filesBeforeMove?.forEach { moveFile(it, destination) }
-        val fileCountAfterMove = destination.list()?.size
-        if (fileCountAfterMove != filesBeforeMove?.count()) {
-            throw RuntimeException("File count after moving files from temp to output folder differs.")
-        }
+        File(tempDirectory).listFiles()?.forEach { moveFile(it, destination) }
     }
 
     private fun moveFile(file: File, destination: File) {
diff --git a/encore-common/src/test/kotlin/se/svt/oss/encore/process/CommandBuilderTest.kt b/encore-common/src/test/kotlin/se/svt/oss/encore/process/CommandBuilderTest.kt
@@ -29,7 +29,7 @@ internal class CommandBuilderTest {
     val profile: Profile = mockk()
     var videoFile = defaultVideoFile
     var encoreJob = defaultEncoreJob()
-    val encodingProperties = mockk<EncodingProperties>()
+    val encodingProperties = EncodingProperties()
 
     private lateinit var commandBuilder: CommandBuilder
 
@@ -38,7 +38,6 @@ internal class CommandBuilderTest {
     @BeforeEach
     internal fun setUp() {
         commandBuilder = CommandBuilder(encoreJob, profile, encoreJob.outputFolder, encodingProperties)
-        every { encodingProperties.defaultChannelLayouts } returns emptyMap()
         every { profile.scaling } returns "scaling"
         every { profile.deinterlaceFilter } returns "yadif"
     }
@@ -59,7 +58,7 @@ internal class CommandBuilderTest {
         )
         val buildCommands = commandBuilder.buildCommands(listOf(output))
         val command = buildCommands.first().joinToString(" ")
-        assertThat(command).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -i /input/test.mp4 -map 0:a -vn -c:a copy -metadata comment=Transcoded using Encore /output/path/out.mp4")
+        assertThat(command).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -i /input/test.mp4 -map 0:a -vn -c:a copy -metadata comment=Transcoded using Encore /output/path/out.mp4")
     }
 
     @Test
@@ -88,7 +87,7 @@ internal class CommandBuilderTest {
         )
         val buildCommands = commandBuilder.buildCommands(listOf(output))
         val command = buildCommands.first().joinToString(" ")
-        assertThat(command).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -i /input/test.mp4 -filter_complex sws_flags=scaling;[0:a]join=inputs=3:channel_layout=3.0:map=0.0-FL|1.0-FR|2.0-FC,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]aformat=channel_layouts=stereo[AUDIO-test-out-0] -map [AUDIO-test-out-0] -vn -c:a:0 aac -metadata comment=Transcoded using Encore /output/path/out.mp4")
+        assertThat(command).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -i /input/test.mp4 -filter_complex sws_flags=scaling;[0:a]join=inputs=3:channel_layout=3.0:map=0.0-FL|1.0-FR|2.0-FC,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]aformat=channel_layouts=stereo[AUDIO-test-out-0] -map [AUDIO-test-out-0] -vn -c:a:0 aac -metadata comment=Transcoded using Encore /output/path/out.mp4")
     }
 
     @Test
@@ -98,7 +97,7 @@ internal class CommandBuilderTest {
         assertThat(buildCommands).hasSize(1)
 
         val command = buildCommands.first().joinToString(" ")
-        assertThat(command).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -i /input/test.mp4 -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[0:a]join=inputs=8:channel_layout=7.1:map=0.0-FL|1.0-FR|2.0-FC|3.0-LFE|4.0-BL|5.0-BR|6.0-SL|7.0-SR,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0] -map [VIDEO-test-out] -map [AUDIO-test-out-0] video params audio params -metadata comment=Transcoded using Encore /output/path/out.mp4")
+        assertThat(command).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -i /input/test.mp4 -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[0:a]join=inputs=8:channel_layout=7.1:map=0.0-FL|1.0-FR|2.0-FC|3.0-LFE|4.0-BL|5.0-BR|6.0-SL|7.0-SR,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0] -map [VIDEO-test-out] -map [AUDIO-test-out-0] video params audio params -metadata comment=Transcoded using Encore /output/path/out.mp4")
     }
 
     @Test
@@ -109,8 +108,8 @@ internal class CommandBuilderTest {
         val firstPass = buildCommands[0].joinToString(" ")
         val secondPass = buildCommands[1].joinToString(" ")
 
-        assertThat(firstPass).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -i ${defaultVideoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out] -map [VIDEO-test-out] -an first pass -f mp4 /dev/null")
-        assertThat(secondPass).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -i ${defaultVideoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[0:a]join=inputs=8:channel_layout=7.1:map=0.0-FL|1.0-FR|2.0-FC|3.0-LFE|4.0-BL|5.0-BR|6.0-SL|7.0-SR,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0] -map [VIDEO-test-out] -map [AUDIO-test-out-0] video params audio params -metadata comment=$metadataComment /output/path/out.mp4")
+        assertThat(firstPass).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -i ${defaultVideoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out] -map [VIDEO-test-out] -an first pass -f mp4 /dev/null")
+        assertThat(secondPass).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -i ${defaultVideoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[0:a]join=inputs=8:channel_layout=7.1:map=0.0-FL|1.0-FR|2.0-FC|3.0-LFE|4.0-BL|5.0-BR|6.0-SL|7.0-SR,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0] -map [VIDEO-test-out] -map [AUDIO-test-out-0] video params audio params -metadata comment=$metadataComment /output/path/out.mp4")
     }
 
     @Test
@@ -132,8 +131,8 @@ internal class CommandBuilderTest {
         val firstPass = buildCommands[0].joinToString(" ")
         val secondPass = buildCommands[1].joinToString(" ")
 
-        assertThat(firstPass).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -ss 47.11 -i ${videoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out] -map [VIDEO-test-out] -an first pass -f mp4 /dev/null")
-        assertThat(secondPass).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -ss 47.11 -i ${videoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[0:a]join=inputs=8:channel_layout=7.1:map=0.0-FL|1.0-FR|2.0-FC|3.0-LFE|4.0-BL|5.0-BR|6.0-SL|7.0-SR,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0] -map [VIDEO-test-out] -map [AUDIO-test-out-0] video params audio params -metadata comment=$metadataComment /output/path/out.mp4")
+        assertThat(firstPass).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -ss 47.11 -i ${videoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out] -map [VIDEO-test-out] -an first pass -f mp4 /dev/null")
+        assertThat(secondPass).isEqualTo("ffmpeg -xerror -hide_banner -loglevel +level -y -ss 47.11 -i ${videoFile.file} -filter_complex sws_flags=scaling;[0:v]split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[0:a]join=inputs=8:channel_layout=7.1:map=0.0-FL|1.0-FR|2.0-FC|3.0-LFE|4.0-BL|5.0-BR|6.0-SL|7.0-SR,asplit=1[AUDIO-main-test-out-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0] -map [VIDEO-test-out] -map [AUDIO-test-out-0] video params audio params -metadata comment=$metadataComment /output/path/out.mp4")
     }
 
     @Test
@@ -196,7 +195,12 @@ internal class CommandBuilderTest {
             inputs = inputs
         )
 
-        commandBuilder = CommandBuilder(encoreJob, profile, "/tmp/123", encodingProperties)
+        commandBuilder = CommandBuilder(
+            encoreJob,
+            profile,
+            "/tmp/123",
+            encodingProperties.copy(exitOnError = false, globalParams = linkedMapOf("err_detect" to "explode"))
+        )
 
         val buildCommands = commandBuilder.buildCommands(listOf(output(true), audioOutput("other", "extra")))
 
@@ -205,8 +209,8 @@ internal class CommandBuilderTest {
         val firstPass = buildCommands[0].joinToString(" ")
         val secondPass = buildCommands[1].joinToString(" ")
 
-        assertThat(firstPass).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -f mp4 -t 22.5 -i /input/test.mp4 -filter_complex sws_flags=scaling;[0:v:1]yadif,setdar=16/9,scale=iw*sar:ih,crop=min(iw\\,ih*1/1):min(ih\\,iw/(1/1)),pad=aspect=16/9:x=(ow-iw)/2:y=(oh-ih)/2,video,filter,split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out] -map [VIDEO-test-out] -ss 12.1 -an -t 10.4 first pass -f mp4 /dev/null")
-        assertThat(secondPass).isEqualTo("ffmpeg -hide_banner -loglevel +level -y -f mp4 -t 22.5 -i /input/test.mp4 -ac 4 -t 22.5 -i /input/main-audio.mp4 -t 22.5 -i /input/other-audio.mp4 -filter_complex sws_flags=scaling;[0:v:1]yadif,setdar=16/9,scale=iw*sar:ih,crop=min(iw\\,ih*1/1):min(ih\\,iw/(1/1)),pad=aspect=16/9:x=(ow-iw)/2:y=(oh-ih)/2,video,filter,split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[1:a]join=inputs=4:channel_layout=4.0:map=0.0-FL|1.0-FR|2.0-FC|3.0-BC,audio-main,main-filter,asplit=1[AUDIO-main-test-out-0];[2:a:3]asplit=1[AUDIO-other-extra-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0];[AUDIO-other-extra-0]audio-filter-extra[AUDIO-extra-0] -map [VIDEO-test-out] -ss 12.1 -map [AUDIO-test-out-0] -ss 12.1 -t 10.4 video params audio params -metadata comment=Transcoded using Encore /tmp/123/out.mp4 -map [AUDIO-extra-0] -ss 12.1 -t 10.4 -vn audio extra -metadata comment=Transcoded using Encore /tmp/123/extra.mp4")
+        assertThat(firstPass).isEqualTo("ffmpeg -err_detect explode -hide_banner -loglevel +level -y -f mp4 -t 22.5 -i /input/test.mp4 -filter_complex sws_flags=scaling;[0:v:1]yadif,setdar=16/9,scale=iw*sar:ih,crop=min(iw\\,ih*1/1):min(ih\\,iw/(1/1)),pad=aspect=16/9:x=(ow-iw)/2:y=(oh-ih)/2,video,filter,split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out] -map [VIDEO-test-out] -ss 12.1 -an -t 10.4 first pass -f mp4 /dev/null")
+        assertThat(secondPass).isEqualTo("ffmpeg -err_detect explode -hide_banner -loglevel +level -y -f mp4 -t 22.5 -i /input/test.mp4 -ac 4 -t 22.5 -i /input/main-audio.mp4 -t 22.5 -i /input/other-audio.mp4 -filter_complex sws_flags=scaling;[0:v:1]yadif,setdar=16/9,scale=iw*sar:ih,crop=min(iw\\,ih*1/1):min(ih\\,iw/(1/1)),pad=aspect=16/9:x=(ow-iw)/2:y=(oh-ih)/2,video,filter,split=1[VIDEO-main-test-out];[VIDEO-main-test-out]video-filter[VIDEO-test-out];[1:a]join=inputs=4:channel_layout=4.0:map=0.0-FL|1.0-FR|2.0-FC|3.0-BC,audio-main,main-filter,asplit=1[AUDIO-main-test-out-0];[2:a:3]asplit=1[AUDIO-other-extra-0];[AUDIO-main-test-out-0]audio-filter[AUDIO-test-out-0];[AUDIO-other-extra-0]audio-filter-extra[AUDIO-extra-0] -map [VIDEO-test-out] -ss 12.1 -map [AUDIO-test-out-0] -ss 12.1 -t 10.4 video params audio params -metadata comment=Transcoded using Encore /tmp/123/out.mp4 -map [AUDIO-extra-0] -ss 12.1 -t 10.4 -vn audio extra -metadata comment=Transcoded using Encore /tmp/123/extra.mp4")
     }
 
     private fun output(twoPass: Boolean): Output {

Original file line number	Diff line number	Diff line change
`@@ -12,5 +12,7 @@ data class EncodingProperties(`
`12`	`12`	`val audioMixPresets: Map<String, AudioMixPreset> = mapOf("default" to AudioMixPreset()),`
`13`	`13`	`@NestedConfigurationProperty`
`14`	`14`	`val defaultChannelLayouts: Map<Int, ChannelLayout> = emptyMap(),`
`15`		`- val flipWidthHeightIfPortrait: Boolean = true`
	`15`	`+ val flipWidthHeightIfPortrait: Boolean = true,`
	`16`	`+ val exitOnError: Boolean = true,`
	`17`	`+ val globalParams: LinkedHashMap<String, Any?> = linkedMapOf(),`
`16`	`18`	`)`
Original file line number	Diff line number	Diff line change
`@@ -83,4 +83,5 @@ fun AudioFile.selectAudioStream(index: Int?): AudioFile {`
`83`	`83`	`fun Map<String, Any?>.toParams(): List<String> =`
`84`	`84`	`flatMap { entry ->`
`85`	`85`	`listOfNotNull("-${entry.key}", entry.value?.let { "$it" })`
	`86`	`+ .filterNot { it.isEmpty() }`
`86`	`87`	`}`