From 9f46ba8eb49d57b52ea940de6d9254bc2d8dea1b Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Tue, 18 Mar 2025 12:42:09 -0400 Subject: [PATCH] fix(server): set pixel format when scaling and not tonemapping (#16932) set pixel format when scaling and not tonemapping --- server/src/services/media.service.spec.ts | 26 ++++++++++---- server/src/utils/media.ts | 41 +++++++++-------------- server/test/fixtures/media.stub.ts | 16 +++++++++ 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/server/src/services/media.service.spec.ts b/server/src/services/media.service.spec.ts index d98cff866f..9b8cc820cc 100644 --- a/server/src/services/media.service.spec.ts +++ b/server/src/services/media.service.spec.ts @@ -1608,7 +1608,7 @@ describe(MediaService.name, () => { 'upload/encoded-video/user-id/as/se/asset-id.mp4', expect.objectContaining({ inputOptions: expect.arrayContaining(['-hwaccel cuda', '-hwaccel_output_format cuda']), - outputOptions: expect.arrayContaining([expect.stringContaining('format=nv12')]), + outputOptions: expect.arrayContaining([expect.stringContaining('scale_cuda=-2:720:format=nv12')]), twoPass: false, }), ); @@ -1766,9 +1766,7 @@ describe(MediaService.name, () => { '-threads 1', '-qsv_device /dev/dri/renderD128', ]), - outputOptions: expect.arrayContaining([ - expect.stringContaining('scale_qsv=-1:720:async_depth=4:mode=hq:format=nv12'), - ]), + outputOptions: expect.arrayContaining([expect.stringContaining('scale_qsv=-1:720:async_depth=4:mode=hq')]), twoPass: false, }), ); @@ -2010,9 +2008,7 @@ describe(MediaService.name, () => { '-threads 1', '-hwaccel_device /dev/dri/renderD128', ]), - outputOptions: expect.arrayContaining([ - expect.stringContaining('scale_vaapi=-2:720:mode=hq:out_range=pc:format=nv12'), - ]), + outputOptions: expect.arrayContaining([expect.stringContaining('scale_vaapi=-2:720:mode=hq:out_range=pc')]), twoPass: false, }), ); @@ -2368,6 +2364,22 @@ describe(MediaService.name, () => { ); }); + it('should convert to yuv420p when scaling without tone-mapping', async () => { + mocks.media.probe.mockResolvedValue(probeStub.videoStream4K10Bit); + mocks.systemMetadata.get.mockResolvedValue({ ffmpeg: { transcode: TranscodePolicy.REQUIRED } }); + mocks.asset.getByIds.mockResolvedValue([assetStub.video]); + await sut.handleVideoConversion({ id: assetStub.video.id }); + expect(mocks.media.transcode).toHaveBeenCalledWith( + '/original/path.ext', + 'upload/encoded-video/user-id/as/se/asset-id.mp4', + expect.objectContaining({ + inputOptions: expect.any(Array), + outputOptions: expect.arrayContaining(['-c:v h264', '-c:a copy', '-vf scale=-2:720,format=yuv420p']), + twoPass: false, + }), + ); + }); + it('should count frames for progress when log level is debug', async () => { mocks.media.probe.mockResolvedValue(probeStub.matroskaContainer); mocks.logger.isLevelEnabled.mockReturnValue(true); diff --git a/server/src/utils/media.ts b/server/src/utils/media.ts index 62cf6500fb..d26f8d0fb6 100644 --- a/server/src/utils/media.ts +++ b/server/src/utils/media.ts @@ -159,9 +159,11 @@ export class BaseConfig implements VideoCodecSWConfig { options.push(`scale=${this.getScaling(videoStream)}`); } - options.push(...this.getToneMapping(videoStream)); - if (options.length === 0 && !videoStream.pixelFormat.endsWith('420p')) { - options.push(`format=yuv420p`); + const tonemapOptions = this.getToneMapping(videoStream); + if (tonemapOptions.length > 0) { + options.push(...tonemapOptions); + } else if (!videoStream.pixelFormat.endsWith('420p')) { + options.push('format=yuv420p'); } return options; @@ -606,14 +608,13 @@ export class NvencHwDecodeConfig extends NvencSwDecodeConfig { getFilterOptions(videoStream: VideoStreamInfo) { const options = []; - if (this.shouldScale(videoStream)) { + const tonemapOptions = this.getToneMapping(videoStream); + if (this.shouldScale(videoStream) || (tonemapOptions.length === 0 && !videoStream.pixelFormat.endsWith('420p'))) { options.push(`scale_cuda=${this.getScaling(videoStream)}`); } - options.push(...this.getToneMapping(videoStream)); + options.push(...tonemapOptions); if (options.length > 0) { options[options.length - 1] += ':format=nv12'; - } else if (!videoStream.pixelFormat.endsWith('420p')) { - options.push('format=nv12'); } return options; } @@ -732,17 +733,12 @@ export class QsvHwDecodeConfig extends QsvSwDecodeConfig { getFilterOptions(videoStream: VideoStreamInfo) { const options = []; const tonemapOptions = this.getToneMapping(videoStream); - if (this.shouldScale(videoStream) || tonemapOptions.length === 0) { - let scaling = `scale_qsv=${this.getScaling(videoStream)}:async_depth=4:mode=hq`; - if (tonemapOptions.length === 0) { - scaling += ':format=nv12'; - } - options.push(scaling); + if (tonemapOptions.length === 0 && !videoStream.pixelFormat.endsWith('420p')) { + options.push(`scale_qsv=${this.getScaling(videoStream)}:async_depth=4:mode=hq:format=nv12`); + } else if (this.shouldScale(videoStream)) { + options.push(`scale_qsv=${this.getScaling(videoStream)}:async_depth=4:mode=hq`); } options.push(...tonemapOptions); - if (options.length === 0 && !videoStream.pixelFormat.endsWith('420p')) { - options.push('format=nv12'); - } return options; } @@ -848,17 +844,12 @@ export class VaapiHwDecodeConfig extends VaapiSwDecodeConfig { getFilterOptions(videoStream: VideoStreamInfo) { const options = []; const tonemapOptions = this.getToneMapping(videoStream); - if (this.shouldScale(videoStream) || tonemapOptions.length === 0) { - let scaling = `scale_vaapi=${this.getScaling(videoStream)}:mode=hq:out_range=pc`; - if (tonemapOptions.length === 0) { - scaling += ':format=nv12'; - } - options.push(scaling); + if (tonemapOptions.length === 0 && !videoStream.pixelFormat.endsWith('420p')) { + options.push(`scale_vaapi=${this.getScaling(videoStream)}:mode=hq:out_range=pc:format=nv12`); + } else if (this.shouldScale(videoStream)) { + options.push(`scale_vaapi=${this.getScaling(videoStream)}:mode=hq:out_range=pc`); } options.push(...tonemapOptions); - if (options.length === 0 && !videoStream.pixelFormat.endsWith('420p')) { - options.push('format=nv12'); - } return options; } diff --git a/server/test/fixtures/media.stub.ts b/server/test/fixtures/media.stub.ts index 021f899ae5..e1579435f5 100644 --- a/server/test/fixtures/media.stub.ts +++ b/server/test/fixtures/media.stub.ts @@ -134,6 +134,22 @@ export const probeStub = { }, ], }), + videoStream4K10Bit: Object.freeze({ + ...probeStubDefault, + videoStreams: [ + { + index: 0, + height: 2160, + width: 3840, + codecName: 'h264', + frameCount: 100, + rotation: 0, + isHDR: false, + bitrate: 0, + pixelFormat: 'yuv420p10le', + }, + ], + }), videoStreamVertical2160p: Object.freeze({ ...probeStubDefault, videoStreams: [