diff --git a/CHANGELOG.md b/CHANGELOG.md index 9eb62c0a..d8b30a88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to - ✨(frontend) add participant color gradient when camera is off #1490 - ✨(all) allow forcing SSO display name for authenticated users - ➕(frontend) install vite-plugin-static-copy for MediaPipe WASM assets +- ✨(backend) add per-recording encoding quality presets to start-recording API ### Changed diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 4a7a5637..a86f1bc0 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -13,7 +13,12 @@ from django.core.exceptions import SuspiciousOperation from django.utils.translation import gettext_lazy as _ from django_pydantic_field.rest_framework import SchemaField -from pydantic import BaseModel, Field, field_serializer +from pydantic import ( + BaseModel, + Field, + field_serializer, + field_validator, +) from pydantic import ValidationError as PydanticValidationError from rest_framework import serializers from rest_framework.exceptions import PermissionDenied @@ -227,6 +232,49 @@ class BaseValidationOnlySerializer(serializers.Serializer): raise NotImplementedError(f"{self.__class__.__name__} is validation-only") +class EncodingConfig(BaseModel): + """Configuration options for recording encoding. + + The allowed `resolution` and `profile` values are derived at validation time + from ``settings.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS`` and + ``settings.RECORDING_ENCODING_AVAILABLE_PROFILES``, so adding a resolution or profile + to those maps is enough to make it accepted here. + + 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. + """ + + resolution: str + profile: str | None = None + model_config = {"extra": "forbid"} + + @field_validator("resolution") + @classmethod + def _validate_resolution(cls, value): + """Reject resolutions absent from RECORDING_ENCODING_AVAILABLE_RESOLUTIONS.""" + allowed = set(settings.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS) + if value not in allowed: + raise ValueError( + f"Invalid resolution '{value}'. Choose from {sorted(allowed)}." + ) + return value + + @field_validator("profile") + @classmethod + def _validate_profile(cls, value): + """Reject profiles absent from RECORDING_ENCODING_AVAILABLE_PROFILES.""" + if value is None: + return None + allowed = set(settings.RECORDING_ENCODING_AVAILABLE_PROFILES) + if value not in allowed: + raise ValueError( + f"Invalid profile '{value}'. Choose from {sorted(allowed)}." + ) + return value + + class RecordingOptions(BaseModel): """Configuration options for recording. @@ -247,7 +295,7 @@ class RecordingOptions(BaseModel): transcribe: bool | None = None collect_metadata: bool | None = None original_mode: Literal["screen_recording", "transcript"] | None = None - + encoding: EncodingConfig | None = None model_config = {"extra": "forbid"} diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index ab8b7299..c5b1a914 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -68,6 +68,7 @@ from core.recording.worker.factories import ( from core.recording.worker.mediator import ( WorkerServiceMediator, ) +from core.recording.worker.services import resolve_encoding_config from core.services.invitation import InvitationService from core.services.livekit_events import ( LiveKitEventsService, @@ -383,12 +384,20 @@ class RoomViewSet( options = serializer.validated_data.get("options") room = self.get_object() + options_data = options.model_dump(exclude_none=True) if options else {} + if options is not None and options.encoding is not None: + # Persist the resolved encoding (concrete width/height/framerate/ + # bitrate) alongside the requested resolution/profile for traceability. + options_data["encoding"]["resolved"] = resolve_encoding_config( + options.encoding + ) + try: with transaction.atomic(): recording = models.Recording.objects.create( room=room, mode=mode, - options=options.model_dump(exclude_none=True) if options else {}, + options=options_data, ) models.RecordingAccess.objects.create( user=self.request.user, diff --git a/src/backend/core/recording/worker/factories.py b/src/backend/core/recording/worker/factories.py index c8014785..dfe97b5f 100644 --- a/src/backend/core/recording/worker/factories.py +++ b/src/backend/core/recording/worker/factories.py @@ -78,7 +78,12 @@ class WorkerService(Protocol): def __init__(self, config: WorkerServiceConfig): """Initialize the service with the given configuration.""" - def start(self, room_id: str, recording_id: str) -> str: + def start( + self, + room_id: str, + recording_id: str, + encoding_options: Optional[Dict[str, Any]] = None, + ) -> str: """Start a recording for a specified room.""" def stop(self, worker_id: str) -> str: diff --git a/src/backend/core/recording/worker/mediator.py b/src/backend/core/recording/worker/mediator.py index c73a96fe..b7798d17 100644 --- a/src/backend/core/recording/worker/mediator.py +++ b/src/backend/core/recording/worker/mediator.py @@ -47,8 +47,11 @@ class WorkerServiceMediator: raise RecordingStartError() room_name = str(recording.room.id) + encoding_options = (recording.options.get("encoding") or {}).get("resolved") try: - worker_id = self._worker_service.start(room_name, recording.id) + worker_id = self._worker_service.start( + room_name, recording.id, encoding_options=encoding_options + ) except (WorkerRequestError, WorkerConnectionError, WorkerResponseError) as e: logger.exception( "Failed to start recording for room %s: %s", recording.room.slug, e diff --git a/src/backend/core/recording/worker/services.py b/src/backend/core/recording/worker/services.py index 227e3bb0..9284a808 100644 --- a/src/backend/core/recording/worker/services.py +++ b/src/backend/core/recording/worker/services.py @@ -2,6 +2,8 @@ # pylint: disable=no-member +from django.conf import settings + from asgiref.sync import async_to_sync from livekit import api as livekit_api @@ -11,6 +13,43 @@ from .exceptions import WorkerConnectionError, WorkerResponseError from .factories import WorkerServiceConfig +def resolve_encoding_config(encoding_config): + """Resolve a per-recording EncodingConfig to concrete encoding fields. + + Returns a JSON-serializable dict of the LiveKit ``EncodingOptions`` kwargs + derived from the request's resolution / profile, or None when no + encoding_config is provided. This allows to derive width, height, fps, and + bitrate from (resolution, profile). + + Only the fields that can actually be resolved are included: width/height + require a resolution, framerate/video_bitrate require both a resolution and a + profile. + """ + if encoding_config is None: + return None + + resolution = encoding_config.resolution + profile = encoding_config.profile + + resolved = { + "key_frame_interval": settings.RECORDING_ENCODING_KEY_FRAME_INTERVAL_S, + } + + if resolution: + width, height = settings.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS[resolution] + resolved["width"] = width + resolved["height"] = height + + if resolution and profile: + fps, kbps_by_resolution = settings.RECORDING_ENCODING_AVAILABLE_PROFILES[ + profile + ] + resolved["framerate"] = fps + resolved["video_bitrate"] = kbps_by_resolution[resolution] + + return resolved + + class BaseEgressService: """Base egress defining common methods to manage and interact with LiveKit egress processes.""" @@ -76,7 +115,7 @@ class BaseEgressService: return "FAILED_TO_STOP" - def start(self, room_name, recording_id): + def start(self, room_name, recording_id, encoding_options=None): """Start the egress process for a recording (not implemented in the base class). Each derived class must implement this method, providing the necessary parameters for its specific egress type (e.g. audio_only, streaming output). @@ -99,13 +138,24 @@ class BaseEgressService: 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"]``. + """ + if not encoding_options: + return None + + return livekit_api.EncodingOptions(**encoding_options) + class VideoCompositeEgressService(BaseEgressService): """Record multiple participant video and audio tracks into a single output '.mp4' file.""" hrid = "video-recording-composite-livekit-egress" - def start(self, room_name, recording_id): + def start(self, room_name, recording_id, encoding_options=None): """Start the video composite egress process for a recording.""" # Save room's recording as a mp4 video file. @@ -126,7 +176,10 @@ class VideoCompositeEgressService(BaseEgressService): "layout": "speaker-light", } - advanced = self._build_encoding_options() + advanced = ( + self._resolve_encoding_options(encoding_options) + or self._build_encoding_options() + ) if advanced is not None: request_kwargs["advanced"] = advanced @@ -145,8 +198,13 @@ class AudioCompositeEgressService(BaseEgressService): hrid = "audio-recording-composite-livekit-egress" - def start(self, room_name, recording_id): - """Start the audio composite egress process for a recording.""" + def start(self, room_name, recording_id, encoding_options=None): + """Start the audio composite egress process for a recording. + + ``encoding_options`` is accepted for signature compatibility with the + WorkerService protocol but ignored: audio-only egress has no + encoding to configure. + """ # Save room's recording as an ogg audio file. file_type = livekit_api.EncodedFileType.OGG diff --git a/src/backend/core/tests/recording/worker/test_encoding_resolver.py b/src/backend/core/tests/recording/worker/test_encoding_resolver.py new file mode 100644 index 00000000..333b89ee --- /dev/null +++ b/src/backend/core/tests/recording/worker/test_encoding_resolver.py @@ -0,0 +1,114 @@ +"""Tests for the per-recording encoding resolution in BaseEgressService.""" + +# pylint: disable=protected-access,redefined-outer-name,unused-argument + +from unittest.mock import Mock + +from django.conf import settings + +import pytest +from pydantic import ValidationError as PydanticValidationError + +from core.api.serializers import EncodingConfig +from core.recording.worker.services import ( + VideoCompositeEgressService, + resolve_encoding_config, +) + + +def make_config(): + """Build a minimal WorkerServiceConfig-like mock for service instantiation.""" + config = Mock() + config.bucket_args = { + "endpoint": "https://s3.test.com", + "access_key": "test_key", + "secret": "test_secret", + "region": "test-region", + "bucket": "test-bucket", + "force_path_style": True, + } + config.encoding_options = None + return config + + +@pytest.fixture +def service(): + """Return a VideoCompositeEgressService with mocked handle_request.""" + svc = VideoCompositeEgressService(make_config()) + svc._handle_request = Mock() + return svc + + +# --- resolve_encoding_config --- + + +def test_resolve_config_returns_none_without_config(): + """Resolver should return None when no encoding config is provided.""" + assert resolve_encoding_config(None) is None + + +def test_resolve_config_without_profile_omits_profile_fields(): + """A resolution-only config should resolve dimensions but no framerate/bitrate.""" + resolved = resolve_encoding_config(EncodingConfig(resolution="720p")) + + assert resolved == { + "key_frame_interval": settings.RECORDING_ENCODING_KEY_FRAME_INTERVAL_S, + "width": 1280, + "height": 720, + } + + +def test_encoding_config_requires_resolution(): + """A profile-only or empty encoding config should be rejected at validation.""" + with pytest.raises(PydanticValidationError): + EncodingConfig(profile="mixed") + with pytest.raises(PydanticValidationError): + EncodingConfig() + + +# --- _resolve_encoding_options --- + + +@pytest.mark.parametrize("encoding_options", [None, {}]) +def test_resolve_options_returns_none_when_empty(service, encoding_options): + """Resolver should return None when the resolved dict is empty or missing.""" + assert service._resolve_encoding_options(encoding_options) is None + + +@pytest.mark.parametrize( + "resolution", + list(settings.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS), +) +@pytest.mark.parametrize( + "profile", + list(settings.RECORDING_ENCODING_AVAILABLE_PROFILES), +) +def test_resolve_profile_resolution_combinations(service, profile, resolution): + """Every (profile, resolution) pair should resolve to the values from settings.""" + expected_width, expected_height = settings.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS[ + resolution + ] + expected_fps, kbps_by_resolution = settings.RECORDING_ENCODING_AVAILABLE_PROFILES[ + profile + ] + expected_bitrate = kbps_by_resolution[resolution] + + resolved = resolve_encoding_config( + EncodingConfig(resolution=resolution, profile=profile) + ) + result = service._resolve_encoding_options(resolved) + + assert result.width == expected_width + assert result.height == expected_height + assert result.framerate == expected_fps + assert result.video_bitrate == expected_bitrate + + +def test_resolve_options_none_profile_uses_livekit_defaults(service): + """Missing profile should pass 0 fps/bitrate (LiveKit protobuf default).""" + resolved = resolve_encoding_config(EncodingConfig(resolution="720p")) + result = service._resolve_encoding_options(resolved) + assert result.width == 1280 + assert result.height == 720 + assert result.framerate == 0 + assert result.video_bitrate == 0 diff --git a/src/backend/core/tests/recording/worker/test_mediator.py b/src/backend/core/tests/recording/worker/test_mediator.py index 0b3fd4c3..b28f743d 100644 --- a/src/backend/core/tests/recording/worker/test_mediator.py +++ b/src/backend/core/tests/recording/worker/test_mediator.py @@ -52,7 +52,7 @@ def test_start_recording_success( # Verify worker service call expected_room_name = str(mock_recording.room.id) mock_worker_service.start.assert_called_once_with( - expected_room_name, mock_recording.id + expected_room_name, mock_recording.id, encoding_options=None ) # Verify recording updates @@ -66,6 +66,38 @@ def test_start_recording_success( ) +@mock.patch("core.utils.update_room_metadata") +def test_start_recording_passes_resolved_encoding( + mock_update_room_metadata, mediator, mock_worker_service +): + """The resolved encoding persisted in recording.options reaches the worker.""" + mock_worker_service.start.return_value = "test-worker-123" + + resolved = { + "key_frame_interval": 4.0, + "width": 1280, + "height": 720, + "framerate": 15, + "video_bitrate": 700, + } + mock_recording = RecordingFactory( + status=RecordingStatusChoices.INITIATED, + worker_id=None, + options={ + "encoding": { + "resolution": "720p", + "profile": "talking_heads", + "resolved": resolved, + } + }, + ) + mediator.start(mock_recording) + + mock_worker_service.start.assert_called_once_with( + str(mock_recording.room.id), mock_recording.id, encoding_options=resolved + ) + + @pytest.mark.parametrize( "error_class", [WorkerRequestError, WorkerConnectionError, WorkerResponseError] ) 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 1624cbaf..e7b5e066 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 @@ -470,6 +470,160 @@ def test_start_recording_options_unknown_field_rejected(settings): assert response.status_code == 400 +def test_start_recording_options_encoding_valid( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept a valid encoding configuration.""" + settings.RECORDING_ENABLE = True + 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": "720p", "profile": "talking_heads"}}, + }, + format="json", + ) + + assert response.status_code == 201 + + +def test_start_recording_persists_resolved_encoding( + settings, mock_worker_service_factory, mock_worker_manager +): + """The resolved encoding should be persisted in recording.options alongside + the requested resolution/profile for traceability.""" + settings.RECORDING_ENABLE = True + 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": "720p", "profile": "talking_heads"}}, + }, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert 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, + }, + } + + +def test_start_recording_forwards_resolved_encoding_to_worker( + settings, mock_worker_service, mock_worker_service_factory +): + """The resolved encoding should passed on to the worker.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + mock_worker_service.start.return_value = "egress-123" + + with mock.patch("core.utils.update_room_metadata"): + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + { + "mode": "screen_recording", + "options": { + "encoding": {"resolution": "720p", "profile": "talking_heads"} + }, + }, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + mock_worker_service.start.assert_called_once_with( + str(room.id), + recording.id, + encoding_options=recording.options["encoding"]["resolved"], + ) + + +def test_start_recording_options_encoding_invalid_resolution(settings): + """Should reject invalid encoding resolution values.""" + settings.RECORDING_ENABLE = True + 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": "4K"}}}, + format="json", + ) + + assert response.status_code == 400 + + +def test_start_recording_options_encoding_unknown_key_rejected(settings): + """Should reject unknown keys in encoding configuration.""" + settings.RECORDING_ENABLE = True + 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": {"bitrate": 9000}}, + }, + format="json", + ) + + assert response.status_code == 400 + + +def test_start_recording_options_without_encoding_unchanged( + settings, mock_worker_service_factory, mock_worker_manager +): + """Requests without encoding should keep existing options behavior.""" + settings.RECORDING_ENABLE = True + 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": {"language": "fr"}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {"language": "fr"} + + @pytest.mark.parametrize("value", ["foo", 12]) def test_start_recording_options_invalid_transcribe_type(settings, value): """Should reject non-boolean transcribe values.""" diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index 681ad1e1..70471049 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -754,9 +754,41 @@ class Base(Configuration): environ_prefix=None, ) + # Per-recording encoding presets for the start-recording API. + # Unlike the global RECORDING_ENCODING_* values above (a single fixed + # preset), these maps let a recording request pick a resolution and a + # quality profile. To add a resolution: add one entry to + # RECORDING_ENCODING_AVAILABLE_RESOLUTIONS and one bitrate per profile in + # RECORDING_ENCODING_AVAILABLE_PROFILES. To add a profile: add one entry to + # RECORDING_ENCODING_AVAILABLE_PROFILES with a bitrate per resolution. No + # control-flow changes needed elsewhere. + + # Map resolution string -> (width, height) in pixels. + RECORDING_ENCODING_AVAILABLE_RESOLUTIONS = values.DictValue( + { + "540p": (960, 540), + "720p": (1280, 720), + "1080p": (1920, 1080), + }, + environ_name="RECORDING_ENCODING_AVAILABLE_RESOLUTIONS", + environ_prefix=None, + ) + # Bitrate scales with resolution so quality stays consistent across sizes. + RECORDING_ENCODING_AVAILABLE_PROFILES = values.DictValue( + { + # profile: (fps, kbps per resolution) + "talking_heads": (15, {"540p": 400, "720p": 700, "1080p": 1200}), + "text": (15, {"540p": 600, "720p": 1000, "1080p": 1800}), + "mixed": (20, {"540p": 900, "720p": 1500, "1080p": 2500}), + }, + environ_name="RECORDING_ENCODING_AVAILABLE_PROFILES", + environ_prefix=None, + ) + SUMMARY_SERVICE_VERSION = values.PositiveIntegerValue( 1, environ_name="SUMMARY_SERVICE_VERSION", environ_prefix=None ) + SUMMARY_SERVICE_ENDPOINT = values.Value( None, environ_name="SUMMARY_SERVICE_ENDPOINT", environ_prefix=None ) @@ -1121,6 +1153,28 @@ class Base(Configuration): }, } + @classmethod + def _check_recording_encoding_maps(cls): + """Ensure the per-recording encoding maps are mutually consistent. + + Every profile in RECORDING_ENCODING_AVAILABLE_PROFILES must define a bitrate for + each resolution declared in RECORDING_ENCODING_AVAILABLE_RESOLUTIONS. + """ + resolutions = set(cls.RECORDING_ENCODING_AVAILABLE_RESOLUTIONS) + for profile, ( + _fps, + kbps_by_resolution, + # DictValue resolves to a dict at runtime; pylint sees the descriptor. + ) in cls.RECORDING_ENCODING_AVAILABLE_PROFILES.items(): # pylint: disable=no-member + profile_resolutions = set(kbps_by_resolution) + if profile_resolutions != resolutions: + raise ValueError( + f"Profile '{profile}' in RECORDING_ENCODING_AVAILABLE_PROFILES must " + "define a bitrate for exactly the resolutions in " + "RECORDING_ENCODING_AVAILABLE_RESOLUTIONS, mismatch on: " + f"{resolutions ^ profile_resolutions}" + ) + @classmethod def post_setup(cls): """Post setup configuration. @@ -1134,6 +1188,8 @@ class Base(Configuration): "FILE_UPLOAD_TMP_PATH cannot be the same as FILE_UPLOAD_PATH" ) + cls._check_recording_encoding_maps() + if ( cls.SUMMARY_SERVICE_VERSION == 1 and cls.SUMMARY_SERVICE_ENDPOINT is not None