Skip to content

Commit 53f0be8

Browse files
committed
fix(recordings): prepare playback outside the HTTP worker pool
Run serialized HEVC conversions in a bounded background worker and expose a retryable preparation response. Poll before loading media in recording, timeline, and investigation players; cancel stale polling and remove duplicate media loads. Abort active conversion during shutdown. Validate with 23 C cases, 85 frontend unit tests, three browser integration tests, modal/timeline decoding smoke checks, and 30 concurrent requests against a two-worker server. Preserve the existing FFmpeg thread and memory limits.
1 parent 2f4b75c commit 53f0be8

17 files changed

Lines changed: 697 additions & 119 deletions

‎docs/API.md‎

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

839-
Streams a recording for playback.
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.
841+
842+
Before loading the media URL, poll `GET /api/recordings/play/{id}?prepare=1`:
843+
844+
- `200` with `{"status":"ready"}`: load the media URL without `prepare=1`.
845+
- `202` with `{"status":"preparing"}`: wait for the `Retry-After` interval and poll again.
846+
- `500`: preparation failed; the JSON `error` explains the failure. The original
847+
recording remains available from the download endpoint.
848+
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.
840854

841855
#### Download Recording
842856

‎include/video/recording_transcode.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,23 @@ bool recording_needs_hevc_transcode(const char *file_path);
4444
*/
4545
int ensure_recording_transcode_cache(const char *original_path, const char *cache_path);
4646

47+
typedef enum {
48+
RECORDING_TRANSCODE_FAILED = -1,
49+
RECORDING_TRANSCODE_READY = 0,
50+
RECORDING_TRANSCODE_PENDING = 1
51+
} recording_transcode_status_t;
52+
53+
/**
54+
* Start or poll preparation on a dedicated thread, without waiting for FFmpeg
55+
* in an HTTP worker. Only one background job is admitted; other recordings
56+
* return PENDING and can retry later. Duplicate requests share the same job.
57+
* Failed jobs return FAILED for 30 seconds to avoid a subprocess retry storm.
58+
* Paths are copied before returning. Completed cache hits return READY.
59+
*/
60+
recording_transcode_status_t request_recording_transcode_cache(
61+
const char *original_path, const char *cache_path);
62+
63+
/** Stop admitting playback jobs, abort FFmpeg and join the background worker. */
64+
void shutdown_recording_transcode(void);
65+
4766
#endif /* LIGHTNVR_RECORDING_TRANSCODE_H */

‎include/web/api_handlers_recordings_playback.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
* @brief Backend-agnostic handler for GET /api/recordings/play/:id
88
*
99
* 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.
1013
*
1114
* @param req HTTP request
1215
* @param res HTTP response

