From ad7f760b6b8957c0236fb58ea510ec3608febe43 Mon Sep 17 00:00:00 2001 From: root Date: Mon, 5 Oct 2026 11:45:11 -0400 Subject: [PATCH 1/2] fix(recordings): set explicit rate control on VAAPI playback transcode The HEVC playback-compatibility transcode encoded with h264_vaapi and no rate-control options, so the driver used its default quality level ("No quality level set; using default (20)") with no bound on bitrate. Pass -rc_mode CQP -qp 24. CQP rather than -b:v/-maxrate because some drivers (e.g. Intel HD 630) support only CQP and refuse to open the encoder for bitrate modes. A driver that rejects CQP still falls through to the existing software path. Measured on a 10 s 2560x1920 HEVC camera clip: 8.3 -> 5.0 Mbit/s (10.4 -> 6.3 MB, -40%). The new test wraps ffmpeg on PATH to record the VAAPI invocation's arguments and exit status, so it also proves the driver accepted the options rather than the cache being produced by the software fallback. It is skipped on hosts without a render node. Co-Authored-By: Claude Opus 5.5 --- src/video/recording_transcode.c | 12 +++- tests/unit/test_recording_transcode.c | 99 ++++++++++++++++++++++----- 2 files changed, 92 insertions(+), 19 deletions(-) diff --git a/src/video/recording_transcode.c b/src/video/recording_transcode.c index 59db4661..a1050228 100644 --- a/src/video/recording_transcode.c +++ b/src/video/recording_transcode.c @@ -187,14 +187,22 @@ static int transcode_cache_locked(const char *original_path, const char *cache_p * When the source has no audio stream, ffmpeg's default stream * selection simply produces no audio output track, so this is a no-op * for video-only sources (e.g. any recording made while audio_disabled - * is set). */ + * is set). + * + * -rc_mode CQP -qp 24 sets rate control explicitly. Without it h264_vaapi + * uses the driver's default quality level with no bound on bitrate, + * which produced ~21 Mbit/s from a 2560x1920 source. CQP is used rather + * than -b:v/-maxrate because some drivers (e.g. Intel HD 630) support + * only CQP and refuse to open the encoder otherwise. If a driver rejects + * it anyway, the software fallback below still produces the cache. */ char *argv_vaapi[] = { (char *)FFMPEG_BINARY, "-y", "-hide_banner", "-loglevel", "error", "-nostdin", "-filter_threads", "1", "-threads", "1", "-hwaccel", "vaapi", "-hwaccel_device", VAAPI_RENDER_NODE, "-hwaccel_output_format", "vaapi", "-i", (char *)original_path, - "-c:v", "h264_vaapi", "-threads", "1", "-c:a", "aac", + "-c:v", "h264_vaapi", "-rc_mode", "CQP", "-qp", "24", + "-threads", "1", "-c:a", "aac", "-f", "mp4", tmp_path, NULL }; /* Bound decoder/encoder threads explicitly: FFmpeg otherwise sizes them diff --git a/tests/unit/test_recording_transcode.c b/tests/unit/test_recording_transcode.c index bbfaa22b..2fcea3f4 100644 --- a/tests/unit/test_recording_transcode.c +++ b/tests/unit/test_recording_transcode.c @@ -334,7 +334,7 @@ void test_ensure_cache_concurrent_calls_for_same_recording_dont_corrupt_cache(vo * checking the final cache (which passed even when duplicate jobs caused OOM). * Holding each invocation briefly makes overlapping requests exercise a cache * miss. Force software encoding so these checks also work on VAAPI hosts. */ -static void track_ffmpeg_processes(void) { +static void install_ffmpeg_wrapper(const char *script) { char real_ffmpeg[1024]; FILE *pipe = popen("command -v ffmpeg", "r"); TEST_ASSERT_NOT_NULL(pipe); @@ -348,22 +348,7 @@ static void track_ffmpeg_processes(void) { snprintf(wrapper, sizeof(wrapper), "%s/ffmpeg", g_test_dir); FILE *file = fopen(wrapper, "w"); TEST_ASSERT_NOT_NULL(file); - fputs("#!/bin/sh\n" - "for arg do\n" - " [ \"$arg\" != h264_vaapi ] || exit 1\n" - "done\n" - "printf 'start\\n' >> \"$LIGHTNVR_TEST_TRANSCODE_DIR/starts\"\n" - "owned=0\n" - "if mkdir \"$LIGHTNVR_TEST_TRANSCODE_DIR/active\"; then\n" - " owned=1\n" - "else\n" - " touch \"$LIGHTNVR_TEST_TRANSCODE_DIR/overlap\"\n" - "fi\n" - "sleep 1\n" - "\"$LIGHTNVR_TEST_REAL_FFMPEG\" \"$@\"\n" - "rc=$?\n" - "if [ \"$owned\" = 1 ]; then rmdir \"$LIGHTNVR_TEST_TRANSCODE_DIR/active\"; fi\n" - "exit \"$rc\"\n", file); + fputs(script, file); TEST_ASSERT_EQUAL_INT(0, fclose(file)); TEST_ASSERT_EQUAL_INT(0, chmod(wrapper, 0700)); @@ -378,6 +363,85 @@ static void track_ffmpeg_processes(void) { TEST_ASSERT_EQUAL_INT(0, rc); } +static void track_ffmpeg_processes(void) { + install_ffmpeg_wrapper( + "#!/bin/sh\n" + "for arg do\n" + " [ \"$arg\" != h264_vaapi ] || exit 1\n" + "done\n" + "printf 'start\\n' >> \"$LIGHTNVR_TEST_TRANSCODE_DIR/starts\"\n" + "owned=0\n" + "if mkdir \"$LIGHTNVR_TEST_TRANSCODE_DIR/active\"; then\n" + " owned=1\n" + "else\n" + " touch \"$LIGHTNVR_TEST_TRANSCODE_DIR/overlap\"\n" + "fi\n" + "sleep 1\n" + "\"$LIGHTNVR_TEST_REAL_FFMPEG\" \"$@\"\n" + "rc=$?\n" + "if [ \"$owned\" = 1 ]; then rmdir \"$LIGHTNVR_TEST_TRANSCODE_DIR/active\"; fi\n" + "exit \"$rc\"\n"); +} + +/* Regression test: the VAAPI encode used to pass no rate-control options at + * all, so h264_vaapi fell back to the driver's default quality level ("No + * quality level set; using default (20)") with no bound on bitrate. On a + * 2560x1920 source that produced ~21 Mbit/s output, large enough to make + * browser playback stall. Record the arguments of the VAAPI invocation and + * its exit status, then pass through to the real encoder: the status check + * proves the driver actually accepted the options rather than the request + * quietly succeeding via the software fallback. Needs a render node, so it + * is skipped on GPU-less hosts such as CI. */ +void test_vaapi_transcode_sets_explicit_rate_control(void) { + if (s_skip) { TEST_IGNORE_MESSAGE("ffmpeg/ffprobe not found in PATH"); return; } + if (access("/dev/dri/renderD128", F_OK) != 0) { + TEST_IGNORE_MESSAGE("no VAAPI render node"); + return; + } + install_ffmpeg_wrapper( + "#!/bin/sh\n" + "vaapi=0\n" + "for arg do\n" + " [ \"$arg\" != h264_vaapi ] || vaapi=1\n" + "done\n" + "if [ \"$vaapi\" = 0 ]; then exec \"$LIGHTNVR_TEST_REAL_FFMPEG\" \"$@\"; fi\n" + "printf '%s\\n' \"$@\" > \"$LIGHTNVR_TEST_TRANSCODE_DIR/vaapi_args\"\n" + "\"$LIGHTNVR_TEST_REAL_FFMPEG\" \"$@\"\n" + "rc=$?\n" + "echo \"$rc\" > \"$LIGHTNVR_TEST_TRANSCODE_DIR/vaapi_rc\"\n" + "exit \"$rc\"\n"); + + char cache_path[320]; + snprintf(cache_path, sizeof(cache_path), "%s/rate_control_out.mp4", g_test_dir); + TEST_ASSERT_EQUAL_INT(0, ensure_recording_transcode_cache(g_hevc_fixture, cache_path)); + + char path[320]; + snprintf(path, sizeof(path), "%s/vaapi_args", g_test_dir); + FILE *file = fopen(path, "r"); + TEST_ASSERT_NOT_NULL_MESSAGE(file, "VAAPI encoder was never invoked"); + char prev[256] = ""; + char line[256]; + bool has_cqp = false; + bool has_qp = false; + while (fgets(line, sizeof(line), file)) { + line[strcspn(line, "\n")] = '\0'; + if (strcmp(prev, "-rc_mode") == 0 && strcmp(line, "CQP") == 0) has_cqp = true; + if (strcmp(prev, "-qp") == 0 && atoi(line) > 0) has_qp = true; + snprintf(prev, sizeof(prev), "%s", line); + } + fclose(file); + TEST_ASSERT_TRUE_MESSAGE(has_cqp, "VAAPI encode is missing -rc_mode CQP"); + TEST_ASSERT_TRUE_MESSAGE(has_qp, "VAAPI encode is missing -qp "); + + snprintf(path, sizeof(path), "%s/vaapi_rc", g_test_dir); + file = fopen(path, "r"); + TEST_ASSERT_NOT_NULL(file); + int vaapi_rc = -1; + TEST_ASSERT_EQUAL_INT(1, fscanf(file, "%d", &vaapi_rc)); + fclose(file); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, vaapi_rc, "driver rejected the VAAPI rate-control options"); +} + static void assert_ffmpeg_process_counts(int expected_starts) { char path[320]; snprintf(path, sizeof(path), "%s/starts", g_test_dir); @@ -559,6 +623,7 @@ int main(void) { RUN_TEST(test_ensure_cache_skips_retranscode_when_cache_already_exists); RUN_TEST(test_ensure_cache_fails_gracefully_for_missing_source); RUN_TEST(test_ensure_cache_concurrent_calls_for_same_recording_dont_corrupt_cache); + RUN_TEST(test_vaapi_transcode_sets_explicit_rate_control); RUN_TEST(test_concurrent_playback_requests_share_one_transcode); RUN_TEST(test_different_recordings_transcode_one_at_a_time); RUN_TEST(test_failed_transcode_releases_slot_for_retry); From e1a254949fd9175e9e603d13af4518da278e67ac Mon Sep 17 00:00:00 2001 From: root Date: Tue, 6 Oct 2026 03:45:20 -0400 Subject: [PATCH 2/2] test(recordings): skip VAAPI rate-control test without a usable pipeline Address review feedback on the VAAPI rate-control regression test: - Probe the baseline VAAPI HEVC-decode / H.264-encode pipeline (without rate-control options) and skip when it fails, rather than gating only on the render node path existing. Production falls back to software on hosts with an inaccessible node, FFmpeg built without VAAPI, or a GPU lacking HEVC decode, so the test should not fail there; the probe also isolates the final status assertion to the new options themselves. - Assert -qp is exactly 24 instead of any positive value, so an accidental QP change is caught. Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_recording_transcode.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_recording_transcode.c b/tests/unit/test_recording_transcode.c index 2fcea3f4..73c6703a 100644 --- a/tests/unit/test_recording_transcode.c +++ b/tests/unit/test_recording_transcode.c @@ -390,12 +390,25 @@ static void track_ffmpeg_processes(void) { * browser playback stall. Record the arguments of the VAAPI invocation and * its exit status, then pass through to the real encoder: the status check * proves the driver actually accepted the options rather than the request - * quietly succeeding via the software fallback. Needs a render node, so it - * is skipped on GPU-less hosts such as CI. */ + * quietly succeeding via the software fallback. Needs a working VAAPI + * HEVC-decode/H.264-encode pipeline, so it is skipped on hosts without one + * (GPU-less CI, an inaccessible render node, FFmpeg built without VAAPI); + * production falls back to software in all of those cases. */ void test_vaapi_transcode_sets_explicit_rate_control(void) { if (s_skip) { TEST_IGNORE_MESSAGE("ffmpeg/ffprobe not found in PATH"); return; } - if (access("/dev/dri/renderD128", F_OK) != 0) { - TEST_IGNORE_MESSAGE("no VAAPI render node"); + if (access("/dev/dri/renderD128", R_OK | W_OK) != 0) { + TEST_IGNORE_MESSAGE("no accessible VAAPI render node"); + return; + } + /* Probe the baseline pipeline without rate-control options, so a failure + * below can only mean those options broke an otherwise usable path. */ + char probe[1024]; + snprintf(probe, sizeof(probe), + "-hwaccel vaapi -hwaccel_device /dev/dri/renderD128 " + "-hwaccel_output_format vaapi -i \"%s\" -c:v h264_vaapi -an -f null -", + g_hevc_fixture); + if (run_ffmpeg(probe) != 0) { + TEST_IGNORE_MESSAGE("VAAPI HEVC decode / H.264 encode not usable on this host"); return; } install_ffmpeg_wrapper( @@ -426,12 +439,12 @@ void test_vaapi_transcode_sets_explicit_rate_control(void) { while (fgets(line, sizeof(line), file)) { line[strcspn(line, "\n")] = '\0'; if (strcmp(prev, "-rc_mode") == 0 && strcmp(line, "CQP") == 0) has_cqp = true; - if (strcmp(prev, "-qp") == 0 && atoi(line) > 0) has_qp = true; + if (strcmp(prev, "-qp") == 0 && strcmp(line, "24") == 0) has_qp = true; snprintf(prev, sizeof(prev), "%s", line); } fclose(file); TEST_ASSERT_TRUE_MESSAGE(has_cqp, "VAAPI encode is missing -rc_mode CQP"); - TEST_ASSERT_TRUE_MESSAGE(has_qp, "VAAPI encode is missing -qp "); + TEST_ASSERT_TRUE_MESSAGE(has_qp, "VAAPI encode is missing -qp 24"); snprintf(path, sizeof(path), "%s/vaapi_rc", g_test_dir); file = fopen(path, "r");