From baefe61cff0f4809a41a23241b3214d94834bc7f Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Wed, 11 Sep 2024 16:05:10 +0200
Subject: [PATCH] Fix duplicated resolutions when capping fps

---
 .../src/api/transcoding/transcoder-limits.ts  | 46 ++++++++++++++-----
 server/core/lib/live/live-utils.ts            |  2 +-
 .../job-builders/abstract-job-builder.ts      |  8 ++--
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/packages/tests/src/api/transcoding/transcoder-limits.ts b/packages/tests/src/api/transcoding/transcoder-limits.ts
index d3f3b45d3..8189b8673 100644
--- a/packages/tests/src/api/transcoding/transcoder-limits.ts
+++ b/packages/tests/src/api/transcoding/transcoder-limits.ts
@@ -64,6 +64,16 @@ describe('Test video transcoding limits', function () {
       }
     }
 
+    function updateMaxFPS (value: number) {
+      return servers[1].config.updateExistingConfig({
+        newConfig: {
+          transcoding: {
+            fps: { max: value }
+          }
+        }
+      })
+    }
+
     it('Should transcode a 60 FPS video', async function () {
       this.timeout(60_000)
 
@@ -117,25 +127,37 @@ describe('Test video transcoding limits', function () {
     it('Should configure max FPS', async function () {
       this.timeout(120_000)
 
-      const update = (value: number) => {
-        return servers[1].config.updateExistingConfig({
-          newConfig: {
-            transcoding: {
-              fps: { max: value }
-            }
-          }
-        })
-      }
-
-      await update(15)
+      await updateMaxFPS(15)
 
       const attributes = { name: 'capped 15fps', fixture: '60fps_720p_small.mp4' }
       const { uuid } = await servers[1].videos.upload({ attributes })
 
       await waitJobs(servers)
       await testFPS(uuid, 15, 15)
+    })
 
-      await update(60)
+    it('Should not duplicate resolution on re-transcoding', async function () {
+      this.timeout(120_000)
+
+      await updateMaxFPS(50)
+
+      const attributes = { name: 'capped 50fps', fixture: '60fps_720p_small.mp4' }
+      const { uuid } = await servers[1].videos.upload({ attributes })
+
+      await waitJobs(servers)
+      await testFPS(uuid, 50, 25)
+
+      await servers[1].videos.runTranscoding({ transcodingType: 'web-video', videoId: uuid })
+      await waitJobs(servers)
+
+      const video = await servers[1].videos.get({ id: uuid })
+      expect(video.files.map(f => f.resolution.id)).to.deep.equal([ 720, 480, 360, 240, 144 ])
+
+      await testFPS(uuid, 50, 25)
+    })
+
+    after(async function () {
+      await updateMaxFPS(60)
     })
   })
 
diff --git a/server/core/lib/live/live-utils.ts b/server/core/lib/live/live-utils.ts
index 2760a55c2..fe32e4b52 100644
--- a/server/core/lib/live/live-utils.ts
+++ b/server/core/lib/live/live-utils.ts
@@ -3,7 +3,7 @@ import { FileStorage, LiveVideoLatencyMode, LiveVideoLatencyModeType, VideoState
 import { logger } from '@server/helpers/logger.js'
 import { VIDEO_LIVE } from '@server/initializers/constants.js'
 import { MStreamingPlaylist, MStreamingPlaylistVideo, MVideo } from '@server/types/models/index.js'
-import { pathExists, remove, } from 'fs-extra/esm'
+import { pathExists, remove } from 'fs-extra/esm'
 import { readdir, rmdir } from 'fs/promises'
 import { basename, join } from 'path'
 import { listHLSFileKeysOf, removeHLSFileObjectStorageByFullKey, removeHLSObjectStorage } from '../object-storage/index.js'
diff --git a/server/core/lib/transcoding/shared/job-builders/abstract-job-builder.ts b/server/core/lib/transcoding/shared/job-builders/abstract-job-builder.ts
index 062bd6de0..06875df5c 100644
--- a/server/core/lib/transcoding/shared/job-builders/abstract-job-builder.ts
+++ b/server/core/lib/transcoding/shared/job-builders/abstract-job-builder.ts
@@ -38,8 +38,6 @@ export abstract class AbstractJobBuilder <P> {
         const probe = await ffprobePromise(videoFilePath)
         const quickTranscode = await canDoQuickTranscode(videoFilePath, CONFIG.TRANSCODING.FPS.MAX, probe)
 
-        let inputFPS: number
-
         let maxFPS: number
         let maxResolution: number
 
@@ -47,7 +45,7 @@ export abstract class AbstractJobBuilder <P> {
 
         if (videoFile.isAudio()) {
           // The first transcoding job will transcode to this FPS value
-          inputFPS = maxFPS = Math.min(DEFAULT_AUDIO_MERGE_RESOLUTION, CONFIG.TRANSCODING.FPS.MAX)
+          maxFPS = Math.min(DEFAULT_AUDIO_MERGE_RESOLUTION, CONFIG.TRANSCODING.FPS.MAX)
           maxResolution = DEFAULT_AUDIO_RESOLUTION
 
           mergeOrOptimizePayload = this.buildMergeAudioPayload({
@@ -58,7 +56,7 @@ export abstract class AbstractJobBuilder <P> {
             fps: maxFPS
           })
         } else {
-          inputFPS = videoFile.fps
+          const inputFPS = videoFile.fps
           maxResolution = buildOriginalFileResolution(videoFile.resolution)
           maxFPS = computeOutputFPS({ inputFPS, resolution: maxResolution, isOriginResolution: true, type: 'vod' })
 
@@ -116,7 +114,7 @@ export abstract class AbstractJobBuilder <P> {
         const lowerResolutionJobPayloads = await this.buildLowerResolutionJobPayloads({
           video,
           inputVideoResolution: maxResolution,
-          inputVideoFPS: inputFPS,
+          inputVideoFPS: maxFPS,
           hasAudio: videoFile.hasAudio(),
           isNewVideo,
           hlsAudioAlreadyGenerated