From a875f4cf00dd8ca34591c71a4f38a5d63b4ae740 Mon Sep 17 00:00:00 2001
From: Mikael Finstad <finstaden@gmail.com>
Date: Thu, 29 Oct 2020 20:43:31 +0100
Subject: [PATCH] refactor

---
 audio.js       | 20 ++++++++++----------
 parseConfig.js | 12 +++++++-----
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/audio.js b/audio.js
index dfd3a7e..61acff8 100644
--- a/audio.js
+++ b/audio.js
@@ -95,7 +95,7 @@ module.exports = ({ ffmpegPath, ffprobePath, enableFfmpegLog, verbose, tmpDir })
 
         if (processedAudioLayers.length === 1) return { clipAudioPath: processedAudioLayers[0].layerAudioPath };
 
-        // Merge/mix all layer's audio
+        // Merge/mix all layers' audio
         const weights = processedAudioLayers.map(({ audioLayer }) => (audioLayer.mixVolume != null ? audioLayer.mixVolume : 1));
         const args = [
           ...getFfmpegCommonArgs({ enableFfmpegLog }),
@@ -120,12 +120,12 @@ module.exports = ({ ffmpegPath, ffprobePath, enableFfmpegLog, verbose, tmpDir })
     }, { concurrency: 4 });
   }
 
-  async function mergeFadeClipAudio(clipAudio) {
+  async function crossFadeConcatClipAudio(clipAudio) {
     if (clipAudio.length < 2) {
       return clipAudio[0].path;
     }
 
-    const mergedClipAudioPath = join(tmpDir, 'audio-merged.flac');
+    const outPath = join(tmpDir, 'audio-concat.flac');
 
     if (verbose) console.log('Combining audio', clipAudio.map(({ path }) => basename(path)));
 
@@ -149,11 +149,11 @@ module.exports = ({ ffmpegPath, ffprobePath, enableFfmpegLog, verbose, tmpDir })
       filterGraph,
       '-c', 'flac',
       '-y',
-      mergedClipAudioPath,
+      outPath,
     ];
     await execa(ffmpegPath, args);
 
-    return mergedClipAudioPath;
+    return outPath;
   }
 
   async function mixArbitraryAudio({ streams, audioNorm }) {
@@ -214,19 +214,19 @@ module.exports = ({ ffmpegPath, ffprobePath, enableFfmpegLog, verbose, tmpDir })
     if (clipAudio.every((ca) => ca.silent) && arbitraryAudio.length === 0) return undefined;
 
     // Merge & fade the clip audio files
-    const mergedClipAudioPath = await mergeFadeClipAudio(clipAudio);
+    const concatedClipAudioPath = await crossFadeConcatClipAudio(clipAudio);
 
     const streams = [
-      // The first stream is required, and it determines the length of the output audio.
-      // All other streams will be truncated to this length
-      { path: mergedClipAudioPath, mixVolume: clipsAudioVolume },
+      // The first stream is required, as it determines the length of the output audio.
+      // All other streams will be truncated to its length
+      { path: concatedClipAudioPath, mixVolume: clipsAudioVolume },
 
       ...arbitraryAudio,
     ];
 
     console.log('Mixing clip audio with arbitrary audio');
 
-    if (streams.length < 2) return mergedClipAudioPath;
+    if (streams.length < 2) return concatedClipAudioPath;
 
     const mixedFile = await mixArbitraryAudio({ streams, audioNorm });
     return mixedFile;
diff --git a/parseConfig.js b/parseConfig.js
index 99ed55a..eb8a4fb 100644
--- a/parseConfig.js
+++ b/parseConfig.js
@@ -216,17 +216,18 @@ async function parseConfig({ defaults: defaultsIn = {}, clips, arbitraryAudio: a
       }
 
       // These audio tracks are detached from the clips (can run over multiple clips)
-      // This is useful so we can have audio start relative to clip start time
+      // This is useful so we can have audio start relative to their parent clip's start time
       if (type === 'detached-audio') {
         const { cutFrom, cutTo, mixVolume } = layer;
         if (!detachedAudioByClip[clipIndex]) detachedAudioByClip[clipIndex] = [];
         detachedAudioByClip[clipIndex].push({ path, cutFrom, cutTo, mixVolume, start });
-        return undefined;
+        return undefined; // Will be filtered out
       }
 
       return layer;
     });
 
+    // Filter out deleted layers
     layersOut = layersOut.filter((l) => l);
 
     return {
@@ -240,11 +241,12 @@ async function parseConfig({ defaults: defaultsIn = {}, clips, arbitraryAudio: a
   let totalClipDuration = 0;
   const clipDetachedAudio = [];
 
-  // Need to map again because now we know all clip durations
+  // Need to map again because now we know all clip durations, and we can adjust transitions so they are safe
   clipsOut = await pMap(clipsOut, async (clip, i) => {
     const nextClip = clipsOut[i + 1];
 
-    // We clamp all transitions to half the length of every clip
+    // We clamp all transitions to half the length of every clip. If not, we risk that clips that are too short,
+    // will be eaten by transitions and could cause de-sync issues with audio/video
     // NOTE: similar logic is duplicated in index.js
     let safeTransitionDuration = 0;
     if (nextClip) {
@@ -269,7 +271,7 @@ async function parseConfig({ defaults: defaultsIn = {}, clips, arbitraryAudio: a
     };
   });
 
-  // Audio can either come from `audioFilePath`, `audio` or from "detached" audio layers in clips
+  // Audio can either come from `audioFilePath`, `audio` or from "detached" audio layers from clips
   const arbitraryAudio = [
     // Background audio is treated just like arbitrary audio
     ...(backgroundAudioPath ? [{ path: backgroundAudioPath, mixVolume: 1, loop: loopAudio ? -1 : 0 }] : []),