diff --git a/app/src/components/VideoEditMenu.js b/app/src/components/VideoEditMenu.js index d8244cef..8a231e5d 100644 --- a/app/src/components/VideoEditMenu.js +++ b/app/src/components/VideoEditMenu.js @@ -148,6 +148,12 @@ export function JobProgress({jobId, onFinished}) { * The selects start at the user's preferred codecs from the Videos settings (the same preference * lists the downloader honors), so the common case is a glance and a click. */ +/** The container of a video file, when it is one ffmpeg can write for us. */ +export function currentContainer(video) { + const suffix = String(video?.video_path || '').split('.').pop().toLowerCase(); + return transcodeContainerOptions.some(i => i.value === suffix) ? suffix : null; +} + export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) { const wrolModeEnabled = useWROLMode(); const [videoCodec, setVideoCodec] = useState(TRANSCODE_COPY); @@ -166,6 +172,8 @@ export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) // Each opening starts over: a finished Job's progress must not linger on the next open. setJobId(null); setDefaultsLoaded(false); + // Start from the file's own container so a codec-only change keeps it. + setContainer(currentContainer(video) || 'mp4'); let stale = false; const load = async () => { try { @@ -185,7 +193,9 @@ export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) }; }, [open]); - const nothingToDo = videoCodec === TRANSCODE_COPY && audioCodec === TRANSCODE_COPY; + // Both streams kept: a remux. Cheap, and it fixes the container (or adds fast start). + const remuxOnly = videoCodec === TRANSCODE_COPY && audioCodec === TRANSCODE_COPY; + const sameContainer = container === currentContainer(video); const handleStart = async () => { setSubmitting(true); @@ -246,8 +256,10 @@ export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) onChange={value => setContainer(value || 'mp4')} allowDeselect={false} /> - {nothingToDo && defaultsLoaded && - Choose at least one codec to convert. + {remuxOnly && defaultsLoaded && + {sameContainer + ? 'Both streams are kept: the file is rewritten as-is with fast start (moves the index to the front for quicker playback start).' + : `Both streams are kept: only the container changes to ${container} (a quick remux, no quality loss).`} } } {jobId && } @@ -260,9 +272,9 @@ export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) icon='film' onClick={handleStart} loading={submitting} - disabled={submitting || nothingToDo || !defaultsLoaded || !!wrolModeEnabled} + disabled={submitting || !defaultsLoaded || !!wrolModeEnabled} > - Transcode + {remuxOnly ? 'Remux' : 'Transcode'} } diff --git a/app/src/components/VideoEditMenu.test.js b/app/src/components/VideoEditMenu.test.js index ab438140..6285c230 100644 --- a/app/src/components/VideoEditMenu.test.js +++ b/app/src/components/VideoEditMenu.test.js @@ -1,7 +1,9 @@ import React from 'react'; import {act, fireEvent, screen, waitFor} from '@testing-library/react'; import {renderWithProviders} from '../test-utils'; -import {currentCodecs, EditVideoModal, JobProgress, preferredTarget, TranscodeModal, VideoEditMenu} from './VideoEditMenu'; +import { + currentCodecs, currentContainer, EditVideoModal, JobProgress, preferredTarget, TranscodeModal, VideoEditMenu, +} from './VideoEditMenu'; import {TRANSCODE_COPY, transcodeAudioCodecOptions, transcodeVideoCodecOptions} from './Vars'; jest.mock('../api', () => ({ @@ -39,6 +41,13 @@ describe('helpers', () => { expect(currentCodecs(null)).toEqual({video: [], audio: []}); }); + test('currentContainer reads the file suffix, only for containers we can write', () => { + expect(currentContainer({video_path: 'videos/a/movie.MP4'})).toBe('mp4'); + expect(currentContainer({video_path: 'videos/a/movie.mkv'})).toBe('mkv'); + expect(currentContainer({video_path: 'videos/a/movie.webm'})).toBeNull(); + expect(currentContainer(null)).toBeNull(); + }); + test('preferredTarget picks the first preference ffmpeg can produce', () => { expect(preferredTarget(['vp9', 'h264'], transcodeVideoCodecOptions)).toBe('h264'); expect(preferredTarget(['vp9', 'av1'], transcodeVideoCodecOptions)).toBe(TRANSCODE_COPY); @@ -77,13 +86,30 @@ describe('TranscodeModal', () => { expect(await screen.findByText('pending')).toBeInTheDocument(); }); - test('refuses to start when every stream is kept', async () => { + test('keeping every stream is a remux into the chosen container', async () => { fetchVideoDownloadDefaults.mockResolvedValue({video_codecs: [], audio_codecs: []}); + // A webm cannot be kept (not a container we write), so the select starts at mp4: a change. renderWithProviders(); - await waitFor(() => expect(screen.getByText('Choose at least one codec to convert.')).toBeInTheDocument()); - expect(screen.getByRole('button', {name: /Transcode/})).toBeDisabled(); - expect(transcodeVideo).not.toHaveBeenCalled(); + await waitFor(() => expect(selectInput('Container')).toHaveValue('mp4')); + expect(await screen.findByText(/only the container changes to mp4/)).toBeInTheDocument(); + + const button = screen.getByRole('button', {name: 'Remux'}); + expect(button).toBeEnabled(); + fireEvent.click(button); + await waitFor(() => expect(transcodeVideo).toHaveBeenCalledWith(7, { + video_codec: null, audio_codec: null, container: 'mp4', + })); + }); + + test('a remux into the same container is described as a fast-start rewrite', async () => { + fetchVideoDownloadDefaults.mockResolvedValue({video_codecs: [], audio_codecs: []}); + const mkv = {...video, video_path: 'videos/movie.mkv'}; + renderWithProviders(); + + await waitFor(() => expect(selectInput('Container')).toHaveValue('mkv')); + expect(await screen.findByText(/rewritten as-is with fast start/)).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Remux'})).toBeEnabled(); }); test('WROL Mode disables starting a transcode', async () => { diff --git a/modules/videos/test/test_transcode.py b/modules/videos/test/test_transcode.py index e7967e53..41247c6c 100644 --- a/modules/videos/test/test_transcode.py +++ b/modules/videos/test/test_transcode.py @@ -211,6 +211,20 @@ async def test_transcode_lock_serializes(test_directory, async_client): @pytest.mark.asyncio -async def test_transcode_video_file_requires_target(): - with pytest.raises(RuntimeError, match='target codec'): - await transcode_video_file(pathlib.Path('/tmp/video.mp4')) +async def test_transcode_video_file_remux(test_directory, async_client): + """No target codec is a remux: both streams are copied into the requested container.""" + video_path = test_directory / 'video.mkv' + video_path.write_bytes(b'fake video data') + + async def fake_run_command(cmd, **kwargs): + pathlib.Path(cmd[-1]).write_bytes(b'remuxed data') + return CommandResult(return_code=0, cancelled=False, stdout=b'', stderr=b'', elapsed=1) + + with mock.patch('modules.videos.transcode.run_command', side_effect=fake_run_command) as mock_run: + result = await transcode_video_file(video_path, container='mp4') + + assert result == test_directory / 'video.mp4' and result.is_file() + assert not video_path.exists() + cmd = mock_run.call_args[0][0] + assert cmd[cmd.index('-c:v') + 1] == 'copy' and cmd[cmd.index('-c:a') + 1] == 'copy' + assert '+faststart' in cmd diff --git a/modules/videos/test/test_transcode_job.py b/modules/videos/test/test_transcode_job.py index 8e1326e1..da433b35 100644 --- a/modules/videos/test/test_transcode_job.py +++ b/modules/videos/test/test_transcode_job.py @@ -79,6 +79,30 @@ async def test_transcode_video_job(test_session, test_directory, async_client, v assert test_session.query(FileGroup).count() == 1 +@pytest.mark.asyncio +async def test_transcode_video_job_remux(test_session, test_directory, async_client, video_factory): + """No target codec is a remux: both streams are copied into the requested container.""" + video_path = test_directory / 'videos/NO CHANNEL/movie.mkv' + video_path.parent.mkdir(parents=True, exist_ok=True) + video = video_factory(with_video_file=video_path) + test_session.commit() + + async def fake_run_command(cmd, stdout_callback=None, **kwargs): + shutil.copy(PROJECT_DIR / 'test/big_buck_bunny_720p_1mb.mp4', cmd[-1]) + return CommandResult(return_code=0, cancelled=False, stdout=b'', stderr=b'', elapsed=1) + + with mock.patch('wrolpi.cmd.run_command', side_effect=fake_run_command) as mock_run: + job_id = transcode_video_job.enqueue(file_group_id=video.file_group_id, container='mp4') + record = await jobs.wait_for_job(job_id) + + assert record['status'] == jobs.COMPLETE, record + cmd = mock_run.call_args[0][0] + assert cmd[cmd.index('-c:v') + 1] == 'copy' and cmd[cmd.index('-c:a') + 1] == 'copy' + assert '+faststart' in cmd + assert not video_path.exists() + assert (test_directory / 'videos/NO CHANNEL/movie.mp4').is_file() + + @pytest.mark.asyncio async def test_transcode_video_job_failure(test_session, test_directory, async_client, video_factory): """A failed ffmpeg fails the Job and leaves the original file in place.""" @@ -103,10 +127,6 @@ async def test_video_transcode_api(async_client, test_session, video_factory, wr video = video_factory() test_session.commit() - # Nothing to transcode. - request, response = await async_client.post(f'/api/videos/{video.file_group_id}/transcode', - content=json.dumps({})) - assert response.status == 400, response.json # Unsupported target. request, response = await async_client.post(f'/api/videos/{video.file_group_id}/transcode', content=json.dumps({'video_codec': 'av1'})) @@ -130,6 +150,14 @@ async def test_video_transcode_api(async_client, test_session, video_factory, wr # Do not run it (ffmpeg); cancel instead. jobs.cancel_job(job_id) + # No codec at all is allowed: a container-only remux. + request, response = await async_client.post(f'/api/videos/{video.file_group_id}/transcode', + content=json.dumps({'container': 'mkv'})) + assert response.status == 200, response.json + assert jobs.get_job(response.json['job_id'])['kwargs'] == { + 'file_group_id': video.file_group_id, 'video_codec': None, 'audio_codec': None, 'container': 'mkv'} + jobs.cancel_job(response.json['job_id']) + # WROL mode forbids modifying files. await wrol_mode_fixture(True) request, response = await async_client.post(f'/api/videos/{video.file_group_id}/transcode', diff --git a/modules/videos/transcode.py b/modules/videos/transcode.py index 45001513..016c2d9b 100644 --- a/modules/videos/transcode.py +++ b/modules/videos/transcode.py @@ -133,8 +133,8 @@ async def transcode_video_file(video_path: pathlib.Path, """ if not FFMPEG_BIN: raise RuntimeError('ffmpeg was not found') - if not target_vcodec and not target_acodec: - raise RuntimeError('Refusing to transcode without a target codec') + # No target for either stream is a remux: both streams are copied into the (new) container + # with faststart. Cheap, and what a user wants when only the container is wrong. if container not in TRANSCODE_CONTAINERS: logger.info(f'Forcing mp4 container for transcode of {video_path} ({container} is not supported)') container = 'mp4' @@ -250,9 +250,7 @@ async def _transcode_video_file(video_path: pathlib.Path, target_vcodec: Optiona def validate_transcode_request(video_codec: Optional[str], audio_codec: Optional[str], container: str): - """@raise ValueError: when the request cannot be transcoded.""" - if not video_codec and not audio_codec: - raise ValueError('At least one of video_codec or audio_codec is required') + """@raise ValueError: when the request cannot be transcoded. No codec at all is a remux.""" if video_codec and video_codec not in TRANSCODE_VIDEO_TARGETS: raise ValueError(f'Cannot transcode video to {video_codec!r}; supported: {sorted(TRANSCODE_VIDEO_TARGETS)}') if audio_codec and audio_codec not in TRANSCODE_AUDIO_TARGETS: