Skip to content

Commit 4196b94

Browse files
committed
fix(playback): load original recordings before requesting conversion
Serve original recording bytes without codec probing or transcoding by default. Recording, timeline, and investigation players attempt native playback and request a bounded compatibility conversion only after a browser decode or unsupported-format error. Keep network and aborted loads out of the conversion path and cancel stale fallback polling. Verify actual timeline decoding, seeking, playback, and fallback cancellation with a playable MP4 fixture and range responses. Validation: 8 playback-handler C cases, 15 playback JavaScript cases, 18 browser regression cases, and an HTTP check that HEVC playback serves original bytes/ranges without FFmpeg.
1 parent dbc1ca6 commit 4196b94

13 files changed

Lines changed: 274 additions & 125 deletions

‎docs/API.md‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -836,21 +836,24 @@ Deletes a recording.
836836
GET /api/recordings/play/{id}
837837
```
838838

839-
Streams a recording for playback with byte-range support. HEVC recordings use
840-
a cached H.264 copy prepared by a single background transcode worker.
839+
Streams the original recording immediately with byte-range support. Normal
840+
playback does not probe or convert the video, including HEVC recordings.
841841

842-
Before loading the media URL, poll `GET /api/recordings/play/{id}?prepare=1`:
842+
If a browser rejects the original with a decode or unsupported-format error,
843+
it can request a compatible H.264 copy by polling
844+
`GET /api/recordings/play/{id}?prepare=1`:
843845

844-
- `200` with `{"status":"ready"}`: load the media URL without `prepare=1`.
846+
- `200` with `{"status":"ready"}`: load `/api/recordings/play/{id}?transcode=1`.
845847
- `202` with `{"status":"preparing"}`: wait for the `Retry-After` interval and poll again.
846848
- `500`: preparation failed; the JSON `error` explains the failure. The original
847849
recording remains available from the download endpoint.
848850

849-
Preparation uses the same replay authorization as playback. A media request
850-
made while conversion is pending returns `503` with `Retry-After`; it does not
851-
hold an HTTP worker open for the conversion. H.264 recordings and completed
852-
cache entries are ready immediately. Clients should stop polling when playback
853-
is closed or another recording is selected.
851+
Preparation uses the same replay authorization as playback and retains a single
852+
background conversion worker. A `transcode=1` request made while conversion is
853+
pending returns `503` with `Retry-After`; original playback remains available.
854+
H.264 recordings and completed cache entries are ready immediately. Clients
855+
should stop polling when playback is closed or another recording is selected,
856+
and should not request conversion for a network error or aborted media load.
854857

855858
#### Download Recording
856859

‎include/web/api_handlers_recordings_playback.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
/**
77
* @brief Backend-agnostic handler for GET /api/recordings/play/:id
88
*
9-
* Serves a recording file for playback with range request support for seeking.
10-
* With ?prepare=1, returns JSON: 200 when ready or 202 with Retry-After while
11-
* a bounded background transcode is pending. Media requests made before the
12-
* cache is ready return 503 with Retry-After instead of blocking an HTTP worker.
9+
* Serves the original recording with range support, without probing or encoding.
10+
* For browsers that reject it, ?prepare=1 requests a compatibility copy and
11+
* returns JSON: 200 when ready or 202 with Retry-After while pending. Load the
12+
* prepared media using ?transcode=1 (503 with Retry-After if not yet ready).
1313
*
1414
* @param req HTTP request
1515
* @param res HTTP response

‎src/web/api_handlers_recordings_playback.c‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -81,39 +81,45 @@ void handle_recordings_playback(const http_request_t *req, http_response_t *res)
8181

8282
log_info("Serving file for playback: %s (%ld bytes)", recording.file_path, st.st_size);
8383

84-
// The modal polls ?prepare=1 before attaching the media URL. Never wait
85-
// for an encode in a libuv worker: enough duplicate/range requests would
86-
// starve both other APIs and the filesystem reads used to serve videos.
84+
// Direct playback always serves the original. Only clients that cannot
85+
// decode it request the compatibility copy; HEVC alone does not imply
86+
// that a browser needs a transcode.
8787
char prepare[8];
8888
bool prepare_only = http_request_get_query_param(req, "prepare", prepare,
8989
sizeof(prepare)) > 0 &&
9090
strcmp(prepare, "1") == 0;
91+
char transcode[8];
92+
bool compatibility_requested = prepare_only ||
93+
(http_request_get_query_param(req, "transcode", transcode, sizeof(transcode)) > 0 &&
94+
strcmp(transcode, "1") == 0);
9195

9296
const char *serve_path = recording.file_path;
9397
char transcode_cache_path[MAX_PATH_LENGTH];
94-
bool have_cache_path = build_recording_transcode_cache_path(
95-
g_config.storage_path, id, transcode_cache_path,
96-
sizeof(transcode_cache_path)) == 0;
97-
struct stat cache_st;
98-
if (have_cache_path && stat(transcode_cache_path, &cache_st) == 0 &&
99-
S_ISREG(cache_st.st_mode) && cache_st.st_size > 0) {
100-
serve_path = transcode_cache_path;
101-
} else if (recording_needs_hevc_transcode(recording.file_path)) {
102-
recording_transcode_status_t status = have_cache_path
103-
? request_recording_transcode_cache(recording.file_path, transcode_cache_path)
104-
: RECORDING_TRANSCODE_FAILED;
105-
if (status == RECORDING_TRANSCODE_PENDING) {
106-
http_response_add_header(res, "Retry-After", "2");
107-
http_response_set_json(res, prepare_only ? 202 : 503,
108-
"{\"status\":\"preparing\"}");
109-
return;
98+
if (compatibility_requested) {
99+
bool have_cache_path = build_recording_transcode_cache_path(
100+
g_config.storage_path, id, transcode_cache_path,
101+
sizeof(transcode_cache_path)) == 0;
102+
struct stat cache_st;
103+
if (have_cache_path && stat(transcode_cache_path, &cache_st) == 0 &&
104+
S_ISREG(cache_st.st_mode) && cache_st.st_size > 0) {
105+
serve_path = transcode_cache_path;
106+
} else if (recording_needs_hevc_transcode(recording.file_path)) {
107+
recording_transcode_status_t status = have_cache_path
108+
? request_recording_transcode_cache(recording.file_path, transcode_cache_path)
109+
: RECORDING_TRANSCODE_FAILED;
110+
if (status == RECORDING_TRANSCODE_PENDING) {
111+
http_response_add_header(res, "Retry-After", "2");
112+
http_response_set_json(res, prepare_only ? 202 : 503,
113+
"{\"status\":\"preparing\"}");
114+
return;
115+
}
116+
if (status == RECORDING_TRANSCODE_FAILED) {
117+
http_response_set_json_error(res, 500,
118+
"Unable to prepare recording for playback. Download the original or try again later.");
119+
return;
120+
}
121+
serve_path = transcode_cache_path;
110122
}
111-
if (status == RECORDING_TRANSCODE_FAILED) {
112-
http_response_set_json_error(res, 500,
113-
"Unable to prepare recording for playback. Download the original or try again later.");
114-
return;
115-
}
116-
serve_path = transcode_cache_path;
117123
}
118124

119125
if (prepare_only) {
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { readFileSync } from 'node:fs';
2+
import { join } from 'node:path';
3+
import type { Route } from '@playwright/test';
4+
5+
// Ten minutes of blue, 64x64 H.264 at 1 fps. Long enough for timeline seeks,
6+
// small enough to serve inline, and decodable without cameras or local FFmpeg.
7+
// Generated with: ffmpeg -f lavfi -i color=c=blue:size=64x64:rate=1:duration=600
8+
// -c:v libx264 -threads 1 -pix_fmt yuv420p -movflags +faststart recording.mp4
9+
export const PLAYABLE_RECORDING_MP4 = readFileSync(join(__dirname, 'recording.mp4'));
10+
11+
// Match the production file server's range contract so Chromium exposes a
12+
// seekable duration; an empty media response cannot test timeline seeking.
13+
export function serveRecordingMedia(route: Route) {
14+
const range = route.request().headers().range?.match(/^bytes=(\d+)-(\d*)$/);
15+
const headers: Record<string, string> = { 'Accept-Ranges': 'bytes' };
16+
let body = PLAYABLE_RECORDING_MP4;
17+
if (range) {
18+
const start = Number(range[1]);
19+
const end = Math.min(range[2] ? Number(range[2]) : body.length - 1, body.length - 1);
20+
headers['Content-Range'] = `bytes ${start}-${end}/${body.length}`;
21+
body = body.subarray(start, end + 1);
22+
}
23+
return route.fulfill({ status: range ? 206 : 200, contentType: 'video/mp4', headers, body });
24+
}
17.7 KB
Binary file not shown.

‎tests/integration/specs/recordings.ui.spec.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import { test, expect } from '@playwright/test';
99
import { RecordingsPage } from '../pages/RecordingsPage';
1010
import { USERS, login, sleep } from '../fixtures/test-fixtures';
11+
import { serveRecordingMedia } from '../fixtures/recording-media';
1112

1213
type MockRecording = {
1314
id: number;
@@ -316,18 +317,15 @@ test.describe('Recordings Page @ui @recordings', () => {
316317
}],
317318
pagination: { total: 1, pages: 1, limit: 20 }
318319
} }));
319-
let prepared = false;
320320
let preparationRequests = 0;
321321
await page.route('**/api/recordings/play/601*', route => {
322322
if (new URL(route.request().url()).searchParams.has('prepare')) {
323323
preparationRequests++;
324324
return route.fulfill({
325-
status: prepared ? 200 : 202,
326-
headers: { 'Retry-After': '1' },
327-
json: { status: prepared ? 'ready' : 'preparing' },
325+
json: { status: 'ready' },
328326
});
329327
}
330-
return route.fulfill({ status: 204, body: '' });
328+
return serveRecordingMedia(route);
331329
});
332330
await page.route('**/api/recordings/601', route => route.fulfill({ json: {
333331
id: 601,
@@ -356,13 +354,12 @@ test.describe('Recordings Page @ui @recordings', () => {
356354
await page.locator('button[title="Play"]').first().click();
357355

358356
const modal = page.locator('#video-preview-modal');
359-
await expect(modal.getByRole('status')).toHaveText('Preparing recording for playback…');
360-
await expect(modal.locator('video')).not.toHaveAttribute('src');
357+
await expect.poll(() => modal.locator('video').evaluate((video: HTMLVideoElement) => video.videoWidth)).toBe(64);
358+
expect(preparationRequests).toBe(0);
361359
await modal.locator('button.close').click();
362360
const requestsAtClose = preparationRequests;
363361
await page.waitForTimeout(1200);
364362
expect(preparationRequests).toBe(requestsAtClose);
365-
prepared = true;
366363
await page.locator('button[title="Play"]').first().click();
367364
await expect(modal.locator('video')).toHaveAttribute('src', '/api/recordings/play/601');
368365

‎tests/integration/specs/timeline-boundary.ui.spec.ts‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { test, expect, Page } from '@playwright/test';
22
import { login, USERS } from '../fixtures/test-fixtures';
33
import { TimelinePage } from '../pages/TimelinePage';
4+
import { serveRecordingMedia } from '../fixtures/recording-media';
45

56
type Segment = { id: number; stream: string; start_timestamp: number; end_timestamp: number };
67

@@ -268,7 +269,34 @@ test.describe('Timeline boundary flows @ui @timeline', () => {
268269
await expect(timelinePage.nextRecordingButton).toBeDisabled();
269270
});
270271

271-
test('waits for playback preparation and cancels polling when switching recordings', async ({ page }) => {
272+
test('plays and seeks the original recording without preparing a copy', async ({ page }) => {
273+
const stream = 'front_door';
274+
const date = '2026-03-08';
275+
const segments: Segment[] = [
276+
{ id: 320, stream, start_timestamp: localTimestamp(date, '11:00:00'), end_timestamp: localTimestamp(date, '11:10:00') }
277+
];
278+
await mockTimelineApis(page, stream, segments);
279+
const playbackRequests: string[] = [];
280+
await page.route('**/api/recordings/play/*', route => {
281+
playbackRequests.push(route.request().url());
282+
return serveRecordingMedia(route);
283+
});
284+
await page.goto(`/timeline.html?stream=${stream}&date=${date}&time=11:02:00`, { waitUntil: 'domcontentloaded' });
285+
const timelinePage = new TimelinePage(page);
286+
await expect.poll(() => timelinePage.videoPlayer.evaluate((video: HTMLVideoElement) => video.videoWidth)).toBe(64);
287+
await expect.poll(() => timelinePage.videoPlayer.evaluate((video: HTMLVideoElement) => Math.round(video.currentTime))).toBe(120);
288+
await timelinePage.videoPlayer.evaluate(async (video: HTMLVideoElement) => {
289+
video.muted = true;
290+
await video.play();
291+
});
292+
await expect.poll(() => timelinePage.videoPlayer.evaluate((video: HTMLVideoElement) => video.currentTime)).toBeGreaterThan(120);
293+
expect(playbackRequests.length).toBeGreaterThan(0);
294+
expect(playbackRequests.every(url => !new URL(url).searchParams.has('prepare') &&
295+
!new URL(url).searchParams.has('transcode'))).toBe(true);
296+
await expect(timelinePage.videoContainer.getByRole('status')).toHaveCount(0);
297+
});
298+
299+
test('prepares only rejected recordings and cancels fallback when switching recordings', async ({ page }) => {
272300
const stream = 'front_door';
273301
const date = '2026-03-08';
274302
const segments: Segment[] = [
@@ -278,7 +306,7 @@ test.describe('Timeline boundary flows @ui @timeline', () => {
278306
];
279307
await mockTimelineApis(page, stream, segments);
280308
const preparations: number[] = [];
281-
const mediaRequests: number[] = [];
309+
const mediaRequests: Array<{ id: number; transcode: boolean }> = [];
282310
let firstReady = false;
283311
await page.route('**/api/recordings/play/*', route => {
284312
const url = new URL(route.request().url());
@@ -292,22 +320,28 @@ test.describe('Timeline boundary flows @ui @timeline', () => {
292320
json: { status: ready ? 'ready' : 'preparing' }
293321
});
294322
}
295-
mediaRequests.push(id);
296-
return route.fulfill({ status: 204, body: '' });
323+
const transcode = url.searchParams.get('transcode') === '1';
324+
mediaRequests.push({ id, transcode });
325+
if (transcode || id === 323) return serveRecordingMedia(route);
326+
return route.fulfill({
327+
contentType: 'video/mp4',
328+
body: Buffer.from('unsupported video')
329+
});
297330
});
298331
await page.goto(`/timeline.html?stream=${stream}&date=${date}&time=11:00:00`, { waitUntil: 'domcontentloaded' });
299332

300333
const timelinePage = new TimelinePage(page);
301334
const status = timelinePage.videoContainer.getByRole('status');
302-
await expect(status).toHaveText('Preparing recording for playback…');
335+
await expect(status).toHaveText('Preparing a compatible version for this browser…');
303336
await expect(timelinePage.videoPlayer).not.toHaveAttribute('src');
304-
expect(mediaRequests).toEqual([]);
337+
expect(mediaRequests).toEqual([{ id: 321, transcode: false }]);
305338
firstReady = true;
306339
await expect(timelinePage.videoPlayer).toHaveAttribute('src', /\/api\/recordings\/play\/321(?:\?|$)/);
340+
await expect.poll(() => timelinePage.videoPlayer.evaluate((video: HTMLVideoElement) => video.videoWidth)).toBe(64);
307341
expect(preparations.filter(id => id === 321).length).toBeGreaterThanOrEqual(2);
308342

309343
await timelinePage.nextRecordingButton.click();
310-
await expect(status).toHaveText('Preparing recording for playback…');
344+
await expect(status).toHaveText('Preparing a compatible version for this browser…');
311345
await expect(timelinePage.videoPlayer).not.toHaveAttribute('src');
312346
await timelinePage.nextRecordingButton.click();
313347
await expect(timelinePage.videoPlayer).toHaveAttribute('src', /\/api\/recordings\/play\/323(?:\?|$)/);
@@ -316,7 +350,11 @@ test.describe('Timeline boundary flows @ui @timeline', () => {
316350
// Wait beyond Retry-After to catch polling or media loads from the old clip.
317351
await page.waitForTimeout(1200);
318352
expect(preparations.filter(id => id === 322)).toHaveLength(abandonedRequests);
319-
expect(mediaRequests).toEqual([321, 323]);
353+
expect(mediaRequests).toEqual([
354+
{ id: 321, transcode: false }, { id: 321, transcode: true },
355+
{ id: 322, transcode: false }, { id: 323, transcode: false }
356+
]);
357+
expect(preparations).not.toContain(323);
320358
await expect(timelinePage.videoPlayer).toHaveAttribute('src', /\/api\/recordings\/play\/323(?:\?|$)/);
321359
});
322360

‎tests/unit/test_api_handlers_recordings_playback.c‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,23 @@ void test_hevc_preparation_returns_retryable_json_without_serving_media(void) {
9696
}
9797

9898
void test_uncached_media_request_returns_prompt_retry_instead_of_waiting(void) {
99-
request.query_string[0] = '\0';
99+
strcpy(request.query_string, "transcode=1");
100100
handle_recordings_playback(&request, &response);
101101
TEST_ASSERT_EQUAL_INT(503, response.status_code);
102102
TEST_ASSERT_EQUAL_STRING("2", response_header("Retry-After"));
103103
TEST_ASSERT_EQUAL_INT(0, serve_calls);
104104
}
105105

106+
void test_hevc_media_plays_original_without_probe_or_preparation(void) {
107+
request.query_string[0] = '\0';
108+
handle_recordings_playback(&request, &response);
109+
TEST_ASSERT_EQUAL_INT(200, response.status_code);
110+
TEST_ASSERT_EQUAL_INT(1, serve_calls);
111+
TEST_ASSERT_EQUAL_STRING(recording.file_path, served_path);
112+
TEST_ASSERT_EQUAL_INT(0, probe_calls);
113+
TEST_ASSERT_EQUAL_INT(0, prepare_calls);
114+
}
115+
106116
void test_h264_preparation_is_ready_without_a_transcode(void) {
107117
hevc = false;
108118
handle_recordings_playback(&request, &response);
@@ -146,7 +156,7 @@ void test_cached_hevc_media_skips_probe_and_conversion(void) {
146156
TEST_ASSERT_NOT_NULL(file);
147157
fputs("cached video", file);
148158
fclose(file);
149-
request.query_string[0] = '\0';
159+
strcpy(request.query_string, "transcode=1");
150160
handle_recordings_playback(&request, &response);
151161
unlink(cache);
152162
rmdir(cache_dir);
@@ -167,6 +177,7 @@ int main(void) {
167177
UNITY_BEGIN();
168178
RUN_TEST(test_hevc_preparation_returns_retryable_json_without_serving_media);
169179
RUN_TEST(test_uncached_media_request_returns_prompt_retry_instead_of_waiting);
180+
RUN_TEST(test_hevc_media_plays_original_without_probe_or_preparation);
170181
RUN_TEST(test_h264_preparation_is_ready_without_a_transcode);
171182
RUN_TEST(test_h264_media_uses_original_with_range_support);
172183
RUN_TEST(test_failed_transcode_reports_error_instead_of_unplayable_original);

‎web/js/components/preact/UI.jsx‎

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import dayjs from 'dayjs';
1313
import utc from 'dayjs/plugin/utc';
1414
import customParseFormat from 'dayjs/plugin/customParseFormat';
1515
import { ConfirmDialog } from './common/ModalDialog.jsx';
16-
import { prepareRecordingPlayback } from '../../utils/recording-playback.js';
16+
import { loadRecordingPlayback } from '../../utils/recording-playback.js';
1717

1818
export { ConfirmDialog } from './common/ModalDialog.jsx';
1919

@@ -602,27 +602,12 @@ export function VideoModal({ isOpen, onClose, videoUrl, title, downloadUrl }) {
602602
useEffect(() => {
603603
if (!isOpen || !videoUrl || !videoRef.current) return;
604604
const video = videoRef.current;
605-
const controller = new AbortController();
606-
setPlaybackMessage('Loading recording…');
607-
prepareRecordingPlayback(videoUrl, {
608-
signal: controller.signal,
609-
onWaiting: () => setPlaybackMessage('Preparing recording for playback…'),
610-
}).then(url => {
611-
if (controller.signal.aborted) return;
612-
setPlaybackMessage('');
613-
video.src = url;
614-
video.load();
615-
}).catch(error => {
616-
if (controller.signal.aborted) return;
617-
console.error('Recording preparation failed:', error);
618-
setPlaybackMessage(error.message);
605+
setPlaybackMessage('');
606+
return loadRecordingPlayback(video, videoUrl, {
607+
onPreparing: () => setPlaybackMessage('Preparing a compatible version for this browser…'),
608+
onReady: () => setPlaybackMessage(''),
609+
onError: error => setPlaybackMessage(error.message),
619610
});
620-
return () => {
621-
controller.abort();
622-
video.pause();
623-
video.removeAttribute('src');
624-
video.load();
625-
};
626611
}, [isOpen, videoUrl]);
627612

628613
// Update detection overlay when enabled/disabled
@@ -814,11 +799,6 @@ export function VideoModal({ isOpen, onClose, videoUrl, title, downloadUrl }) {
814799
controls
815800
controlsList="nofullscreen"
816801
key={videoUrl} /* Add key to force re-render when URL changes */
817-
onError={(e) => {
818-
if (!e.currentTarget.hasAttribute('src')) return;
819-
console.error('Video error:', e);
820-
showStatusMessage('Error loading video. Please try again.', 'error');
821-
}}
822802
onLoadStart={() => console.log('Video load started')}
823803
onLoadedData={() => console.log('Video data loaded')}
824804
/>

0 commit comments

Comments
 (0)