From 3a6f351b255d21ec42578632600ba699885f350e Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 29 Jun 2018 16:41:29 +0200 Subject: Handle higher FPS for high resolution (test) --- client/src/assets/player/resolution-menu-button.ts | 7 +++++- server/controllers/api/videos/index.ts | 7 ++++-- server/helpers/custom-validators/videos.ts | 5 ++++ server/helpers/ffmpeg-utils.ts | 25 ++++++++++++++------ server/initializers/constants.ts | 6 +++-- server/initializers/migrations/0225-video-fps.ts | 22 +++++++++++++++++ server/models/video/video-file.ts | 15 ++++++++++-- server/models/video/video.ts | 26 +++++++++++++++------ server/tests/api/videos/video-transcoder.ts | 17 ++++++++++---- server/tests/fixtures/60fps_720p_small.mp4 | Bin 0 -> 276786 bytes server/tests/fixtures/video_60fps_short.mp4 | Bin 33968 -> 0 bytes shared/models/videos/video.model.ts | 1 + 12 files changed, 106 insertions(+), 25 deletions(-) create mode 100644 server/initializers/migrations/0225-video-fps.ts create mode 100644 server/tests/fixtures/60fps_720p_small.mp4 delete mode 100644 server/tests/fixtures/video_60fps_short.mp4 diff --git a/client/src/assets/player/resolution-menu-button.ts b/client/src/assets/player/resolution-menu-button.ts index e30074173..d53a24151 100644 --- a/client/src/assets/player/resolution-menu-button.ts +++ b/client/src/assets/player/resolution-menu-button.ts @@ -35,11 +35,16 @@ class ResolutionMenuButton extends MenuButton { createMenu () { const menu = new Menu(this.player_) for (const videoFile of this.player_.peertube().videoFiles) { + let label = videoFile.resolution.label + if (videoFile.fps && videoFile.fps >= 50) { + label += videoFile.fps + } + menu.addChild(new ResolutionMenuItem( this.player_, { id: videoFile.resolution.id, - label: videoFile.resolution.label, + label, src: videoFile.magnetUri }) ) 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(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 { + { + 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 { @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 { }, 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 { 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 { // 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 { 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 { } 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 { // 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 Binary files /dev/null and b/server/tests/fixtures/60fps_720p_small.mp4 differ 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 Binary files a/server/tests/fixtures/video_60fps_short.mp4 and /dev/null differ diff --git a/shared/models/videos/video.model.ts b/shared/models/videos/video.model.ts index f88f381cb..4e1f15ee3 100644 --- a/shared/models/videos/video.model.ts +++ b/shared/models/videos/video.model.ts @@ -18,6 +18,7 @@ export interface VideoFile { torrentDownloadUrl: string fileUrl: string fileDownloadUrl: string + fps: number } export interface Video { -- cgit v1.2.3