‎src/core/main.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "video/streams.h"
4444
#include "video/hls_streaming.h"
4545
#include "video/mp4_recording.h"
46+
#include "video/recording_transcode.h"
4647
#include "video/stream_transcoding.h"
4748
#include "video/hls_writer.h"
4849
#include "video/detection_stream.h"
@@ -1408,6 +1409,7 @@ int main(int argc, char *argv[]) {
14081409
// Cleanup
14091410
cleanup:
14101411
log_info("Starting cleanup process...");
1412+
shutdown_recording_transcode();
14111413

14121414
// Stop request producers before any state they can inspect is dismantled.
14131415
// Keeping this at the shared cleanup label also covers partial startup.

‎src/video/recording_transcode.c‎

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
#include <fcntl.h>
77
#include <inttypes.h>
88
#include <pthread.h>
9+
#include <signal.h>
910
#include <stdatomic.h>
1011
#include <stdio.h>
1112
#include <string.h>
1213
#include <sys/stat.h>
1314
#include <sys/wait.h>
15+
#include <time.h>
1416
#include <unistd.h>
1517

1618
#include <libavformat/avformat.h>
@@ -28,10 +30,24 @@
2830
* PATH already covers both. */
2931
#define FFMPEG_BINARY "ffmpeg"
3032

31-
/* Playback handlers run concurrently in the HTTP worker pool. Keep only one
32-
* recording transcode active across all recordings: even different clips can
33-
* exhaust the memory shared with recording and go2rtc on a small NVR. */
33+
/* Keep only one recording transcode active, including synchronous callers:
34+
* different clips can exhaust memory shared with recording on a small NVR. */
3435
static pthread_mutex_t s_transcode_mutex = PTHREAD_MUTEX_INITIALIZER;
36+
static atomic_bool s_stopping = false;
37+
38+
/* This mutex only protects job bookkeeping, never an encode or a slot wait.
39+
* A separate thread keeps both FFmpeg and s_transcode_mutex waits out of
40+
* libuv's shared pool (which also serves APIs and reads video file chunks). */
41+
static pthread_mutex_t s_job_mutex = PTHREAD_MUTEX_INITIALIZER;
42+
static struct {
43+
pthread_t thread;
44+
bool started;
45+
bool active;
46+
char original_path[512];
47+
char cache_path[512];
48+
int result;
49+
struct timespec finished;
50+
} s_job;
3551

3652
static bool file_exists_nonempty(const char *path) {
3753
struct stat st;
@@ -91,11 +107,20 @@ static int run_and_wait(char *const argv[]) {
91107
}
92108

93109
int status = 0;
94-
while (waitpid(pid, &status, 0) < 0) {
95-
if (errno != EINTR) {
110+
for (;;) {
111+
pid_t result = waitpid(pid, &status, WNOHANG);
112+
if (result == pid) break;
113+
if (result < 0 && errno != EINTR) {
96114
log_error("recording_transcode: waitpid failed: %s", strerror(errno));
97115
return -1;
98116
}
117+
if (atomic_load(&s_stopping)) {
118+
kill(pid, SIGKILL);
119+
while (waitpid(pid, &status, 0) < 0 && errno == EINTR) {}
120+
return -1;
121+
}
122+
const struct timespec delay = { .tv_nsec = 100000000 };
123+
nanosleep(&delay, NULL);
99124
}
100125

101126
return (WIFEXITED(status) && WEXITSTATUS(status) == 0) ? 0 : -1;
@@ -105,6 +130,7 @@ static int run_and_wait(char *const argv[]) {
105130
* completed cache file. Recheck the cache because another request may have
106131
* produced it while this caller was waiting. */
107132
static int transcode_cache_locked(const char *original_path, const char *cache_path) {
133+
if (atomic_load(&s_stopping)) return -1;
108134
if (file_exists_nonempty(cache_path)) {
109135
return 0;
110136
}
@@ -193,7 +219,7 @@ static int transcode_cache_locked(const char *original_path, const char *cache_p
193219
unlink(tmp_path);
194220
}
195221
}
196-
if (rc != 0) {
222+
if (rc != 0 && !atomic_load(&s_stopping)) {
197223
log_info("recording_transcode: transcoding %s -> %s via software libx264", original_path, cache_path);
198224
rc = run_and_wait(argv_software);
199225
}
@@ -234,3 +260,71 @@ int ensure_recording_transcode_cache(const char *original_path, const char *cach
234260
pthread_mutex_unlock(&s_transcode_mutex);
235261
return rc;
236262
}
263+
264+
static void *transcode_worker(void *unused) {
265+
(void)unused;
266+
int result = ensure_recording_transcode_cache(s_job.original_path, s_job.cache_path);
267+
pthread_mutex_lock(&s_job_mutex);
268+
s_job.result = result;
269+
clock_gettime(CLOCK_MONOTONIC, &s_job.finished);
270+
s_job.active = false;
271+
pthread_mutex_unlock(&s_job_mutex);
272+
return NULL;
273+
}
274+
275+
recording_transcode_status_t request_recording_transcode_cache(
276+
const char *original_path, const char *cache_path) {
277+
if (!original_path || !original_path[0] || !cache_path || !cache_path[0] ||
278+
strlen(original_path) >= sizeof(s_job.original_path) ||
279+
strlen(cache_path) >= sizeof(s_job.cache_path)) {
280+
return RECORDING_TRANSCODE_FAILED;
281+
}
282+
if (file_exists_nonempty(cache_path)) return RECORDING_TRANSCODE_READY;
283+
284+
pthread_mutex_lock(&s_job_mutex);
285+
recording_transcode_status_t status = RECORDING_TRANSCODE_PENDING;
286+
if (atomic_load(&s_stopping)) {
287+
status = RECORDING_TRANSCODE_FAILED;
288+
} else if (file_exists_nonempty(cache_path)) {
289+
status = RECORDING_TRANSCODE_READY;
290+
} else if (!s_job.active) {
291+
if (s_job.started) {
292+
pthread_join(s_job.thread, NULL);
293+
s_job.started = false;
294+
}
295+
struct timespec now;
296+
clock_gettime(CLOCK_MONOTONIC, &now);
297+
if (s_job.result != 0 &&
298+
strcmp(original_path, s_job.original_path) == 0 &&
299+
strcmp(cache_path, s_job.cache_path) == 0 &&
300+
now.tv_sec - s_job.finished.tv_sec < 30) {
301+
status = RECORDING_TRANSCODE_FAILED;
302+
} else {
303+
strcpy(s_job.original_path, original_path);
304+
strcpy(s_job.cache_path, cache_path);
305+
s_job.active = true;
306+
int err = pthread_create(&s_job.thread, NULL, transcode_worker, NULL);
307+
if (err != 0) {
308+
log_error("recording_transcode: failed to start worker: %s", strerror(err));
309+
s_job.active = false;
310+
s_job.result = -1;
311+
s_job.finished = now;
312+
status = RECORDING_TRANSCODE_FAILED;
313+
} else {
314+
s_job.started = true;
315+
}
316+
}
317+
}
318+
pthread_mutex_unlock(&s_job_mutex);
319+
return status;
320+
}
321+
322+
void shutdown_recording_transcode(void) {
323+
pthread_mutex_lock(&s_job_mutex);
324+
atomic_store(&s_stopping, true);
325+
bool join = s_job.started;
326+
pthread_t thread = s_job.thread;
327+
s_job.started = false;
328+
pthread_mutex_unlock(&s_job_mutex);
329+
if (join) pthread_join(thread, NULL);
330+
}

‎src/web/api_handlers_recordings_playback.c‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,25 +81,44 @@ 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-
// Most browsers have no royalty-free HEVC decoder for an HTML5 <video>
85-
// element, so an HEVC recording (e.g. FrontDoor, whose native stream is
86-
// HEVC and whose recordings are a raw stream-copy of it) fails in-browser
87-
// with "no video with supported format and MIME type found" even though
88-
// it plays fine in a local player. Transparently swap in a cached H.264
89-
// copy when that's the case; the original file on disk is untouched.
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.
87+
char prepare[8];
88+
bool prepare_only = http_request_get_query_param(req, "prepare", prepare,
89+
sizeof(prepare)) > 0 &&
90+
strcmp(prepare, "1") == 0;
91+
9092
const char *serve_path = recording.file_path;
9193
char transcode_cache_path[MAX_PATH_LENGTH];
92-
if (recording_needs_hevc_transcode(recording.file_path) &&
93-
build_recording_transcode_cache_path(g_config.storage_path, id,
94-
transcode_cache_path,
95-
sizeof(transcode_cache_path)) == 0) {
96-
if (ensure_recording_transcode_cache(recording.file_path, transcode_cache_path) == 0) {
97-
serve_path = transcode_cache_path;
98-
} else {
99-
log_warn("Failed to prepare HEVC playback cache for recording %llu, "
100-
"serving original file (browser playback may fail)",
101-
(unsigned long long)id);
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;
110+
}
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;
102115
}
116+
serve_path = transcode_cache_path;
117+
}
118+
119+
if (prepare_only) {
120+
http_response_set_json(res, 200, "{\"status\":\"ready\"}");
121+
return;
103122
}
104123

105124
// Determine content type based on file extension

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ test.describe('Investigation player regressions @ui @investigation', () => {
6868
},
6969
},
7070
}));
71-
await page.route('**/api/recordings/play/568*', route => route.fulfill({
71+
await page.route('**/api/recordings/play/568*', route => route.fulfill(
72+
new URL(route.request().url()).searchParams.has('prepare')
73+
? { json: { status: 'ready' } }
74+
: {
7275
status: 200,
7376
contentType: 'video/webm',
7477
headers: { 'Accept-Ranges': 'bytes' },

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,19 @@ test.describe('Recordings Page @ui @recordings', () => {
316316
}],
317317
pagination: { total: 1, pages: 1, limit: 20 }
318318
} }));
319-
await page.route('**/api/recordings/play/601*', route => route.fulfill({ status: 204, body: '' }));
319+
let prepared = false;
320+
let preparationRequests = 0;
321+
await page.route('**/api/recordings/play/601*', route => {
322+
if (new URL(route.request().url()).searchParams.has('prepare')) {
323+
preparationRequests++;
324+
return route.fulfill({
325+
status: prepared ? 200 : 202,
326+
headers: { 'Retry-After': '1' },
327+
json: { status: prepared ? 'ready' : 'preparing' },
328+
});
329+
}
330+
return route.fulfill({ status: 204, body: '' });
331+
});
320332
await page.route('**/api/recordings/601', route => route.fulfill({ json: {
321333
id: 601,
322334
stream: 'cam1',
@@ -343,6 +355,17 @@ test.describe('Recordings Page @ui @recordings', () => {
343355
await expect(page.locator('#recordings-table')).toBeVisible();
344356
await page.locator('button[title="Play"]').first().click();
345357

358+
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');
361+
await modal.locator('button.close').click();
362+
const requestsAtClose = preparationRequests;
363+
await page.waitForTimeout(1200);
364+
expect(preparationRequests).toBe(requestsAtClose);
365+
prepared = true;
366+
await page.locator('button[title="Play"]').first().click();
367+
await expect(modal.locator('video')).toHaveAttribute('src', '/api/recordings/play/601');
368+
346369
await expect(page.locator('#video-preview-modal')).toBeVisible();
347370
await expect(page.locator('#recording-playback-position')).toHaveText('cam1 - 00:00:00');
348371

@@ -417,7 +440,11 @@ test.describe('Recordings Page @ui @recordings', () => {
417440
contentType: 'image/svg+xml',
418441
body: '<svg xmlns="http://www.w3.org/2000/svg" width="320" height="180"><rect width="320" height="180" fill="#111827"/></svg>'
419442
}));
420-
await page.route('**/api/recordings/play/602*', route => route.fulfill({ status: 204, body: '' }));
443+
await page.route('**/api/recordings/play/602*', route => route.fulfill(
444+
new URL(route.request().url()).searchParams.has('prepare')
445+
? { json: { status: 'ready' } }
446+
: { status: 204, body: '' }
447+
));
421448
await page.route('**/api/recordings/602', route => route.fulfill({ json: {
422449
id: 602,
423450
stream: 'front-door-camera-with-a-long-name',

‎tests/unit/CMakeLists.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,14 @@ add_layer2_test(test_db_camera_collections)
165165
add_layer2_test(test_api_handlers_camera_collections)
166166
add_layer2_test(test_db_recordings_extended)
167167
add_layer2_test(test_recording_transcode)
168+
add_layer2_test(test_api_handlers_recordings_playback)
169+
target_link_options(test_api_handlers_recordings_playback PRIVATE
170+
"-Wl,--wrap=get_recording_metadata_by_id"
171+
"-Wl,--wrap=httpd_authorize_camera_identity_action_with_context"
172+
"-Wl,--wrap=recording_needs_hevc_transcode"
173+
"-Wl,--wrap=request_recording_transcode_cache"
174+
"-Wl,--wrap=http_serve_file"
175+
)
168176
add_layer2_test(test_storage_manager_retention)
169177
add_layer2_test(test_storage_target_pressure_cleanup)
170178
add_layer2_test(test_storage_migration)

0 commit comments

Comments
 (0)