Handle higher FPS for high resolution (test)

10 files changed, 99 insertions, 24 deletions

diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts
index b4ced8c1e..8c93ae89c 100644
--- a/server/controllers/api/videos/index.ts
+++ b/server/controllers/api/videos/index.ts
@@ -2,7 +2,7 @@ import * as express from 'express'
 import { extname, join } from 'path'
 import { VideoCreate, VideoPrivacy, VideoState, VideoUpdate } from '../../../../shared'
 import { renamePromise } from '../../../helpers/core-utils'
-import { getVideoFileResolution } from '../../../helpers/ffmpeg-utils'
+import { getVideoFileFPS, getVideoFileResolution } from '../../../helpers/ffmpeg-utils'
 import { processImage } from '../../../helpers/image-utils'
 import { logger } from '../../../helpers/logger'
 import { getFormattedObjects, getServerActor, resetSequelizeInstance } from '../../../helpers/utils'
@@ -184,10 +184,13 @@ async function addVideo (req: express.Request, res: express.Response) {
 
   // Build the file object
   const { videoFileResolution } = await getVideoFileResolution(videoPhysicalFile.path)
+  const fps = await getVideoFileFPS(videoPhysicalFile.path)
+
   const videoFileData = {
     extname: extname(videoPhysicalFile.filename),
     resolution: videoFileResolution,
-    size: videoPhysicalFile.size
+    size: videoPhysicalFile.size,
+    fps
   }
   const videoFile = new VideoFileModel(videoFileData)
 
diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts
index ae392f8c2..672f06dc0 100644
--- a/server/helpers/custom-validators/videos.ts
+++ b/server/helpers/custom-validators/videos.ts
@@ -118,6 +118,10 @@ function isVideoFileResolutionValid (value: string) {
   return exists(value) && validator.isInt(value + '')
 }
 
+function isVideoFPSResolutionValid (value: string) {
+  return value === null || validator.isInt(value + '')
+}
+
 function isVideoFileSizeValid (value: string) {
   return exists(value) && validator.isInt(value + '', VIDEOS_CONSTRAINTS_FIELDS.FILE_SIZE)
 }
@@ -182,6 +186,7 @@ export {
   isVideoFileInfoHashValid,
   isVideoNameValid,
   isVideoTagsValid,
+  isVideoFPSResolutionValid,
   isScheduleVideoUpdatePrivacyValid,
   isVideoAbuseReasonValid,
   isVideoFile,
diff --git a/server/helpers/ffmpeg-utils.ts b/server/helpers/ffmpeg-utils.ts
index bfc942fa3..4086335d7 100644
--- a/server/helpers/ffmpeg-utils.ts
+++ b/server/helpers/ffmpeg-utils.ts
@@ -26,7 +26,7 @@ async function getVideoFileFPS (path: string) {
     if (!frames || !seconds) continue
 
     const result = parseInt(frames, 10) / parseInt(seconds, 10)
-    if (result > 0) return result
+    if (result > 0) return Math.round(result)
   }
 
   return 0
@@ -83,8 +83,6 @@ type TranscodeOptions = {
 
 function transcode (options: TranscodeOptions) {
   return new Promise<void>(async (res, rej) => {
-    const fps = await getVideoFileFPS(options.inputPath)
-
     let command = ffmpeg(options.inputPath)
                     .output(options.outputPath)
                     .videoCodec('libx264')
@@ -92,14 +90,27 @@ function transcode (options: TranscodeOptions) {
                     .outputOption('-movflags faststart')
                     // .outputOption('-crf 18')
 
-    // Our player has some FPS limits
-    if (fps > VIDEO_TRANSCODING_FPS.MAX) command = command.withFPS(VIDEO_TRANSCODING_FPS.MAX)
-    else if (fps < VIDEO_TRANSCODING_FPS.MIN) command = command.withFPS(VIDEO_TRANSCODING_FPS.MIN)
-
+    let fps = await getVideoFileFPS(options.inputPath)
     if (options.resolution !== undefined) {
       // '?x720' or '720x?' for example
       const size = options.isPortraitMode === true ? `${options.resolution}x?` : `?x${options.resolution}`
       command = command.size(size)
+
+      // On small/medium resolutions, limit FPS
+      if (
+        options.resolution < VIDEO_TRANSCODING_FPS.KEEP_ORIGIN_FPS_RESOLUTION_MIN &&
+        fps > VIDEO_TRANSCODING_FPS.AVERAGE
+      ) {
+        fps = VIDEO_TRANSCODING_FPS.AVERAGE
+      }
+    }
+
+    if (fps) {
+      // Hard FPS limits
+      if (fps > VIDEO_TRANSCODING_FPS.MAX) fps = VIDEO_TRANSCODING_FPS.MAX
+      else if (fps < VIDEO_TRANSCODING_FPS.MIN) fps = VIDEO_TRANSCODING_FPS.MIN
+
+      command = command.withFPS(fps)
     }
 
     command
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 67df3df80..24b7e2655 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -14,7 +14,7 @@ let config: IConfig = require('config')
 
 // ---------------------------------------------------------------------------
 
-const LAST_MIGRATION_VERSION = 220
+const LAST_MIGRATION_VERSION = 225
 
 // ---------------------------------------------------------------------------
 
@@ -282,7 +282,9 @@ const RATES_LIMIT = {
 let VIDEO_VIEW_LIFETIME = 60000 * 60 // 1 hour
 const VIDEO_TRANSCODING_FPS = {
   MIN: 10,
-  MAX: 30
+  AVERAGE: 30,
+  MAX: 60,
+  KEEP_ORIGIN_FPS_RESOLUTION_MIN: 720 // We keep the original FPS on high resolutions (720 minimum)
 }
 
 const VIDEO_RATE_TYPES: { [ id: string ]: VideoRateType } = {
diff --git a/server/initializers/migrations/0225-video-fps.ts b/server/initializers/migrations/0225-video-fps.ts
new file mode 100644
index 000000000..733676845
--- /dev/null
+++ b/server/initializers/migrations/0225-video-fps.ts
@@ -0,0 +1,22 @@
+import * as Sequelize from 'sequelize'
+
+async function up (utils: {
+  transaction: Sequelize.Transaction
+  queryInterface: Sequelize.QueryInterface
+  sequelize: Sequelize.Sequelize
+}): Promise<void> {
+  {
+    const data = {
+      type: Sequelize.INTEGER,
+      allowNull: true,
+      defaultValue: null
+    }
+    await utils.queryInterface.addColumn('videoFile', 'fps', data)
+  }
+}
+
+function down (options) {
+  throw new Error('Not implemented.')
+}
+
+export { up, down }
diff --git a/server/models/video/video-file.ts b/server/models/video/video-file.ts
index df4067a4e..372d18d69 100644
--- a/server/models/video/video-file.ts
+++ b/server/models/video/video-file.ts
@@ -1,6 +1,11 @@
 import { values } from 'lodash'
-import { AllowNull, BelongsTo, Column, CreatedAt, DataType, ForeignKey, Is, Model, Table, UpdatedAt } from 'sequelize-typescript'
-import { isVideoFileInfoHashValid, isVideoFileResolutionValid, isVideoFileSizeValid } from '../../helpers/custom-validators/videos'
+import { AllowNull, BelongsTo, Column, CreatedAt, DataType, Default, ForeignKey, Is, Model, Table, UpdatedAt } from 'sequelize-typescript'
+import {
+  isVideoFileInfoHashValid,
+  isVideoFileResolutionValid,
+  isVideoFileSizeValid,
+  isVideoFPSResolutionValid
+} from '../../helpers/custom-validators/videos'
 import { CONSTRAINTS_FIELDS } from '../../initializers'
 import { throwIfNotValid } from '../utils'
 import { VideoModel } from './video'
@@ -42,6 +47,12 @@ export class VideoFileModel extends Model<VideoFileModel> {
   @Column
   infoHash: string
 
+  @AllowNull(true)
+  @Default(null)
+  @Is('VideoFileFPS', value => throwIfNotValid(value, isVideoFPSResolutionValid, 'fps'))
+  @Column
+  fps: number
+
   @ForeignKey(() => VideoModel)
   @Column
   videoId: number
diff --git a/server/models/video/video.ts b/server/models/video/video.ts
index 5d8089328..ab33b7c99 100644
--- a/server/models/video/video.ts
+++ b/server/models/video/video.ts
@@ -52,7 +52,7 @@ import {
   isVideoStateValid,
   isVideoSupportValid
 } from '../../helpers/custom-validators/videos'
-import { generateImageFromVideoFile, getVideoFileResolution, transcode } from '../../helpers/ffmpeg-utils'
+import { generateImageFromVideoFile, getVideoFileFPS, getVideoFileResolution, transcode } from '../../helpers/ffmpeg-utils'
 import { logger } from '../../helpers/logger'
 import { getServerActor } from '../../helpers/utils'
 import {
@@ -1168,6 +1168,7 @@ export class VideoModel extends Model<VideoModel> {
             },
             magnetUri: this.generateMagnetUri(videoFile, baseUrlHttp, baseUrlWs),
             size: videoFile.size,
+            fps: videoFile.fps,
             torrentUrl: this.getTorrentUrl(videoFile, baseUrlHttp),
             torrentDownloadUrl: this.getTorrentDownloadUrl(videoFile, baseUrlHttp),
             fileUrl: this.getVideoFileUrl(videoFile, baseUrlHttp),
@@ -1303,11 +1304,11 @@ export class VideoModel extends Model<VideoModel> {
     const newExtname = '.mp4'
     const inputVideoFile = this.getOriginalFile()
     const videoInputPath = join(videosDirectory, this.getVideoFilename(inputVideoFile))
-    const videoOutputPath = join(videosDirectory, this.id + '-transcoded' + newExtname)
+    const videoTranscodedPath = join(videosDirectory, this.id + '-transcoded' + newExtname)
 
     const transcodeOptions = {
       inputPath: videoInputPath,
-      outputPath: videoOutputPath
+      outputPath: videoTranscodedPath
     }
 
     // Could be very long!
@@ -1319,10 +1320,13 @@ export class VideoModel extends Model<VideoModel> {
       // Important to do this before getVideoFilename() to take in account the new file extension
       inputVideoFile.set('extname', newExtname)
 
-      await renamePromise(videoOutputPath, this.getVideoFilePath(inputVideoFile))
-      const stats = await statPromise(this.getVideoFilePath(inputVideoFile))
+      const videoOutputPath = this.getVideoFilePath(inputVideoFile)
+      await renamePromise(videoTranscodedPath, videoOutputPath)
+      const stats = await statPromise(videoOutputPath)
+      const fps = await getVideoFileFPS(videoOutputPath)
 
       inputVideoFile.set('size', stats.size)
+      inputVideoFile.set('fps', fps)
 
       await this.createTorrentAndSetInfoHash(inputVideoFile)
       await inputVideoFile.save()
@@ -1360,8 +1364,10 @@ export class VideoModel extends Model<VideoModel> {
     await transcode(transcodeOptions)
 
     const stats = await statPromise(videoOutputPath)
+    const fps = await getVideoFileFPS(videoOutputPath)
 
     newVideoFile.set('size', stats.size)
+    newVideoFile.set('fps', fps)
 
     await this.createTorrentAndSetInfoHash(newVideoFile)
 
@@ -1371,10 +1377,15 @@ export class VideoModel extends Model<VideoModel> {
   }
 
   async importVideoFile (inputFilePath: string) {
+    const { videoFileResolution } = await getVideoFileResolution(inputFilePath)
+    const { size } = await statPromise(inputFilePath)
+    const fps = await getVideoFileFPS(inputFilePath)
+
     let updatedVideoFile = new VideoFileModel({
-      resolution: (await getVideoFileResolution(inputFilePath)).videoFileResolution,
+      resolution: videoFileResolution,
       extname: extname(inputFilePath),
-      size: (await statPromise(inputFilePath)).size,
+      size,
+      fps,
       videoId: this.id
     })
 
@@ -1390,6 +1401,7 @@ export class VideoModel extends Model<VideoModel> {
       // Update the database
       currentVideoFile.set('extname', updatedVideoFile.extname)
       currentVideoFile.set('size', updatedVideoFile.size)
+      currentVideoFile.set('fps', updatedVideoFile.fps)
 
       updatedVideoFile = currentVideoFile
     }
diff --git a/server/tests/api/videos/video-transcoder.ts b/server/tests/api/videos/video-transcoder.ts
index 2b203c26b..fe750253e 100644
--- a/server/tests/api/videos/video-transcoder.ts
+++ b/server/tests/api/videos/video-transcoder.ts
@@ -91,13 +91,13 @@ describe('Test video transcoding', function () {
     expect(torrent.files[0].path).match(/\.mp4$/)
   })
 
-  it('Should transcode to 30 FPS', async function () {
+  it('Should transcode a 60 FPS video', async function () {
     this.timeout(60000)
 
     const videoAttributes = {
       name: 'my super 30fps name for server 2',
       description: 'my super 30fps description for server 2',
-      fixture: 'video_60fps_short.mp4'
+      fixture: '60fps_720p_small.mp4'
     }
     await uploadVideo(servers[1].url, servers[1].accessToken, videoAttributes)
 
@@ -109,14 +109,23 @@ describe('Test video transcoding', function () {
     const res2 = await getVideo(servers[1].url, video.id)
     const videoDetails: VideoDetails = res2.body
 
-    expect(videoDetails.files).to.have.lengthOf(1)
+    expect(videoDetails.files).to.have.lengthOf(4)
+    expect(videoDetails.files[0].fps).to.be.above(58).and.below(62)
+    expect(videoDetails.files[1].fps).to.be.below(31)
+    expect(videoDetails.files[2].fps).to.be.below(31)
+    expect(videoDetails.files[3].fps).to.be.below(31)
 
-    for (const resolution of [ '240' ]) {
+    for (const resolution of [ '240', '360', '480' ]) {
       const path = join(root(), 'test2', 'videos', video.uuid + '-' + resolution + '.mp4')
       const fps = await getVideoFileFPS(path)
 
       expect(fps).to.be.below(31)
     }
+
+    const path = join(root(), 'test2', 'videos', video.uuid + '-720.mp4')
+    const fps = await getVideoFileFPS(path)
+
+    expect(fps).to.be.above(58).and.below(62)
   })
 
   it('Should wait transcoding before publishing the video', async function () {
diff --git a/server/tests/fixtures/60fps_720p_small.mp4 b/server/tests/fixtures/60fps_720p_small.mp4
new file mode 100644
index 000000000..74bf968a4
--- /dev/null
+++ b/server/tests/fixtures/60fps_720p_small.mp4
diff --git a/server/tests/fixtures/video_60fps_short.mp4 b/server/tests/fixtures/video_60fps_short.mp4
deleted file mode 100644
index ff0593cf3..000000000
--- a/server/tests/fixtures/video_60fps_short.mp4
+++ /dev/null