From 2f754d2a038768472addd03361eff628f0268dfe Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sun, 5 Feb 2023 17:33:24 +0800 Subject: [PATCH] sanity check file sizes after merge files #1453 also notify about export confirm dialog --- src/App.jsx | 30 +++++++++++++++++++------- src/dialogs/index.jsx | 9 +++++--- src/hooks/useFfmpegOperations.js | 8 ++++--- src/util.js | 36 +++++++++++++++++++++++--------- src/util/streams.js | 19 +++++++++++------ src/util/streams.test.js | 3 ++- 6 files changed, 75 insertions(+), 30 deletions(-) diff --git a/src/App.jsx b/src/App.jsx index 473a2dfd..74dae65d 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -14,6 +14,7 @@ import fromPairs from 'lodash/fromPairs'; import sortBy from 'lodash/sortBy'; import flatMap from 'lodash/flatMap'; import isEqual from 'lodash/isEqual'; +import sum from 'lodash/sum'; import theme from './theme'; import useTimelineScroll from './hooks/useTimelineScroll'; @@ -65,7 +66,7 @@ import { checkDirWriteAccess, dirExists, isMasBuild, isStoreBuild, dragPreventer, filenamify, getOutFileExtension, generateSegFileName, defaultOutSegTemplate, havePermissionToReadFile, resolvePathIfNeeded, getPathReadAccessError, html5ifiedPrefix, html5dummySuffix, findExistingHtml5FriendlyFile, - deleteFiles, isOutOfSpaceError, shuffleArray, getNumDigits, isExecaFailure, + deleteFiles, isOutOfSpaceError, shuffleArray, getNumDigits, isExecaFailure, readFileSize, readFileSizes, checkFileSizes, } from './util'; import { formatDuration } from './util/duration'; import { adjustRate } from './util/rate-calculator'; @@ -257,7 +258,11 @@ const App = memo(() => { } }, [timelineMode]); - const toggleExportConfirmEnabled = useCallback(() => setExportConfirmEnabled((v) => !v), [setExportConfirmEnabled]); + const toggleExportConfirmEnabled = useCallback(() => setExportConfirmEnabled((v) => { + const newVal = !v; + toast.fire({ text: newVal ? i18n.t('Export options will be shown before exporting.') : i18n.t('Export options will not be shown before exporting.') }); + return newVal; + }), [setExportConfirmEnabled]); const toggleSegmentsToChapters = useCallback(() => setSegmentsToChapters((v) => !v), [setSegmentsToChapters]); @@ -1171,13 +1176,22 @@ const App = memo(() => { chaptersFromSegments = await createChaptersFromSegments({ segmentPaths: paths, chapterNames }); } + const inputSize = sum(await readFileSizes(paths)); + // console.log('merge', paths); const metadataFromPath = paths[0]; - await concatFiles({ paths, outPath, outDir, outFormat, metadataFromPath, includeAllStreams, streams, ffmpegExperimental, onProgress: setCutProgress, preserveMovData, movFastStart, preserveMetadataOnMerge, chapters: chaptersFromSegments, appendFfmpegCommandLog }); - if (clearBatchFilesAfterConcat) closeBatch(); + const { haveExcludedStreams } = await concatFiles({ paths, outPath, outDir, outFormat, metadataFromPath, includeAllStreams, streams, ffmpegExperimental, onProgress: setCutProgress, preserveMovData, movFastStart, preserveMetadataOnMerge, chapters: chaptersFromSegments, appendFfmpegCommandLog }); + + const warnings = []; const notices = []; - if (!includeAllStreams) notices.push(i18n.t('If your source files have more than two tracks, the extra tracks might have been removed. You can change this option before merging.')); - if (!hideAllNotifications) openConcatFinishedToast({ filePath: outPath, notices }); + + const outputSize = await readFileSize(outPath); // * 1.06; // testing:) + const sizeCheckResult = checkFileSizes(inputSize, outputSize); + if (sizeCheckResult != null) warnings.push(sizeCheckResult); + + if (clearBatchFilesAfterConcat) closeBatch(); + if (!includeAllStreams && haveExcludedStreams) notices.push(i18n.t('Some extra tracks have been discarded. You can change this option before merging.')); + if (!hideAllNotifications) openConcatFinishedToast({ filePath: outPath, notices, warnings }); } catch (err) { if (err.killed === true) { // assume execa killed (aborted by user) @@ -1410,6 +1424,8 @@ const App = memo(() => { const notices = []; const warnings = []; + if (!exportConfirmEnabled) notices.push(i18n.t('Export options are not shown. You can enable export options by clicking the icon right next to the export button.')); + // https://github.com/mifi/lossless-cut/issues/329 if (isIphoneHevc(mainFileFormatData, mainStreams)) warnings.push(i18n.t('There is a known issue with cutting iPhone HEVC videos. The output file may not work in all players.')); @@ -1461,7 +1477,7 @@ const App = memo(() => { setWorking(); setCutProgress(); } - }, [numStreamsToCopy, setWorking, segmentsToChaptersOnly, outSegTemplateOrDefault, generateOutSegFileNames, segmentsToExport, getOutSegError, cutMultiple, outputDir, customOutDir, fileFormat, duration, isRotationSet, effectiveRotation, copyFileStreams, allFilesMeta, keyframeCut, shortestFlag, ffmpegExperimental, preserveMovData, preserveMetadataOnMerge, movFastStart, avoidNegativeTs, customTagsByFile, customTagsByStreamId, dispositionByStreamId, detectedFps, enableSmartCut, enableOverwriteOutput, willMerge, mainFileFormatData, mainStreams, exportExtraStreams, areWeCutting, hideAllNotifications, cleanupChoices, cleanupFiles, selectedSegmentsOrInverse, segmentsToChapters, invertCutSegments, autoConcatCutSegments, isCustomFormatSelected, autoDeleteMergedSegments, nonCopiedExtraStreams, filePath, handleExportFailed]); + }, [numStreamsToCopy, setWorking, segmentsToChaptersOnly, outSegTemplateOrDefault, generateOutSegFileNames, segmentsToExport, getOutSegError, cutMultiple, outputDir, customOutDir, fileFormat, duration, isRotationSet, effectiveRotation, copyFileStreams, allFilesMeta, keyframeCut, shortestFlag, ffmpegExperimental, preserveMovData, preserveMetadataOnMerge, movFastStart, avoidNegativeTs, customTagsByFile, customTagsByStreamId, dispositionByStreamId, detectedFps, enableSmartCut, enableOverwriteOutput, willMerge, exportConfirmEnabled, mainFileFormatData, mainStreams, exportExtraStreams, areWeCutting, hideAllNotifications, cleanupChoices, cleanupFiles, selectedSegmentsOrInverse, segmentsToChapters, invertCutSegments, autoConcatCutSegments, isCustomFormatSelected, autoDeleteMergedSegments, nonCopiedExtraStreams, filePath, handleExportFailed]); const onExportPress = useCallback(async () => { if (!filePath || workingRef.current || segmentsToExport.length < 1) return; diff --git a/src/dialogs/index.jsx b/src/dialogs/index.jsx index b54b5127..7c1f50f7 100644 --- a/src/dialogs/index.jsx +++ b/src/dialogs/index.jsx @@ -516,9 +516,10 @@ const Warnings = ({ warnings }) => warnings.map((msg) => {i18n.t('If output does not look right, see the Help menu.')}; export async function openCutFinishedToast({ filePath, warnings, notices }) { + const hasWarnings = warnings.length > 0; const html = ( - {i18n.t('Export is done!')} + {hasWarnings ? i18n.t('Export finished with warning(s)') : i18n.t('Export is done!')} {i18n.t('Please test the output file in your desired player/editor before you delete the source file.')} @@ -529,13 +530,15 @@ export async function openCutFinishedToast({ filePath, warnings, notices }) { await openDirToast({ filePath, html, width: 800, position: 'center', timer: 30000 }); } -export async function openConcatFinishedToast({ filePath, notices }) { +export async function openConcatFinishedToast({ filePath, warnings, notices }) { + const hasWarnings = warnings.length > 0; const html = ( - {i18n.t('Files merged!')} + {hasWarnings ? i18n.t('Files merged with warning(s)') : i18n.t('Files merged!')} {i18n.t('Please test the output files in your desired player/editor before you delete the source files.')} + ); diff --git a/src/hooks/useFfmpegOperations.js b/src/hooks/useFfmpegOperations.js index b3f36043..dc2d338f 100644 --- a/src/hooks/useFfmpegOperations.js +++ b/src/hooks/useFfmpegOperations.js @@ -104,7 +104,7 @@ function useFfmpegOperations({ filePath, enableTransferTimestamps }) { chaptersInputIndex = addInput(getChaptersInputArgs(chaptersPath)); } - const streamIdsToCopy = getStreamIdsToCopy({ streams, includeAllStreams }); + const { streamIdsToCopy, excludedStreamIds } = getStreamIdsToCopy({ streams, includeAllStreams }); const mapStreamsArgs = getMapStreamsArgs({ allFilesMeta: { [metadataFromPath]: { streams } }, copyFileStreams: [{ path: metadataFromPath, streamIds: streamIdsToCopy }], @@ -162,11 +162,13 @@ function useFfmpegOperations({ filePath, enableTransferTimestamps }) { const { stdout } = await process; console.log(stdout); + + await optionalTransferTimestamps(metadataFromPath, outPath); + + return { haveExcludedStreams: excludedStreamIds.length > 0 }; } finally { if (chaptersPath) await tryDeleteFiles([chaptersPath]); } - - await optionalTransferTimestamps(metadataFromPath, outPath); }, [optionalTransferTimestamps]); const cutSingle = useCallback(async ({ diff --git a/src/util.js b/src/util.js index e21fd2c8..13ad2eb4 100644 --- a/src/util.js +++ b/src/util.js @@ -3,16 +3,18 @@ import i18n from 'i18next'; import lodashTemplate from 'lodash/template'; import pMap from 'p-map'; import ky from 'ky'; +import prettyBytes from 'pretty-bytes'; import isDev from './isDev'; const { dirname, parse: parsePath, join, extname, isAbsolute, resolve } = window.require('path'); -const fs = window.require('fs-extra'); +const fsExtra = window.require('fs-extra'); +const { stat } = window.require('fs/promises'); const os = window.require('os'); const { ipcRenderer } = window.require('electron'); const remote = window.require('@electron/remote'); -const { readdir, unlink } = fs; +const { readdir, unlink } = fsExtra; const trashFile = async (path) => ipcRenderer.invoke('tryTrashItem', path); @@ -47,9 +49,9 @@ export function getSuffixedOutPath({ customOutDir, filePath, nameSuffix }) { export async function havePermissionToReadFile(filePath) { try { - const fd = await fs.open(filePath, 'r'); + const fd = await fsExtra.open(filePath, 'r'); try { - await fs.close(fd); + await fsExtra.close(fd); } catch (err) { console.error('Failed to close fd', err); } @@ -62,7 +64,7 @@ export async function havePermissionToReadFile(filePath) { export async function checkDirWriteAccess(dirPath) { try { - await fs.access(dirPath, fs.constants.W_OK); + await fsExtra.access(dirPath, fsExtra.constants.W_OK); } catch (err) { if (err.code === 'EPERM') return false; // Thrown on Mac (MAS build) when user has not yet allowed access if (err.code === 'EACCES') return false; // Thrown on Linux when user doesn't have access to output dir @@ -72,12 +74,12 @@ export async function checkDirWriteAccess(dirPath) { } export async function pathExists(pathIn) { - return fs.pathExists(pathIn); + return fsExtra.pathExists(pathIn); } export async function getPathReadAccessError(pathIn) { try { - await fs.access(pathIn, fs.constants.R_OK); + await fsExtra.access(pathIn, fsExtra.constants.R_OK); return undefined; } catch (err) { return err.code; @@ -85,13 +87,13 @@ export async function getPathReadAccessError(pathIn) { } export async function dirExists(dirPath) { - return (await pathExists(dirPath)) && (await fs.lstat(dirPath)).isDirectory(); + return (await pathExists(dirPath)) && (await fsExtra.lstat(dirPath)).isDirectory(); } export async function transferTimestamps(inPath, outPath, offset = 0) { try { - const { atime, mtime } = await fs.stat(inPath); - await fs.utimes(outPath, (atime.getTime() / 1000) + offset, (mtime.getTime() / 1000) + offset); + const { atime, mtime } = await stat(inPath); + await fsExtra.utimes(outPath, (atime.getTime() / 1000) + offset, (mtime.getTime() / 1000) + offset); } catch (err) { console.error('Failed to set output file modified time', err); } @@ -362,3 +364,17 @@ export const getNumDigits = (value) => Math.floor(value > 0 ? Math.log10(value) export function escapeRegExp(string) { return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string } + +export const readFileSize = async (path) => (await stat(path)).size; + +export const readFileSizes = (paths) => pMap(paths, async (path) => readFileSize(path), { concurrency: 5 }); + +export function checkFileSizes(inputSize, outputSize) { + const diff = Math.abs(outputSize - inputSize); + const relDiff = diff / inputSize; + const maxDiffPercent = 5; + const sourceFilesTotalSize = prettyBytes(inputSize); + const outputFileTotalSize = prettyBytes(outputSize); + if (relDiff > maxDiffPercent / 100) return i18n.t('The size of the merged output file ({{outputFileTotalSize}}) differs from the total size of source files ({{sourceFilesTotalSize}}) by more than {{maxDiffPercent}}%. This could indicate that there was a problem during the merge.', { maxDiffPercent, sourceFilesTotalSize, outputFileTotalSize }); + return undefined; +} diff --git a/src/util/streams.js b/src/util/streams.js index e386f8b0..ac63c518 100644 --- a/src/util/streams.js +++ b/src/util/streams.js @@ -218,21 +218,28 @@ export const getRealVideoStreams = (streams) => streams.filter(stream => stream. export const getSubtitleStreams = (streams) => streams.filter(stream => stream.codec_type === 'subtitle'); export function getStreamIdsToCopy({ streams, includeAllStreams }) { - if (includeAllStreams) return streams.map((stream) => stream.index); + if (includeAllStreams) { + return { + streamIdsToCopy: streams.map((stream) => stream.index), + excludedStreamIds: [], + }; + } // If preserveMetadataOnMerge option is enabled, we MUST explicitly map all streams even if includeAllStreams=false. // We cannot use the ffmpeg's automatic stream selection or else ffmpeg might use the metadata source input (index 1) // instead of the concat input (index 0) // https://ffmpeg.org/ffmpeg.html#Automatic-stream-selection - const ret = []; + const streamIdsToCopy = []; // TODO try to mimic ffmpeg default mapping https://ffmpeg.org/ffmpeg.html#Automatic-stream-selection const videoStreams = getRealVideoStreams(streams); const audioStreams = getAudioStreams(streams); const subtitleStreams = getSubtitleStreams(streams); - if (videoStreams.length > 0) ret.push(videoStreams[0].index); - if (audioStreams.length > 0) ret.push(audioStreams[0].index); - if (subtitleStreams.length > 0) ret.push(subtitleStreams[0].index); - return ret; + if (videoStreams.length > 0) streamIdsToCopy.push(videoStreams[0].index); + if (audioStreams.length > 0) streamIdsToCopy.push(audioStreams[0].index); + if (subtitleStreams.length > 0) streamIdsToCopy.push(subtitleStreams[0].index); + + const excludedStreamIds = streams.filter((s) => !streamIdsToCopy.includes(s.index)).map((s) => s.index); + return { streamIdsToCopy, excludedStreamIds }; } // this is just a rough check, could be improved diff --git a/src/util/streams.test.js b/src/util/streams.test.js index c1e5073d..0402e2d4 100644 --- a/src/util/streams.test.js +++ b/src/util/streams.test.js @@ -104,6 +104,7 @@ test('getMapStreamsArgs, smart cut', () => { }); test('getStreamIdsToCopy, includeAllStreams false', () => { - const streamIdsToCopy = getStreamIdsToCopy({ streams: streams1, includeAllStreams: false }); + const { streamIdsToCopy, excludedStreamIds } = getStreamIdsToCopy({ streams: streams1, includeAllStreams: false }); expect(streamIdsToCopy).toEqual([2, 1, 7]); + expect(excludedStreamIds).toEqual([0, 3, 4, 5, 6]); });