✨(backend) Add per recording encoding config

[cameledev]

Purpose

#1344

NB: Work based on #1357 by @sarthakbahal

Proposal

Changes on the original PR:

• encoding.py goes into settings
• profile settings are validated
• profile encoding serializers are validated and no longer use hardcoded variables
• profiles are video only and do not apply to audio
• persist query and resolved encoding settings:

recording.options["encoding"] == {
        "resolution": "720p",
        "profile": "talking_heads",
        "resolved": {
            "key_frame_interval": settings.RECORDING_ENCODING_KEY_FRAME_INTERVAL_S,
            "width": 1280,
            "height": 720,
            "framerate": 15,
            "video_bitrate": 700,
        },
    }
    ```

[lebaudantoine]

Great, love it!

Just one small concern: could we also test the API without passing any parameters, to confirm that the default resolution and profile are applied? That would help ensure we’re not introducing any unintended changes

[lebaudantoine]

If the resolution isn’t explicitly passed through the API, should it fall back to the default resolution (720p) when determining the profile values?

[cameledev]

Since profile and resolution are inherently linked, I would actually enforce providing both values. I believe this would also be clearer for future devs.

[greptile-apps]

Confidence Score: 1/5

Not safe to merge — the server will not start because the new startup consistency check always raises ValueError due to a data-format mismatch, and factories.py has stale references to settings deleted in this PR.

The startup validation check in _check_recording_encoding_maps unpacks profile dict values as positional tuples, which yields string keys instead of the intended fps/kbps values, making the comparison always fail and the server unlaunchable. resolve_encoding_config has the same mismatch and would crash on kbps_by_resolution[resolution]. factories.py was not updated alongside the removed settings, so enabling RECORDING_ENCODING_ENABLED=True causes AttributeError on every request.

src/backend/meet/settings.py (_check_recording_encoding_maps), src/backend/core/recording/worker/services.py (resolve_encoding_config), and src/backend/core/recording/worker/factories.py (from_settings) all need fixes before this can ship.

Important Files Changed

Filename	Overview
src/backend/core/recording/worker/services.py	Adds resolve_encoding_config and _resolve_encoding_options; critical mismatch — tuple-unpack code assumes list/tuple settings values but settings store dicts, producing wrong string keys instead of integer dimensions/fps
src/backend/meet/settings.py	Introduces new resolution/profile dict settings and a startup consistency check; _check_recording_encoding_maps always raises ValueError because it tuple-unpacks dict profile values, preventing the server from starting
src/backend/core/recording/worker/factories.py	Still references four removed settings (RECORDING_ENCODING_WIDTH/HEIGHT/FRAMERATE/VIDEO_BITRATE_KBPS); causes AttributeError when RECORDING_ENCODING_ENABLED=True
src/backend/core/api/serializers.py	Adds EncodingConfig Pydantic model with resolution/profile validators; logic is sound
src/backend/core/api/viewsets.py	Resolves and persists the encoding config into recording.options before DB write; logic is correct assuming resolve_encoding_config works
src/backend/core/tests/recording/worker/test_encoding_resolver.py	New test file; tests also use the same wrong tuple-unpack pattern from settings, so they would fail alongside the production code
src/backend/core/tests/recording/worker/test_mediator.py	Adds test for resolved encoding forwarding to worker; logic is correct
src/backend/core/tests/rooms/test_api_rooms_start_recording.py	Comprehensive new API tests; well-structured but would fail at runtime due to the settings/code mismatch

Comments Outside Diff (1)

1. src/backend/core/recording/worker/factories.py, line 46-56 (link)

   Stale references to settings removed by this PR

   from_settings() still reads settings.RECORDING_ENCODING_WIDTH, RECORDING_ENCODING_HEIGHT, RECORDING_ENCODING_FRAMERATE, and RECORDING_ENCODING_VIDEO_BITRATE_KBPS — all four of which were deleted from settings.py by this PR. Any operator who sets RECORDING_ENCODING_ENABLED=True will hit an AttributeError on every request that calls WorkerServiceConfig.from_settings(), breaking all video recordings.

_{Reviews (7): Last reviewed commit: "fixup! ✨(backend) add per-recording enco..." | Re-trigger Greptile}

[lebaudantoine]

LGTM! I think a deeper refactoring of the WorkerService protocol and BaseEgressService would be beneficial.

Personally, I would probably introduce a BaseVideoEgressService to override the start signature and encapsulate all the encoding-related logic. That said, it's more of a design preference than a blocker for this PR.

Thanks for your work!

[lebaudantoine]

wip add type

[greptile-apps]

Startup failure: dict unpacking yields keys, not values

_check_recording_encoding_maps tries to unpack each profile value as a two-element tuple (_fps, kbps_by_resolution), but RECORDING_ENCODING_AVAILABLE_PROFILES stores profile values as dicts like {"fps": 15, "kbps": {...}}. Python dict iteration yields keys, so after the unpack _fps = "fps" and kbps_by_resolution = "kbps". Then set("kbps") equals {'k', 'b', 'p', 's'}, which never equals {"540p", "720p", "1080p"}, so this method raises ValueError on every startup for every profile — the server will never start.

The settings values need to be lists/tuples (e.g. "talking_heads": [15, {"540p": 400, "720p": 700, "1080p": 1200}]) so that positional unpacking yields the intended fps integer and kbps dict, or alternatively the check should use dict key access (profile_config["kbps"]) instead of tuple destructuring.

[greptile-apps]

Dict unpacking produces string keys instead of integer dimensions

RECORDING_ENCODING_AVAILABLE_RESOLUTIONS["720p"] returns the dict {"width": 1280, "height": 720}. Unpacking that dict into width, height iterates over its keys, giving width = "width" and height = "height" — the integer values are never extracted. resolved then contains {"width": "width", "height": "height"}, which is either wrong data or causes a TypeError when passed to livekit_api.EncodingOptions.

The same problem occurs on line 44: fps, kbps_by_resolution = RECORDING_ENCODING_AVAILABLE_PROFILES[profile] yields the string keys "fps" and "kbps", so kbps_by_resolution[resolution] immediately raises TypeError: string indices must be integers.

Fix by either changing the settings defaults to use lists ("720p": [1280, 720], "talking_heads": [15, {"540p": 400, ...}]) or accessing them with explicit key lookups (res["width"], profile_dict["fps"], profile_dict["kbps"]).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cameledev]

Claude (opus 4.8) review

Review: per-recording encoding config (since ab707d8)

Overall

A well-executed, well-tested change. It replaces the old boolean-gated
RECORDING_ENCODING_* flat settings with a profile × resolution model: a
default encoding always applied, plus an opt-in per-recording encoding
override on the start-recording API. Startup validation, docs, and a 💥
changelog entry for the breaking settings change are all present. No
correctness bugs found — the logic is sound.

What I verified

• Resolution-only encoding relies on LiveKit treating framerate=0 /
  video_bitrate=0 as "use server default" — confirmed correct against
  livekit_egress.proto (framerate → default 30, video_bitrate →
  default 4500 for the unset/zero case). The docs' claim that a
  resolution-only request "leaves LiveKit's default framerate/bitrate" holds.
• New DB-persistence path (options["encoding"]["resolved"]): the LiveKit
  VideoCodec / AudioCodec enums are plain ints, so the resolved dict is
  JSON-serializable, round-trips through the JSONField, and reconstructs into
  EncodingOptions(**dict) cleanly. Verified in-process.
• _resolve_encoding_options(...) or _build_encoding_options() in
  services.py: protobuf EncodingOptions are always truthy (even empty), and
  the resolver returns None only for an empty/missing dict — so per-recording
  correctly wins over the default, never the reverse. No falsy-message trap.
• Env override of the new DictValue maps: DictValue.to_python uses
  ast.literal_eval, which handles the nested-dict literals fine — so
  RECORDING_ENCODING_AVAILABLE_PROFILES etc. remain operator-overridable.
• Gating in viewsets.py: with RECORDING_CUSTOM_ENCODING_ENABLED=False, a
  client-supplied encoding is rejected (400) and can't influence output;
  without encoding, the default applies in both toggle states. extra: "forbid" on both RecordingOptions and EncodingConfig blocks unknown keys.
• warnings import for the new startup check exists; the
  fall-back-to-preset path (missing default → warn + return) matches
  from_settings' if resolution and profile guard.
• Tests: ran the DB-independent suites (test_encoding_resolver.py,
  test_factories.py) — 29 passed.

Minor observations (all low severity — no action required)

1. Encoding accepted for audio-only / transcript recordings. The API
   validates and persists an encoding object regardless of mode, but
   AudioCompositeEgressService.start ignores it (documented). Harmless and
   consistent with the docs' "silently ignored," but a transcript recording
   with encoding still gets a 201 and a stored-but-inert resolved blob.
   Consider rejecting encoding when the mode has no video egress — otherwise
   fine as-is.

2. _check_recording_encoding_maps robustness (settings.py): it indexes
   profile_config["kbps"] and (in build_encoding_options)
   resolution_config["width"] without guarding shape. A malformed
   operator-provided map (via env ast.literal_eval) surfaces as a raw
   KeyError / TypeError rather than the friendly ValueError the rest of
   the method raises. Still fails fast at startup, so low impact — just a less
   clear message.

3. Default now always emits explicit advanced EncodingOptions. Previously
   the default (RECORDING_ENCODING_ENABLED=False) sent none and let LiveKit
   apply its implicit preset; now full / 720p is sent on every recording.
   Values are chosen to match H264_720P_30, and this is intentional +
   changelog-flagged. Note a couple of fields are now pinned explicitly
   (audio_frequency=48000, key_frame_interval=4.0) where the bare preset may
   have used protobuf defaults — negligible, but a behavior delta beyond the
   settings rename.