Transcode modal: allow a container-only remux

Keeping both streams is no longer refused: ffmpeg copies them into the
chosen container with fast start.  The container select starts at the
file's own container, the hint explains what a remux does, and the button
reads Remux when nothing is re-encoded.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-09-14 19:02:13 -06:00
parent ca1dc82aa9
commit 749d5ac32b
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
5 changed files with 100 additions and 22 deletions

View file

@ -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 && <Text size='sm' c='var(--muted)'>
Choose at least one codec to convert.
{remuxOnly && defaultsLoaded && <Text size='sm' c='var(--muted)'>
{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).`}
</Text>}
</>}
{jobId && <JobProgress jobId={jobId} onFinished={handleFinished}/>}
@ -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'}
</Button>}
</Modal.Actions>
</Modal>

View file

@ -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(<TranscodeModal open={true} onClose={jest.fn()} fileGroupId={7} video={video}/>);
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(<TranscodeModal open={true} onClose={jest.fn()} fileGroupId={7} video={mkv}/>);
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 () => {

View file

@ -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

View file

@ -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',

View file

@ -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: