From 7b81edc854902a536083298472bf92bb6726edcf Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 10 Mar 2020 14:49:02 +0100 Subject: [PATCH] Video file metadata PR cleanup --- .../custom-validators/activitypub/videos.ts | 14 +++- server/helpers/ffmpeg-utils.ts | 2 +- server/lib/activitypub/videos.ts | 26 +++--- server/lib/video-transcoding.ts | 3 - server/models/video/video-file.ts | 79 +++++++++---------- server/models/video/video-format-utils.ts | 4 +- server/models/video/video.ts | 3 +- server/tests/api/videos/video-transcoder.ts | 72 ++++++++--------- 8 files changed, 103 insertions(+), 100 deletions(-) diff --git a/server/helpers/custom-validators/activitypub/videos.ts b/server/helpers/custom-validators/activitypub/videos.ts index af8c8a0c8..876cc7f50 100644 --- a/server/helpers/custom-validators/activitypub/videos.ts +++ b/server/helpers/custom-validators/activitypub/videos.ts @@ -13,6 +13,7 @@ import { import { isActivityPubUrlValid, isBaseActivityValid, setValidAttributedTo } from './misc' import { VideoState } from '../../../../shared/models/videos' import { logger } from '@server/helpers/logger' +import { ActivityVideoFileMetadataObject } from '@shared/models' function sanitizeAndCheckVideoTorrentUpdateActivity (activity: any) { return isBaseActivityValid(activity, 'Update') && @@ -104,7 +105,15 @@ function isRemoteVideoUrlValid (url: any) { (url.mediaType || url.mimeType) === 'application/x-mpegURL' && isActivityPubUrlValid(url.href) && isArray(url.tag) - ) + ) || + isAPVideoFileMetadataObject(url) +} + +function isAPVideoFileMetadataObject (url: any): url is ActivityVideoFileMetadataObject { + return url && + url.type === 'Link' && + url.mediaType === 'application/json' && + isArray(url.rel) && url.rel.includes('metadata') } // --------------------------------------------------------------------------- @@ -113,7 +122,8 @@ export { sanitizeAndCheckVideoTorrentUpdateActivity, isRemoteStringIdentifierValid, sanitizeAndCheckVideoTorrentObject, - isRemoteVideoUrlValid + isRemoteVideoUrlValid, + isAPVideoFileMetadataObject } // --------------------------------------------------------------------------- diff --git a/server/helpers/ffmpeg-utils.ts b/server/helpers/ffmpeg-utils.ts index 5ee295635..e0e408ea0 100644 --- a/server/helpers/ffmpeg-utils.ts +++ b/server/helpers/ffmpeg-utils.ts @@ -170,7 +170,7 @@ async function getVideoFileFPS (path: string) { return 0 } -async function getMetadataFromFile (path: string, cb = metadata => metadata) { +async function getMetadataFromFile (path: string, cb = metadata => metadata) { return new Promise((res, rej) => { ffmpeg.ffprobe(path, (err, metadata) => { if (err) return rej(err) diff --git a/server/lib/activitypub/videos.ts b/server/lib/activitypub/videos.ts index 30de4714c..452e43c8c 100644 --- a/server/lib/activitypub/videos.ts +++ b/server/lib/activitypub/videos.ts @@ -9,13 +9,13 @@ import { ActivityPlaylistUrlObject, ActivityTagObject, ActivityUrlObject, + ActivityVideoFileMetadataObject, ActivityVideoUrlObject, - VideoState, - ActivityVideoFileMetadataObject + VideoState } from '../../../shared/index' import { VideoTorrentObject } from '../../../shared/models/activitypub/objects' import { VideoPrivacy } from '../../../shared/models/videos' -import { sanitizeAndCheckVideoTorrentObject } from '../../helpers/custom-validators/activitypub/videos' +import { sanitizeAndCheckVideoTorrentObject, isAPVideoFileMetadataObject } from '../../helpers/custom-validators/activitypub/videos' import { isVideoFileInfoHashValid } from '../../helpers/custom-validators/videos' import { deleteNonExistingModels, resetSequelizeInstance, retryTransactionWrapper } from '../../helpers/database-utils' import { logger } from '../../helpers/logger' @@ -26,7 +26,8 @@ import { P2P_MEDIA_LOADER_PEER_VERSION, PREVIEWS_SIZE, REMOTE_SCHEME, - STATIC_PATHS, THUMBNAILS_SIZE + STATIC_PATHS, + THUMBNAILS_SIZE } from '../../initializers/constants' import { TagModel } from '../../models/video/tag' import { VideoModel } from '../../models/video/video' @@ -69,7 +70,8 @@ import { MVideoAPWithoutCaption, MVideoFile, MVideoFullLight, - MVideoId, MVideoImmutable, + MVideoId, + MVideoImmutable, MVideoThumbnail } from '../../typings/models' import { MThumbnail } from '../../typings/models/video/thumbnail' @@ -527,10 +529,6 @@ function isAPHashTagObject (url: any): url is ActivityHashTagObject { return url && url.type === 'Hashtag' } -function isAPVideoFileMetadataObject (url: any): url is ActivityVideoFileMetadataObject { - return url && url.type === 'Link' && url.mediaType === 'application/json' && url.hasAttribute('rel') && url.rel.includes('metadata') -} - async function createVideo (videoObject: VideoTorrentObject, channel: MChannelAccountLight, waitThumbnail = false) { logger.debug('Adding remote video %s.', videoObject.id) @@ -701,11 +699,11 @@ function videoFileActivityUrlToDBAttributes ( // Fetch associated metadata url, if any const metadata = urls.filter(isAPVideoFileMetadataObject) - .find(u => - u.height === fileUrl.height && - u.fps === fileUrl.fps && - u.rel.includes(fileUrl.mediaType) - ) + .find(u => { + return u.height === fileUrl.height && + u.fps === fileUrl.fps && + u.rel.includes(fileUrl.mediaType) + }) const mediaType = fileUrl.mediaType const attribute = { diff --git a/server/lib/video-transcoding.ts b/server/lib/video-transcoding.ts index 444b0d954..bde4c2633 100644 --- a/server/lib/video-transcoding.ts +++ b/server/lib/video-transcoding.ts @@ -237,12 +237,9 @@ async function onVideoFileTranscoding (video: MVideoWithFile, videoFile: MVideoF await move(transcodingPath, outputPath) - const extractedVideo = extractVideo(video) - videoFile.size = stats.size videoFile.fps = fps videoFile.metadata = metadata - videoFile.metadataUrl = extractedVideo.getVideoFileMetadataUrl(videoFile, extractedVideo.getBaseUrls().baseUrlHttp) await createTorrentAndSetInfoHash(video, videoFile) diff --git a/server/models/video/video-file.ts b/server/models/video/video-file.ts index 029468004..201f0c0f1 100644 --- a/server/models/video/video-file.ts +++ b/server/models/video/video-file.ts @@ -30,18 +30,16 @@ import { MIMETYPES, MEMOIZE_LENGTH, MEMOIZE_TTL } from '../../initializers/const import { MVideoFile, MVideoFileStreamingPlaylistVideo, MVideoFileVideo } from '../../typings/models/video/video-file' import { MStreamingPlaylistVideo, MVideo } from '@server/typings/models' import * as memoizee from 'memoizee' +import validator from 'validator' export enum ScopeNames { WITH_VIDEO = 'WITH_VIDEO', - WITH_VIDEO_OR_PLAYLIST = 'WITH_VIDEO_OR_PLAYLIST', WITH_METADATA = 'WITH_METADATA' } -const METADATA_FIELDS = [ 'metadata', 'metadataUrl' ] - @DefaultScope(() => ({ attributes: { - exclude: [ METADATA_FIELDS[0] ] + exclude: [ 'metadata' ] } })) @Scopes(() => ({ @@ -53,35 +51,9 @@ const METADATA_FIELDS = [ 'metadata', 'metadataUrl' ] } ] }, - [ScopeNames.WITH_VIDEO_OR_PLAYLIST]: (videoIdOrUUID: string | number) => { - const where = (typeof videoIdOrUUID === 'number') - ? { id: videoIdOrUUID } - : { uuid: videoIdOrUUID } - - return { - include: [ - { - model: VideoModel.unscoped(), - required: false, - where - }, - { - model: VideoStreamingPlaylistModel.unscoped(), - required: false, - include: [ - { - model: VideoModel.unscoped(), - required: true, - where - } - ] - } - ] - } - }, [ScopeNames.WITH_METADATA]: { attributes: { - include: METADATA_FIELDS + include: [ 'metadata' ] } } })) @@ -223,10 +195,8 @@ export class VideoFileModel extends Model { static async doesVideoExistForVideoFile (id: number, videoIdOrUUID: number | string) { const videoFile = await VideoFileModel.loadWithVideoOrPlaylist(id, videoIdOrUUID) - return (videoFile?.Video.id === videoIdOrUUID) || - (videoFile?.Video.uuid === videoIdOrUUID) || - (videoFile?.VideoStreamingPlaylist?.Video?.id === videoIdOrUUID) || - (videoFile?.VideoStreamingPlaylist?.Video?.uuid === videoIdOrUUID) + + return !!videoFile } static loadWithMetadata (id: number) { @@ -238,12 +208,41 @@ export class VideoFileModel extends Model { } static loadWithVideoOrPlaylist (id: number, videoIdOrUUID: number | string) { - return VideoFileModel.scope({ - method: [ - ScopeNames.WITH_VIDEO_OR_PLAYLIST, - videoIdOrUUID + const whereVideo = validator.isUUID(videoIdOrUUID + '') + ? { uuid: videoIdOrUUID } + : { id: videoIdOrUUID } + + const options = { + where: { + id + }, + include: [ + { + model: VideoModel.unscoped(), + required: false, + where: whereVideo + }, + { + model: VideoStreamingPlaylistModel.unscoped(), + required: false, + include: [ + { + model: VideoModel.unscoped(), + required: true, + where: whereVideo + } + ] + } ] - }).findByPk(id) + } + + return VideoFileModel.findOne(options) + .then(file => { + // We used `required: false` so check we have at least a video or a streaming playlist + if (!file.Video && !file.VideoStreamingPlaylist) return null + + return file + }) } static listByStreamingPlaylist (streamingPlaylistId: number, transaction: Transaction) { diff --git a/server/models/video/video-format-utils.ts b/server/models/video/video-format-utils.ts index 21f0e0a68..365c9581e 100644 --- a/server/models/video/video-format-utils.ts +++ b/server/models/video/video-format-utils.ts @@ -181,6 +181,8 @@ function videoFilesModelToFormattedJSON ( baseUrlWs: string, videoFiles: MVideoFileRedundanciesOpt[] ): VideoFile[] { + const video = extractVideo(model) + return videoFiles .map(videoFile => { return { @@ -195,7 +197,7 @@ function videoFilesModelToFormattedJSON ( torrentDownloadUrl: model.getTorrentDownloadUrl(videoFile, baseUrlHttp), fileUrl: model.getVideoFileUrl(videoFile, baseUrlHttp), fileDownloadUrl: model.getVideoFileDownloadUrl(videoFile, baseUrlHttp), - metadataUrl: videoFile.metadataUrl // only send the metadataUrl and not the metadata over the wire + metadataUrl: video.getVideoFileMetadataUrl(videoFile, baseUrlHttp) } as VideoFile }) .sort((a, b) => { diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 5e4b7d44c..958a49e65 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1849,7 +1849,8 @@ export class VideoModel extends Model { getVideoFileMetadataUrl (videoFile: MVideoFile, baseUrlHttp: string) { const path = '/api/v1/videos/' - return videoFile.metadata + + return this.isOwned() ? baseUrlHttp + path + this.uuid + '/metadata/' + videoFile.id : videoFile.metadataUrl } diff --git a/server/tests/api/videos/video-transcoder.ts b/server/tests/api/videos/video-transcoder.ts index ce0dd14d5..13b3530b1 100644 --- a/server/tests/api/videos/video-transcoder.ts +++ b/server/tests/api/videos/video-transcoder.ts @@ -27,13 +27,14 @@ import { root, ServerInfo, setAccessTokensToServers, - uploadVideo, + uploadVideo, uploadVideoAndGetId, waitJobs, webtorrentAdd } from '../../../../shared/extra-utils' import { join } from 'path' import { VIDEO_TRANSCODING_FPS } from '../../../../server/initializers/constants' import { FfprobeData } from 'fluent-ffmpeg' +import { VideoFileMetadata } from '@shared/models/videos/video-file-metadata' const expect = chai.expect @@ -470,61 +471,56 @@ describe('Test video transcoding', function () { it('Should provide valid ffprobe data', async function () { this.timeout(160000) - const videoAttributes = { - name: 'my super name for server 1', - description: 'my super description for server 1', - fixture: 'video_short.webm' - } - await uploadVideo(servers[1].url, servers[1].accessToken, videoAttributes) - + const videoUUID = (await uploadVideoAndGetId({ server: servers[1], videoName: 'ffprobe data' })).uuid await waitJobs(servers) - const res = await getVideosList(servers[1].url) - - const videoOnOrigin = res.body.data.find(v => v.name === videoAttributes.name) - const res2 = await getVideo(servers[1].url, videoOnOrigin.id) - const videoOnOriginDetails: VideoDetails = res2.body - { - const path = join(root(), 'test' + servers[1].internalServerNumber, 'videos', videoOnOrigin.uuid + '-240.mp4') - const metadata = await getMetadataFromFile(path) + const path = join(root(), 'test' + servers[1].internalServerNumber, 'videos', videoUUID + '-240.mp4') + const metadata = await getMetadataFromFile(path) + + // expected format properties for (const p of [ - // expected format properties - 'format.encoder', - 'format.format_long_name', - 'format.size', - 'format.bit_rate', - // expected stream properties - 'stream[0].codec_long_name', - 'stream[0].profile', - 'stream[0].width', - 'stream[0].height', - 'stream[0].display_aspect_ratio', - 'stream[0].avg_frame_rate', - 'stream[0].pix_fmt' + 'tags.encoder', + 'format_long_name', + 'size', + 'bit_rate' ]) { - expect(metadata).to.have.nested.property(p) + expect(metadata.format).to.have.nested.property(p) } + + // expected stream properties + for (const p of [ + 'codec_long_name', + 'profile', + 'width', + 'height', + 'display_aspect_ratio', + 'avg_frame_rate', + 'pix_fmt' + ]) { + expect(metadata.streams[0]).to.have.nested.property(p) + } + expect(metadata).to.not.have.nested.property('format.filename') } for (const server of servers) { - const res = await getVideosList(server.url) - - const video = res.body.data.find(v => v.name === videoAttributes.name) - const res2 = await getVideo(server.url, video.id) - const videoDetails = res2.body + const res2 = await getVideo(server.url, videoUUID) + const videoDetails: VideoDetails = res2.body const videoFiles = videoDetails.files - for (const [ index, file ] of videoFiles.entries()) { + .concat(videoDetails.streamingPlaylists[0].files) + expect(videoFiles).to.have.lengthOf(8) + + for (const file of videoFiles) { expect(file.metadata).to.be.undefined + expect(file.metadataUrl).to.exist expect(file.metadataUrl).to.contain(servers[1].url) - expect(file.metadataUrl).to.contain(videoOnOrigin.uuid) + expect(file.metadataUrl).to.contain(videoUUID) const res3 = await getVideoFileMetadataUrl(file.metadataUrl) const metadata: FfprobeData = res3.body expect(metadata).to.have.nested.property('format.size') - expect(metadata.format.size).to.equal(videoOnOriginDetails.files[index].metadata.format.size) } } })