From 653e67de56f31a935c5a9554cd8ae4a54b0bd1e2 Mon Sep 17 00:00:00 2001 From: leo <260626284+cameledev@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:31:58 +0200 Subject: [PATCH] wip --- UPGRADE.md | 38 +++++++++---- docs/features/recording.md | 4 +- src/backend/core/api/serializers.py | 4 +- .../core/recording/worker/factories.py | 12 ++-- src/backend/core/recording/worker/services.py | 30 +++------- .../worker/test_encoding_resolver.py | 57 ++++++++++++++++--- .../rooms/test_api_rooms_start_recording.py | 30 ++++++++++ src/backend/meet/settings.py | 28 +++++++-- 8 files changed, 149 insertions(+), 54 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 3331bb72..7ba28e51 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -37,18 +37,28 @@ the new defaults instead of your tuned values.** | `RECORDING_ENCODING_VIDEO_BITRATE_KBPS` | That profile's `kbps`. | `RECORDING_ENCODING_AUDIO_BITRATE_KBPS` and `RECORDING_ENCODING_KEY_FRAME_INTERVAL_S` -are unchanged and keep their values. +keep their names and meaning. The keyframe interval now defaults to `0` (unset, +encoder's choice) instead of `4.0`. #### If you never set `RECORDING_ENCODING_ENABLED=True` -No action is required. The shipped defaults (`RECORDING_ENCODING_DEFAULT_PROFILE=full`, +The shipped defaults (`RECORDING_ENCODING_DEFAULT_PROFILE=full`, `RECORDING_ENCODING_DEFAULT_RESOLUTION=720p`) match LiveKit's built-in `H264_720P_30` preset: 1280×720, 30 fps, 3000 kbps H.264 MAIN, 128 kbps AAC. +Video output is therefore unchanged. -Note that these values are now sent explicitly as advanced `EncodingOptions` -rather than relying on LiveKit's preset, so `RECORDING_ENCODING_AUDIO_BITRATE_KBPS` -and `RECORDING_ENCODING_KEY_FRAME_INTERVAL_S` now apply to every recording. They -previously applied only when `RECORDING_ENCODING_ENABLED` was `True`. +Audio and keyframing may not be. These values are now sent explicitly as advanced +`EncodingOptions` rather than relying on LiveKit's preset, so +`RECORDING_ENCODING_AUDIO_BITRATE_KBPS` and `RECORDING_ENCODING_KEY_FRAME_INTERVAL_S` +now apply to every recording. They previously applied only when +`RECORDING_ENCODING_ENABLED` was `True`. **If you set either of them while the +feature was disabled, they had no effect and now do**; check them before upgrading. + +If you never set them, no action is required: 128 kbps AAC is what the preset used, +and the keyframe interval now defaults to `0`, which leaves the field unset so the +encoder keeps picking it as before. Set `RECORDING_ENCODING_KEY_FRAME_INTERVAL_S=4.0` +if you want fixed 4-second keyframes (the value the setting defaulted to while it +was gated behind `RECORDING_ENCODING_ENABLED`). To keep letting LiveKit pick the encoding instead, set either default to an empty value: @@ -72,11 +82,14 @@ RECORDING_ENCODING_DEFAULT_RESOLUTION=720p RECORDING_ENCODING_DEFAULT_PROFILE=my_old_profile ``` -Two constraints are validated at startup and may raise a `ValueError`: +Both maps are validated at startup and a malformed one raises a `ValueError`: -- every profile in `RECORDING_ENCODING_AVAILABLE_PROFILES` must define a `kbps` - entry for **exactly** the keys of `RECORDING_ENCODING_AVAILABLE_RESOLUTIONS`; - overriding one of the two maps usually means overriding both; +- every entry of `RECORDING_ENCODING_AVAILABLE_RESOLUTIONS` must declare `width` and + `height`, and every entry of `RECORDING_ENCODING_AVAILABLE_PROFILES` an `fps` and a + `kbps` map; +- every profile must define a `kbps` entry for **exactly** the keys of + `RECORDING_ENCODING_AVAILABLE_RESOLUTIONS`; overriding one of the two maps usually + means overriding both; - `RECORDING_ENCODING_DEFAULT_RESOLUTION` and `RECORDING_ENCODING_DEFAULT_PROFILE`, when non-empty, must be keys of their respective map. @@ -84,8 +97,9 @@ Two constraints are validated at startup and may raise a `ValueError`: `RECORDING_CUSTOM_ENCODING_ENABLED` (default `False`) toggles whether the start-recording API accepts an `encoding` object -(`{"resolution": "720p", "profile": "talking_heads"}`, `profile` optional) that -overrides the default for a single recording. It does not enable or disable the +(`{"resolution": "720p", "profile": "talking_heads"}`, `profile` optional. It +falls back to `RECORDING_ENCODING_DEFAULT_PROFILE`) that overrides the default for +a single recording. It does not enable or disable the default encoding, which is built from the two `RECORDING_ENCODING_DEFAULT_*` settings either way. Leaving it at `False` preserves the previous behaviour, where every recording uses the server-side encoding: requests carrying diff --git a/docs/features/recording.md b/docs/features/recording.md index 5c177162..270a62ff 100644 --- a/docs/features/recording.md +++ b/docs/features/recording.md @@ -99,7 +99,7 @@ sequenceDiagram | **RECORDING_ENCODING_DEFAULT_RESOLUTION** | String | `"720p"` | Resolution used by the default encoding. When set, must be a key of `RECORDING_ENCODING_AVAILABLE_RESOLUTIONS`. Leave unset (together with, or instead of, the default profile) to disable the custom default encoding and fall back to LiveKit's built-in preset (a startup warning is emitted). | | **RECORDING_ENCODING_DEFAULT_PROFILE** | String | `"full"` | Profile used by the default encoding. When set, must be a key of `RECORDING_ENCODING_AVAILABLE_PROFILES`. Leave unset (together with, or instead of, the default resolution) to disable the custom default encoding and fall back to LiveKit's built-in preset (a startup warning is emitted). | | **RECORDING_ENCODING_AUDIO_BITRATE_KBPS** | Integer | `128` | AAC audio bitrate in kbps used in the default encoding. | -| **RECORDING_ENCODING_KEY_FRAME_INTERVAL_S** | Float | `4.0` | Keyframe interval in seconds. Drives seek granularity in the recorded MP4 (a player can only seek to keyframe boundaries). Larger values give the encoder slightly more bits for non-keyframe content at a fixed bitrate. `4.0` is a standard VOD value. | +| **RECORDING_ENCODING_KEY_FRAME_INTERVAL_S** | Float | `0.0` | Keyframe interval in seconds. Drives seek granularity in the recorded MP4 (a player can only seek to keyframe boundaries). Larger values give the encoder slightly more bits for non-keyframe content at a fixed bitrate. `0` leaves the field unset, letting the encoder pick; `4.0` is a standard VOD value. | > [!NOTE] @@ -135,7 +135,7 @@ Every video recording is encoded from a default resolved from `RECORDING_ENCODIN Encoding is chosen from two maps: `RECORDING_ENCODING_AVAILABLE_RESOLUTIONS` (`resolution → {"width", "height"}`) and `RECORDING_ENCODING_AVAILABLE_PROFILES` (`profile → {"fps", "kbps": {resolution: video_bitrate_kbps}}`): - **Default**: `RECORDING_ENCODING_DEFAULT_PROFILE` + `RECORDING_ENCODING_DEFAULT_RESOLUTION` set the encoding used by every recording that doesn't override it. Leave either unset to fall back to LiveKit's built-in preset (a startup warning is emitted). -- **Per recording (opt-in)**: set `RECORDING_CUSTOM_ENCODING_ENABLED=True` to let clients override the default per recording. The start-recording API then accepts an `encoding` object selecting a `resolution` (required) and `profile` (optional): a resolution-only request sets the frame size but leaves LiveKit's default framerate/bitrate; adding a profile pins fps and bitrate too. When `RECORDING_CUSTOM_ENCODING_ENABLED=False`, the API rejects any per-recording `encoding` and the default is used. +- **Per recording (opt-in)**: set `RECORDING_CUSTOM_ENCODING_ENABLED=True` to let clients override the default per recording. The start-recording API then accepts an `encoding` object selecting a `resolution` (required) and `profile` (optional): a resolution-only request keeps `RECORDING_ENCODING_DEFAULT_PROFILE` for fps and bitrate, so clients can only pick from pre-defined values. When `RECORDING_CUSTOM_ENCODING_ENABLED=False`, the API rejects any per-recording `encoding` and the default is used. The resolved values are passed straight through LiveKit's `EncodingOptions.advanced` to the GStreamer pipeline (`x264enc` for video, `faac` for audio), so there are no hidden conversions — what the profile/resolution resolve to is what the encoder receives. diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index a6f50a94..647d0a79 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -259,8 +259,8 @@ class EncodingConfig(BaseModel): Attributes: resolution: Target video resolution. - profile: Encoding profile to balance quality and CPU usage. When `None`, - LiveKit default framerate/bitrate are used for the resolution. + profile: Encoding profile to fps and kbps. When `None`, + `settings.RECORDING_ENCODING_DEFAULT_PROFILE` applies. """ resolution: str diff --git a/src/backend/core/recording/worker/factories.py b/src/backend/core/recording/worker/factories.py index 04df3ea4..5d829002 100644 --- a/src/backend/core/recording/worker/factories.py +++ b/src/backend/core/recording/worker/factories.py @@ -22,7 +22,7 @@ _RECORDING_AUDIO_CODEC = livekit_api.AudioCodec.AAC _RECORDING_AUDIO_FREQUENCY_HZ = 48000 -def build_encoding_options(resolution, profile): +def build_encoding_options(resolution, profile=None): """Assemble the LiveKit ``EncodingOptions`` kwargs for a resolution/profile. Single source of truth shared by the default encoding @@ -32,11 +32,13 @@ def build_encoding_options(resolution, profile): The profile-independent fields (audio bitrate, keyframe interval and the pinned codec / frequency constants) are always included. - The resolution-dependent fields are added only when they can be resolved: - width/height require a resolution; framerate/video_bitrate require both a - resolution and a profile (a resolution-only encoding leaves framerate and - bitrate to LiveKit's defaults). + + An omitted profile falls back to RECORDING_ENCODING_DEFAULT_PROFILE. + Framerate and bitrate are left to LiveKit only when the operator + declared no default profile at all. """ + profile = profile or settings.RECORDING_ENCODING_DEFAULT_PROFILE + options: Dict[str, Any] = { "audio_bitrate": settings.RECORDING_ENCODING_AUDIO_BITRATE_KBPS, "key_frame_interval": settings.RECORDING_ENCODING_KEY_FRAME_INTERVAL_S, diff --git a/src/backend/core/recording/worker/services.py b/src/backend/core/recording/worker/services.py index 9e739748..e6abb10b 100644 --- a/src/backend/core/recording/worker/services.py +++ b/src/backend/core/recording/worker/services.py @@ -83,28 +83,17 @@ class BaseEgressService: """ raise NotImplementedError("Subclass must implement this method.") - def _build_encoding_options(self): - """Build a LiveKit EncodingOptions from the service config, or None. + def _resolve_encoding_options(self, encoding_options): + """Build a LiveKit EncodingOptions from a resolved kwargs dict, or None. + + ``encoding_options`` is the per-recording dict persisted by the API in + ``recording.options["encoding"]["resolved"]``; it falls back to the + default encoding carried by the service config. When None is returned, the caller should omit the `advanced` field so LiveKit Egress falls back to its built-in preset (H264_720P_30). - - The full EncodingOptions kwargs (operator-tunable values + pinned - codec / frequency constants) are assembled in `WorkerServiceConfig`, - so this method is a thin protobuf adapter. - """ - opts = self._config.encoding_options - if not opts: - return None - - return livekit_api.EncodingOptions(**opts) - - def _resolve_encoding_options(self, encoding_options): - """Build LiveKit EncodingOptions from a resolved per-recording dict, or None. - - ``encoding_options`` is the dict persisted by the API in - ``recording.options["encoding"]["resolved"]``. """ + encoding_options = encoding_options or self._config.encoding_options if not encoding_options: return None @@ -137,10 +126,7 @@ class VideoCompositeEgressService(BaseEgressService): "layout": "speaker-light", } - advanced = ( - self._resolve_encoding_options(encoding_options) - or self._build_encoding_options() - ) + advanced = self._resolve_encoding_options(encoding_options) if advanced is not None: request_kwargs["advanced"] = advanced diff --git a/src/backend/core/tests/recording/worker/test_encoding_resolver.py b/src/backend/core/tests/recording/worker/test_encoding_resolver.py index 83aa1df1..cb419792 100644 --- a/src/backend/core/tests/recording/worker/test_encoding_resolver.py +++ b/src/backend/core/tests/recording/worker/test_encoding_resolver.py @@ -5,6 +5,7 @@ from unittest.mock import Mock from django.conf import settings +from django.test import override_settings import pytest from livekit import api as livekit_api @@ -41,12 +42,35 @@ def service(): # --- build_encoding_options --- -def test_build_options_without_profile_omits_profile_fields(): - """A resolution-only config should resolve dimensions but no framerate/bitrate. +def test_build_options_without_profile_uses_default_profile(): + """A resolution-only config should fall back to the default profile. - The profile-independent fields (audio bitrate, keyframe interval, codec / - frequency pins) are always present, matching the default encoding. + Left unset, framerate and video_bitrate take LiveKit's own EncodingOptions + defaults. The profile-independent fields (audio bitrate, keyframe interval, + codec/frequency pins) are always present, matching the default encoding. """ + default_profile = settings.RECORDING_ENCODING_AVAILABLE_PROFILES[ + settings.RECORDING_ENCODING_DEFAULT_PROFILE + ] + + resolved = build_encoding_options("540p") + + assert resolved == { + "audio_bitrate": settings.RECORDING_ENCODING_AUDIO_BITRATE_KBPS, + "key_frame_interval": settings.RECORDING_ENCODING_KEY_FRAME_INTERVAL_S, + "video_codec": livekit_api.VideoCodec.H264_MAIN, + "audio_codec": livekit_api.AudioCodec.AAC, + "audio_frequency": 48000, + "width": 960, + "height": 540, + "framerate": default_profile["fps"], + "video_bitrate": default_profile["kbps"]["540p"], + } + + +@override_settings(RECORDING_ENCODING_DEFAULT_PROFILE="") +def test_build_options_omits_profile_fields_without_default_profile(): + """With no default profile declared, framerate/bitrate are left to LiveKit.""" resolved = build_encoding_options("720p", None) assert resolved == { @@ -110,13 +134,32 @@ def test_resolve_profile_resolution_combinations(service, profile, resolution): assert result.audio_frequency == 48000 -def test_resolve_options_none_profile_uses_livekit_defaults(service): - """Missing profile should pass 0 fps/bitrate (LiveKit protobuf default). +def test_resolve_options_none_profile_uses_default_profile(service): + """A missing profile should resolve to the default profile's fps/bitrate.""" + default_profile = settings.RECORDING_ENCODING_AVAILABLE_PROFILES[ + settings.RECORDING_ENCODING_DEFAULT_PROFILE + ] - The pinned codec / audio fields are still applied even without a profile. + resolved = build_encoding_options("720p", None) + result = service._resolve_encoding_options(resolved) + + assert result.width == 1280 + assert result.height == 720 + assert result.framerate == default_profile["fps"] + assert result.video_bitrate == default_profile["kbps"]["720p"] + assert result.audio_bitrate == settings.RECORDING_ENCODING_AUDIO_BITRATE_KBPS + assert result.video_codec == livekit_api.VideoCodec.H264_MAIN + + +@override_settings(RECORDING_ENCODING_DEFAULT_PROFILE="") +def test_resolve_options_passes_zero_when_no_default_profile(service): + """With no default profile, fps/bitrate reach LiveKit unset (protobuf 0). + + The pinned codec / audio fields are still applied. """ resolved = build_encoding_options("720p", None) result = service._resolve_encoding_options(resolved) + assert result.width == 1280 assert result.height == 720 assert result.framerate == 0 diff --git a/src/backend/core/tests/rooms/test_api_rooms_start_recording.py b/src/backend/core/tests/rooms/test_api_rooms_start_recording.py index 88430e26..708c4e5c 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_start_recording.py +++ b/src/backend/core/tests/rooms/test_api_rooms_start_recording.py @@ -561,6 +561,36 @@ def test_start_recording_persists_resolved_encoding( } +def test_start_recording_resolution_only_uses_default_profile( + settings, mock_worker_service_factory, mock_worker_manager +): + """An encoding without a profile should resolve the default profile.""" + settings.RECORDING_ENABLE = True + settings.RECORDING_CUSTOM_ENCODING_ENABLED = True + settings.RECORDING_ENCODING_DEFAULT_PROFILE = "talking_heads" + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"encoding": {"resolution": "540p"}}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + resolved = recording.options["encoding"]["resolved"] + assert resolved["width"] == 960 + assert resolved["height"] == 540 + assert resolved["framerate"] == 15 + assert resolved["video_bitrate"] == 400 + # The requested payload is persisted as sent: no profile was asked for. + assert "profile" not in recording.options["encoding"] + + def test_start_recording_forwards_resolved_encoding_to_worker( settings, mock_worker_service, mock_worker_service_factory ): diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index 68d6ead6..fe43b498 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -819,7 +819,7 @@ class Base(Configuration): environ_prefix=None, ) RECORDING_ENCODING_KEY_FRAME_INTERVAL_S = values.FloatValue( - 4.0, + 0.0, environ_name="RECORDING_ENCODING_KEY_FRAME_INTERVAL_S", environ_prefix=None, ) @@ -1218,10 +1218,11 @@ class Base(Configuration): @classmethod def _check_recording_encoding_maps(cls): - """Ensure the per-recording encoding maps are mutually consistent. + """Ensure the per-recording encoding maps are well-formed and consistent. - Every profile in RECORDING_ENCODING_AVAILABLE_PROFILES must define a bitrate for - each resolution declared in RECORDING_ENCODING_AVAILABLE_RESOLUTIONS. + Each entry of RECORDING_ENCODING_AVAILABLE_RESOLUTIONS must declare a width and + a height, each entry of RECORDING_ENCODING_AVAILABLE_PROFILES an fps and a kbps + map, and every profile must define a bitrate for each declared resolution. The default profile / resolution feed the default encoding. When either is missing, no custom default encoding can be built: a warning is emitted and @@ -1231,10 +1232,29 @@ class Base(Configuration): resolutions = set(cls.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS) profiles = set(cls.RECORDING_ENCODING_AVAILABLE_PROFILES) # DictValue resolves to a dict at runtime; pylint sees the descriptor. + for ( + resolution, + resolution_config, + ) in cls.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS.items(): # pylint: disable=no-member + missing_keys = {"width", "height"} - set(resolution_config) + if missing_keys: + raise ValueError( + f"Resolution '{resolution}' in " + "RECORDING_ENCODING_AVAILABLE_RESOLUTIONS is missing the key(s): " + f"{sorted(missing_keys)}." + ) + for ( profile, profile_config, ) in cls.RECORDING_ENCODING_AVAILABLE_PROFILES.items(): # pylint: disable=no-member + missing_keys = {"fps", "kbps"} - set(profile_config) + if missing_keys: + raise ValueError( + f"Profile '{profile}' in RECORDING_ENCODING_AVAILABLE_PROFILES is " + f"missing the key(s): {sorted(missing_keys)}." + ) + profile_resolutions = set(profile_config["kbps"]) if profile_resolutions != resolutions: raise ValueError(