RDKDEV-1681: Add Rialto Documentation - #601
gourivarma3 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new documentation contains a couple of concrete API/config inaccuracies that should be corrected to avoid misleading integrators.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a comprehensive component-level docs/README.md describing Rialto’s architecture, IPC model, lifecycle/state flows, configuration, and major internal modules to support onboarding and integration documentation for the project.
Changes:
- Introduces a new high-level Rialto overview and design explanation (client/server split, IPC, shared memory data path).
- Documents threading model, state flows, and key call flows using Mermaid diagrams.
- Adds reference tables for internal modules, component interactions/events, and configuration parameters.
File summaries
| File | Description |
|---|---|
| docs/README.md | New end-to-end architecture and integration documentation for Rialto, including diagrams and configuration reference. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
c587c4b to
da4a7c6
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The documentation has unresolved accuracy issues across IPC, lifecycle, media flows, and configuration.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (25)
docs/README.md:56
CipherModeis part of the media-segment/pipeline data model (MediaCommon.handIMediaPipeline.h), not anIMediaKeysAPI. Listing it underIMediaKeyscan send integrators to the wrong interface; move the four modes to the AV pipeline/media-segment description.
- **EME / DRM Key Management**: Exposes `IMediaKeys` for managing Encrypted Media Extension (EME) key sessions, including key generation, licence updates, session persistence, DRM store management, and cipher mode configuration (CENC, CBC1, CENS, CBCS).
docs/README.md:69
- File descriptors are not passed in-band in protobuf payloads. The
field_is_fdfields are transmitted as UnixSCM_RIGHTSancillary data and then inserted into the protobuf message byRialtoIpc; this should be described as out-of-band fd passing.
The IPC layer is purpose-built to meet Rialto's specific requirements: per-connection client identity (pid/uid), in-band file descriptor passing for sharing memory buffer descriptors, and first-class asynchronous event delivery from server to client. The protobuf service definitions in `proto/` describe all RPC methods and events for the media pipeline, DRM, web audio, control, and server manager channels.
docs/README.md:150
- The overrides file is not limited to debug builds.
ConfigHelperreads the base, SoC, and override paths wheneverRIALTO_ENABLE_CONFIG_FILEis enabled, and the override path is a build-timeRIALTO_CONFIG_OVERRIDES_PATH(default/opt/persistent/sky/rialto-config-overrides.json), not a JSONoverridesfield.
- **Configuration Files**: `rialto-config.json` (installed from `rialto-config.in.json`). The file specifies environment variables for `RialtoServer`, the server binary path, startup timeout, health-check interval, socket permissions, and number of pre-loaded server processes. On debug builds, an override file at the configured `overrides` path is also read.
docs/README.md:161
Initializing,Spawning, andShutdownare descriptive phases, not values of the publicSessionServerStateenum. The enum isUNINITIALIZED,INACTIVE,ACTIVE,NOT_RUNNING,ERROR, andSUSPENDED; the current wording omits failure and suspended paths.
The component transitions through the following states during its lifecycle: **Initializing** (read config, allocate resources) → **Spawning** (fork RialtoServer process, send SetConfiguration) → **Active** (client-facing socket ready, accepting IPC connections) → **Shutdown** (deactivate sessions, unmap shared memory, terminate RialtoServer).
docs/README.md:170
createServerManagerService()does not exist; the public factory is the free functionrialto::servermanager::service::create(stateObserver)declared inServerManagerServiceFactory.h.
AppMgr->>RSM: createServerManagerService()
docs/README.md:215
createServerManagerService(config, stateObserver)is not the public API and reverses the actual factory parameter order. The header exposesservice::create(stateObserver, config).
AppMgr->>RSM: createServerManagerService(config, stateObserver)
docs/README.md:233
- The application callback does not receive
shmInfohere:MediaPipeline::notifyNeedMediaDatacurrently passesnullptr, while the client library stores the server-provided offsets internally andaddSegment()writes through its frame writer. The prose should not instruct applications to read offsets directly from this callback.
The client writes encoded media data into the shared memory buffer at the offset indicated by `shmInfo` in the `notifyNeedMediaData` callback, then calls `addSegment()` to notify the server. The server reads the data from shared memory and pushes it into the GStreamer source element on the `WorkerThread`, completing the cycle without any additional data copy over the socket.
docs/README.md:389
- This repeated state list is also incomplete:
SUSPENDEDis a public enum value and is handled by the manager, including cleanup and later resurrection. Include it alongside the otherSessionServerStatevalues.
- **State / Lifecycle Management**: Session server state (UNINITIALIZED, INACTIVE, ACTIVE, NOT_RUNNING, ERROR) is tracked in the `RialtoServerManager`. On the server side, the `IPlaybackService` interface exposes `switchToActive()` and `switchToInactive()` to change service availability. Playback state (IDLE, PLAYING, PAUSED, SEEKING, SEEK_DONE, STOPPED, END_OF_STREAM, FAILURE) is managed within `GstGenericPlayer` and propagated via the `GstDispatcherThread`.
docs/README.md:407
- The override mechanism is not debug-only and the path is not a JSON
overridessetting.ConfigHelperreadsRIALTO_CONFIG_OVERRIDES_PATHwhenever config-file support is enabled; its default build path is/opt/persistent/sky/rialto-config-overrides.json.
| `/etc/rialto-config.json` (compiled-in default path) | Provides server manager runtime parameters: server binary path, environment variables, timeouts, socket permissions, pre-loaded server count, health-check interval, and log levels | On debug builds, a per-field override file at the configured `overrides` path is read and merged |
docs/README.md:130
RSMSpawnerlaunches theRialtoServerexecutable, not the nestedIPCServercomponent. This edge incorrectly implies that the IPC channel is the spawned process; connect the spawner to theRialtoServerprocess subgraph instead.
RSMSpawner -->|spawns| IPCServer
docs/README.md:151
- Because
RialtoServerManageris a library, it is not a daemon that must be "running" independently. The required ordering is that the application manager creates the service object before callinginitiateApplication(); please phrase this requirement in terms of service creation.
- **Startup Order**: `RialtoServerManager` must be running before any application requests `initiateApplication()`. The server manager spawns `RialtoServer` instances on demand; when `numOfPreloadedServers` is configured to a non-zero value, server processes are pre-launched at startup to reduce application connect latency.
docs/README.md:189
- The recovery counter is not limited to unanswered pings:
HealthcheckServicerecords both ping timeouts and unsuccessful acknowledgements, and restarts after the configured number of failed checks. Describing only unanswered pings gives an incorrect recovery model.
During normal operation, the server manager sends periodic `PingRequest` messages to each `RialtoServer`. The server aggregates acknowledgements from its internal session objects and responds with an `AckEvent`. If the configured number of consecutive unanswered pings (`numOfPingsBeforeRecovery`) is exceeded, the server manager triggers recovery for that server instance.
docs/README.md:244
IMediaPipeline::load()requires theisLiveargument, and the correspondingLoadRequestcarriesis_live. Omitting it from both steps makes this MSE call flow incomplete and can mislead users implementing the request.
App->>RC: IMediaPipeline::load(MSE, mimeType, url)
RC->>RS: LoadRequest (session_id, type, mime_type, url)
docs/README.md:250
- Both the IPC event and the public callback carry the need-data request ID, but this flow omits it and also shows the callback arguments in the wrong order. Without that ID the client cannot correlate the response with the pending request.
RS->>RC: NeedMediaDataEvent (source_id, frameCount, shmInfo)
RC->>App: IMediaPipelineClient::notifyNeedMediaData(source_id, frameCount, shmInfo)
docs/README.md:253
HaveDataRequestincludesnum_framesas well assession_id,status, andrequest_id; the number of frames written is computed by the client before the IPC call. Please include this field in the flow so it reflects the actual request contract.
RC->>RS: HaveDataRequest (session_id, status, requestId)
docs/README.md:200
setLogLevels()acceptsLoggingLevelsseverity values and converts them to logging masks internally; it does not take raw masks as this wording suggests. Use "log level settings" here to match the public API.
- Receiving an updated `setLogLevels()` call propagates new log level masks to all running `RialtoServer` instances at runtime via the server manager IPC channel.
docs/README.md:421
- The build default for
logLevelis3, which maps to theMILESTONEseverity threshold. The current row omits the default and describes the configuration value as a raw bitmask, although the config parser accepts a severity level and performs the conversion.
| `logLevel` | uint | — | Default log level bitmask for all Rialto components at startup |
docs/README.md:423
extraEnvVariablesis parsed as a list and its generated configuration default is an empty list ([]), not the string"". The current type/default combination is misleading for anyone constructingrialto-config.json.
| `extraEnvVariables` | list | `""` | Additional environment variables merged into the server environment |
docs/README.md:67
RialtoClientdoes not translate every media operation into an RPC:MediaPipeline::addSegment()writes the segment into the mapped shared-memory buffer, and onlyhaveData()sends the RPC. Please distinguish control RPCs from the shared-memory data path here.
Rialto is designed around strict process isolation. The `RialtoClient` library runs inside the containerised application and translates every media operation into a protobuf RPC call sent over a Unix domain socket. The `RialtoServer` process runs outside any container, holds access to GStreamer and DRM resources, and executes those operations on behalf of the client. This split ensures that hardware handles, DRM contexts, and GStreamer pipelines remain contained within the trusted server process.
docs/README.md:195
- An
ERRORstate notification only causes the manager to notifyIStateObserver; a restart is triggered by the health-check recovery path afternumOfPingsBeforeRecoveryfailed pings, not by everyStateChangedEvent(ERROR).
- A `StateChangedEvent(ERROR)` from `RialtoServer` indicates an unrecoverable server-side failure; the server manager notifies the registered `IStateObserver` and may restart the server process.
docs/README.md:233
addSegment()only writes a segment into the client's shared-memory frame writer; it does not notify the server. The client must callhaveData(status, needDataRequestId)after adding segments, which is the call that sendsHaveDataRequest.
The client writes encoded media data into the shared memory buffer at the offset indicated by `shmInfo` in the `notifyNeedMediaData` callback, then calls `addSegment()` to notify the server. The server reads the data from shared memory and pushes it into the GStreamer source element on the `WorkerThread`, completing the cycle without any additional data copy over the socket.
docs/README.md:394
MediaKeyErrorStatus::INTERFACE_NOT_IMPLEMENTEDis also a public status inmedia/public/include/MediaCommon.hand is omitted from this documented set.
- **Error Handling Strategy**: GStreamer errors received as `GST_MESSAGE_ERROR` are translated into a `PlaybackState::FAILURE` notification to the client. OpenCDM errors are mapped to `MediaKeyErrorStatus` values (OK, FAIL, BAD_SESSION_ID, NOT_SUPPORTED, INVALID_STATE, BUFFER_TOO_SMALL, OUTPUT_RESTRICTED) returned synchronously from `IMediaKeys` methods. IPC-level failures result in the client receiving a failure response or a disconnect notification.
docs/README.md:323
- This says every API call becomes a protobuf request, but
addSegment()is handled locally by writing to the shared-memory buffer; onlyhaveData()sends the corresponding protobuf request. Please scope this statement to the RPC/control path.
The client library serialises each API call into a protobuf request message and sends it over the per-application Unix domain socket. The server deserialises the request, executes the operation (typically enqueued onto `MainThread` or `WorkerThread`), and serialises the result back as a response message.
docs/README.md:265
- The
RialtoClientmodule description repeats the incorrect claim that all API calls become protobuf requests. MSEaddSegment()is the notable shared-memory data-path exception and should be documented here too.
| `RialtoClient` | Client-side library providing factory implementations for all public interfaces. Translates API calls into protobuf IPC requests and receives asynchronous event notifications from the server. | `media/client/main/`, `media/client/ipc/` |
docs/README.md:225
- As in the earlier lifecycle sequence,
StateChangedEvent(ACTIVE)is emitted fromconfigureServices()before theSetConfigurationResponseis returned. Please swap these two messages so the documented ordering matches the RPC implementation.
RS-->>RSM: SetConfigurationResponse
RS-->>RSM: StateChangedEvent(ACTIVE)
- Files reviewed: 1/1 changed files
- Comments generated: 11
- Review effort level: Lite
RDKDEV-1681: Add Rialto Documentation Reason for change: Add detailed documentation for the rialto and address review comments. Test Procedure: Documentation and review --------- Signed-off-by: Gouri G Varma <gouri_varma@comcast.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The documentation contains unresolved inaccuracies and omissions across lifecycle, threading, API behavior, and configuration.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 10
Open (10)
Document SUSPENDED as a supported session-server state · New Describe recovery as consecutive failed health checks · New Describe ERROR as recoverable when recovery is configured · New Correct media data buffering and notification responsibilities · New Include MainThread in the server API call flow · New Include WorkerThread in the EOS event flow · New Document INTERFACE_NOT_IMPLEMENTED media-key status · New Document the SoC-specific configuration layer · New Describe log-level configuration as ordinal values, not a bitmask · New Document extraEnvVariables default as an empty JSON list · New
Resolved since last review (11)
These defaults are not unset: the root CMake configuration sets bothNUM_OF_PINGS_BEFORE_RECOVERY…ApplicationStateChangeEventalso has anUNKNOWNvalue, and…NeedMediaDataEventcarriesrequest_id, and the client callback receives that ID but currently…PlaybackService::switchToInactive()clears media pipelines and web-audio players and resets… The server emitsStateChangedEvent(UNINITIALIZED)immediately after starting its app-management…ServerManagerServiceFactory::createServerManagerService()is not a public symbol, and… Nothing in this component provides a systemd service: the manager is a library hosted by an… The productionRialtoServerManageris built as a shared library…GetSharedMemoryexposes one buffer per application, and the server creates it when… The documented state list is incomplete:SessionServerStatealso hasUNINITIALIZEDand…RialtoServerManagerSimis not an IPC participant:serverManagerSim/TestService.cppstarts a…
| - **EME / DRM Key Management**: Exposes `IMediaKeys` for managing Encrypted Media Extension (EME) key sessions, including key generation, licence updates, session persistence, DRM store management, and cipher mode configuration (CENC, CBC1, CENS, CBCS). | ||
| - **Web Audio Playback**: Exposes `IWebAudioPlayer` for mixing PCM audio streams into the current audio output, with priority-based resource allocation for platforms with a limited number of concurrent audio mixers. | ||
| - **Shared Memory Data Channel**: Provides a shared memory buffer whose file descriptor is passed from server to client via the `GetSharedMemory` control RPC, allowing media segment data to be transferred without redundant data copies across process boundaries. | ||
| - **Container Lifecycle Management**: `RialtoServerManager` spawns and manages one `RialtoServer` process per application, applies resource limits (maximum simultaneous playback sessions and web audio players), manages session server states (UNINITIALIZED, INACTIVE, ACTIVE, NOT_RUNNING, ERROR), and performs periodic health checks via a ping/ack protocol. |
|
|
||
| #### Runtime State Changes | ||
|
|
||
| During normal operation, the server manager sends periodic `PingRequest` messages to each `RialtoServer`. The server aggregates acknowledgements from its internal session objects and responds with an `AckEvent`. If the configured number of consecutive unanswered pings (`numOfPingsBeforeRecovery`) is exceeded, the server manager triggers recovery for that server instance. |
|
|
||
| - Application manager calls `changeSessionServerState(appId, INACTIVE)` to suspend an active session; the server manager sends a `SetState` RPC to the corresponding `RialtoServer`, which transitions its services to inactive, releasing active decoder resources. | ||
| - Application manager calls `changeSessionServerState(appId, NOT_RUNNING)` to terminate a session; the server manager signals the `RialtoServer` to shut down cleanly. | ||
| - A `StateChangedEvent(ERROR)` from `RialtoServer` indicates an unrecoverable server-side failure; the server manager notifies the registered `IStateObserver` and may restart the server process. |
|
|
||
| The following shows the data path for an MSE playback session from client API call to GStreamer data injection. | ||
|
|
||
| The client writes encoded media data into the shared memory buffer at the offset indicated by `shmInfo` in the `notifyNeedMediaData` callback, then calls `addSegment()` to notify the server. The server reads the data from shared memory and pushes it into the GStreamer source element on the `WorkerThread`, completing the cycle without any additional data copy over the socket. |
| App->>RC: IMediaPipeline::play() | ||
| RC->>IPC: PlayRequest (session_id) | ||
| IPC->>RS: Deserialise PlayRequest | ||
| RS->>RS: Enqueue play task on WorkerThread |
| GstDisp->>RS: Notify EOS state | ||
| RS->>RS: Enqueue EOS task on MainThread |
|
|
||
| - **Event Processing**: GStreamer bus messages are polled by `GstDispatcherThread` in a loop and dispatched to `IGstDispatcherThreadClient` callbacks implemented by `GstGenericPlayer`. The player then enqueues corresponding state or notification events onto the `MainThread` for serialised processing and IPC dispatch. Need-data requests originate from the `GstSrc` element's need-data signal, also handled on the `WorkerThread`. | ||
|
|
||
| - **Error Handling Strategy**: GStreamer errors received as `GST_MESSAGE_ERROR` are translated into a `PlaybackState::FAILURE` notification to the client. OpenCDM errors are mapped to `MediaKeyErrorStatus` values (OK, FAIL, BAD_SESSION_ID, NOT_SUPPORTED, INVALID_STATE, BUFFER_TOO_SMALL, OUTPUT_RESTRICTED) returned synchronously from `IMediaKeys` methods. IPC-level failures result in the client receiving a failure response or a disconnect notification. |
|
|
||
| | Configuration File | Purpose | Override Mechanism | | ||
| | ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ | | ||
| | `/etc/rialto-config.json` (compiled-in default path) | Provides server manager runtime parameters: server binary path, environment variables, timeouts, socket permissions, pre-loaded server count, health-check interval, and log levels | On debug builds, a per-field override file at the configured `overrides` path is read and merged | |
| | `socketGroup` | string | `""` | Group name applied via `chown` to the client-facing IPC socket file | | ||
| | `numOfPreloadedServers` | int | `0` | Number of `RialtoServer` processes to pre-launch at startup to reduce application connect latency | | ||
| | `numOfPingsBeforeRecovery` | int | `3` | Number of consecutive unanswered health-check pings before the server manager triggers recovery for a server instance | | ||
| | `logLevel` | uint | `3` | Default log level bitmask for all Rialto components at startup | |
| | `numOfPingsBeforeRecovery` | int | `3` | Number of consecutive unanswered health-check pings before the server manager triggers recovery for a server instance | | ||
| | `logLevel` | uint | `3` | Default log level bitmask for all Rialto components at startup | | ||
| | `environmentVariables` | list | `XDG_RUNTIME_DIR=/tmp`, `GST_REGISTRY=/tmp/rialto-server-gstreamer-cache.bin`, `WESTEROS_SINK_USE_ESSRMGR=1` | Environment variables set for every spawned `RialtoServer` process | | ||
| | `extraEnvVariables` | list | `""` | Additional environment variables merged into the server environment | |

RDKDEV-1681: Add Rialto Documentation
Reason for Change: To add Component Documentation for Rialto
Test Procedure: https://jira.rdkcentral.com/jira/browse/RDKDEV-1681