From 5dd0b14b0fdf5042b3afffa17c550209afc4bab5 Mon Sep 17 00:00:00 2001 From: CXwudi Date: Mon, 14 Oct 2024 10:44:31 -0400 Subject: [PATCH 1/5] :recycle: Refactor ExtractorDecider for better separation of concerns Extract core logic into ExtractorDeciderCore for improved testability and reusability --- .../extractor/component/ExtractorDecider.kt | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDecider.kt b/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDecider.kt index dd8ff720..897c3d25 100644 --- a/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDecider.kt +++ b/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDecider.kt @@ -2,6 +2,7 @@ package mikufan.cx.vvd.extractor.component import mikufan.cx.inlinelogging.KInlineLogging import mikufan.cx.vvd.common.exception.RuntimeVocaloidException +import mikufan.cx.vvd.extractor.component.extractor.base.BaseAudioExtractor import mikufan.cx.vvd.extractor.component.extractor.impl.AacToM4aAudioExtractor import mikufan.cx.vvd.extractor.component.extractor.impl.OpusToOggAudioExtractor import mikufan.cx.vvd.extractor.config.IOConfig @@ -19,37 +20,52 @@ import kotlin.io.path.div import kotlin.io.path.exists import kotlin.io.path.notExists +@Component +@Order(OrderConstants.EXTRACTOR_DECIDER_ORDER) +class ExtractorDecider( + private val extractorDeciderCore: ExtractorDeciderCore +) : RecordProcessor { + + override fun processRecord(record: Record): Record { + val task = record.payload + val extractor = extractorDeciderCore.decideExtractor( + audioFileName = task.label.audioFileName, + videoFileName = task.label.pvFileName, + baseFileName = task.parameters.songProperFileName.toString() + ) + task.parameters.chosenAudioExtractor = Optional.ofNullable(extractor) + return record + } +} + + /** * @date 2022-07-01 * @author CX无敌 */ @Component -@Order(OrderConstants.EXTRACTOR_DECIDER_ORDER) -class ExtractorDecider( +class ExtractorDeciderCore( ioConfig: IOConfig, private val audioMediaFormatChecker: MediaFormatChecker, private val ctx: ApplicationContext, -) : RecordProcessor { +) { private val inputDirectory = ioConfig.inputDirectory - override fun processRecord(record: Record): Record { - val baseFileName = record.payload.parameters.songProperFileName + fun decideExtractor(audioFileName: String? = null, videoFileName: String, baseFileName: String): BaseAudioExtractor? { log.info { "Start deciding the best audio extractor for $baseFileName" } - // if the label contains a valid audio file, then skip extraction - val audioFileName: String? = record.payload.label.audioFileName + if (!audioFileName.isNullOrBlank()) { val audioFile = inputDirectory / audioFileName if (audioFile.exists()) { log.info { "Skip choosing audio extractor for $baseFileName as it contains an audio file $audioFile" } - record.payload.parameters.chosenAudioExtractor = Optional.empty() - return record + return null } else { - log.warn { "Audio file $audioFileName is declared in label json file but doesn't exist in inout directory, trading as no audio file" } + log.warn { "Audio file $audioFileName is declared but doesn't exist in input directory, treating as no audio file" } } } - val pvFile = inputDirectory / record.payload.label.pvFileName + val pvFile = inputDirectory / videoFileName if (pvFile.notExists()) { throw RuntimeVocaloidException( "pv file not found: ${pvFile.absolute()} for song $baseFileName. " + @@ -57,15 +73,15 @@ class ExtractorDecider( ) } - val chosenAudioExtractor = when (val audioFormat = audioMediaFormatChecker.checkAudioFormat(pvFile)) { + return when (val audioFormat = audioMediaFormatChecker.checkAudioFormat(pvFile)) { "aac" -> ctx.getBean() "opus" -> ctx.getBean() else -> throw RuntimeVocaloidException("Unsupported audio format $audioFormat for song $baseFileName") + }.also { + log.info { "Decided to use ${it.name} to extract audio from $baseFileName" } } - log.info { "Decided to use ${chosenAudioExtractor.name} to extract audio from $baseFileName" } - record.payload.parameters.chosenAudioExtractor = Optional.of(chosenAudioExtractor) - return record } } + private val log = KInlineLogging.logger() From f744cabaab30d40ae62331eaafe4a136eeb935f4 Mon Sep 17 00:00:00 2001 From: CXwudi Date: Mon, 14 Oct 2024 10:44:43 -0400 Subject: [PATCH 2/5] :white_check_mark: Add ExtractorDeciderCoreTest Implement comprehensive unit tests for ExtractorDeciderCore to ensure correct audio extractor selection and error handling --- .../component/ExtractorDeciderCoreTest.kt | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt diff --git a/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt b/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt new file mode 100644 index 00000000..fd45da51 --- /dev/null +++ b/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt @@ -0,0 +1,102 @@ +package mikufan.cx.vvd.extractor.component + +import io.kotest.assertions.fail +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.core.spec.style.ShouldSpec +import io.kotest.matchers.nulls.shouldBeNull +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.string.shouldContain +import io.kotest.matchers.types.shouldBeInstanceOf +import io.mockk.every +import io.mockk.mockk +import mikufan.cx.vvd.common.exception.RuntimeVocaloidException +import mikufan.cx.vvd.extractor.component.extractor.base.BaseAudioExtractor +import mikufan.cx.vvd.extractor.component.extractor.impl.AacToM4aAudioExtractor +import mikufan.cx.vvd.extractor.component.extractor.impl.OpusToOggAudioExtractor +import mikufan.cx.vvd.extractor.config.IOConfig +import org.springframework.beans.factory.getBean +import org.springframework.context.ApplicationContext +import java.nio.file.Files +import kotlin.io.path.createFile +import kotlin.io.path.deleteExisting +import kotlin.io.path.div + +class ExtractorDeciderCoreTest : ShouldSpec({ + + context("extractor decider") { + val tempInputDir = Files.createTempDirectory("extractor-core-test-") + val ioConfig = mockk { + every { inputDirectory } returns tempInputDir + } + val baseInputFileName = "fake input file" + + val mockCtx = mockk { + every { getBean() } returns mockk() + every { getBean() } returns mockk() + } + + should("not set audio extractor if using audio file") { + val audioFileName = "$baseInputFileName.aac" + val audioFile = tempInputDir / audioFileName + audioFile.createFile() + + val extractorDeciderCore = ExtractorDeciderCore(ioConfig, mockk(), mockk()) + + val decideExtractor: BaseAudioExtractor? = extractorDeciderCore.decideExtractor(audioFileName, "", baseInputFileName) + + decideExtractor.shouldBeNull() + audioFile.deleteExisting() + } + + context("on pv files") { + listOf("aac", "opus").forEach { format -> + val mockChecker = mockk { + every { checkAudioFormat(any()) } returns format + } + + val pvFileName = "$baseInputFileName.mp4" + val pvFile = tempInputDir / pvFileName + pvFile.createFile() + + should("set the correct extractor for $format format") { + val extractorDeciderCore = ExtractorDeciderCore(ioConfig, mockChecker, mockCtx) + val decideExtractor: BaseAudioExtractor? = extractorDeciderCore.decideExtractor("", pvFileName, baseInputFileName) + decideExtractor.shouldNotBeNull() + when (format) { + "aac" -> decideExtractor.shouldBeInstanceOf() + "opus" -> decideExtractor.shouldBeInstanceOf() + else -> fail("Unknown format $format") + } + } + pvFile.deleteExisting() + } + } + + should("fails if encounter an unknown pv file format") { + val mockChecker = mockk { + every { checkAudioFormat(any()) } returns "wired format" + } + + val pvFileName = "$baseInputFileName.mp4" + val pvFile = tempInputDir / pvFileName + pvFile.createFile() + + val extractorDeciderCore = ExtractorDeciderCore(ioConfig, mockChecker, mockCtx) + val exception = shouldThrow { + extractorDeciderCore.decideExtractor("", pvFileName, baseInputFileName) + } + exception.message shouldContain "Unsupported audio format" + + pvFile.deleteExisting() + } + + should("fails if neither audio file nor pv file exists") { + val extractorDeciderCore = ExtractorDeciderCore(ioConfig, mockk(), mockk()) + val exception = shouldThrow { + extractorDeciderCore.decideExtractor("fake.mp3", "fake.mp4", baseInputFileName) + } + + exception.message shouldContain "pv file not found" + } + } +}) \ No newline at end of file From 410f4e5b5279fb03cf366e3ea3995871493b9f65 Mon Sep 17 00:00:00 2001 From: CXwudi Date: Sat, 14 Dec 2024 13:43:43 -0500 Subject: [PATCH 3/5] :bug: Fix mock setup in ExtractorDeciderCoreTest Add name property to mocked audio extractors to resolve test failures --- .../vvd/extractor/component/ExtractorDeciderCoreTest.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt b/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt index fd45da51..e68907aa 100644 --- a/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt +++ b/vvd-extractor/src/test/kotlin/mikufan/cx/vvd/extractor/component/ExtractorDeciderCoreTest.kt @@ -31,8 +31,12 @@ class ExtractorDeciderCoreTest : ShouldSpec({ val baseInputFileName = "fake input file" val mockCtx = mockk { - every { getBean() } returns mockk() - every { getBean() } returns mockk() + every { getBean() } returns mockk { + every { name } returns "Mock AAC to M4A Audio Extractor" + } + every { getBean() } returns mockk { + every { name } returns "Mock Opus to Ogg Audio Extractor" + } } should("not set audio extractor if using audio file") { From b99b89d2bb3b09944ad84d1e13f7705349a35d19 Mon Sep 17 00:00:00 2001 From: CXwudi Date: Sat, 14 Dec 2024 13:43:58 -0500 Subject: [PATCH 4/5] :memo: Update audio extractor documentation Improve clarity and formatting of comments in OpusToOggAudioExtractor and AacToM4aAudioExtractor classes --- .../component/extractor/impl/AacToM4aAudioExtractor.kt | 9 +++++---- .../component/extractor/impl/OpusToOggAudioExtractor.kt | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/AacToM4aAudioExtractor.kt b/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/AacToM4aAudioExtractor.kt index d8b9bc32..c30e8d57 100644 --- a/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/AacToM4aAudioExtractor.kt +++ b/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/AacToM4aAudioExtractor.kt @@ -12,12 +12,13 @@ import kotlin.io.path.deleteIfExists import kotlin.io.path.exists /** - * The lossless audio extractor for any video with AAC LC audio track. + * The lossless audio extractor for video with AAC LC audio track. * Extracted audio will be in m4a format. * - * It execute two commands: - * 1. ffmpeg -i input.mp4 -vn -acodec copy -y temp.aac - * 2. ffmpeg -i temp.aac -vn --acodec copy -y -movflags +faststart output.m4a + * It executes two commands: + * 1. `ffmpeg -i input.mp4 -vn -acodec copy -y temp.aac` to extract the raw audio stream + * 2. `ffmpeg -i temp.aac -vn --acodec copy -y -movflags +faststart output.m4a` + * to package the audio stream into m4a container with iTunes style faststart flag * * @date 2022-07-16 * @author CX无敌 diff --git a/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/OpusToOggAudioExtractor.kt b/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/OpusToOggAudioExtractor.kt index e581d875..8221cdab 100644 --- a/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/OpusToOggAudioExtractor.kt +++ b/vvd-extractor/src/main/kotlin/mikufan/cx/vvd/extractor/component/extractor/impl/OpusToOggAudioExtractor.kt @@ -10,12 +10,13 @@ import java.util.concurrent.ThreadPoolExecutor import kotlin.io.path.exists /** - * The lossless audio extractor for any video with opus audio track (or any ogg/opus related audio codec). + * The lossless audio extractor for video with opus audio track (or any ogg/opus related audio codec). * Extracted audio will be in ogg format. - * Although it is preferred to extracted as opus, but since NetEase Cloud Music does not support opus, it will be extracted as ogg. + * Although it is preferred to be extracted as opus, + * but since NetEase Cloud Music does not support opus, it will be extracted as ogg. * * It will run this command: - * ffmpeg -i input.mkv -vn -acodec copy output.ogg + * `ffmpeg -i input.mkv -vn -acodec copy output.ogg` * * @date 2022-07-16 * @author CX无敌 From e528c2cf70ffa93c09f2941a193f87cc517f9e38 Mon Sep 17 00:00:00 2001 From: CXwudi Date: Sat, 14 Dec 2024 13:45:59 -0500 Subject: [PATCH 5/5] :recycle: Remove deprecated version tag from Docker Compose files Improve compatibility with newer Docker Compose versions --- docker/docker-compose.base.yml | 2 -- docker/docker-compose.debug-test-all.yml | 2 -- docker/docker-compose.test-all.yml | 2 -- 3 files changed, 6 deletions(-) diff --git a/docker/docker-compose.base.yml b/docker/docker-compose.base.yml index 30164f15..e20a83fb 100644 --- a/docker/docker-compose.base.yml +++ b/docker/docker-compose.base.yml @@ -1,5 +1,3 @@ -version: '3.7' - services: base: image: vvd-env diff --git a/docker/docker-compose.debug-test-all.yml b/docker/docker-compose.debug-test-all.yml index fd8b3263..a9d08d04 100644 --- a/docker/docker-compose.debug-test-all.yml +++ b/docker/docker-compose.debug-test-all.yml @@ -1,6 +1,4 @@ # this compose file should be run together with .base.yml file -version: '3.7' - services: base: container_name: vvd-debug-test-all diff --git a/docker/docker-compose.test-all.yml b/docker/docker-compose.test-all.yml index 3eba64d7..e09836dc 100644 --- a/docker/docker-compose.test-all.yml +++ b/docker/docker-compose.test-all.yml @@ -1,6 +1,4 @@ # this compose file should be run together with .base.yml file -version: '3.7' - services: base: container_name: vvd-test-all