Conversation
| const std::string &method, const std::string &payload, | ||
| std::optional<double> response_timeout = std::nullopt); | ||
| std::string | ||
| performRpc(const std::string &destination_identity, const std::string &method, |
There was a problem hiding this comment.
did you run clang format ?
There was a problem hiding this comment.
yes, not sure how the original unformatted got in to begin with. Maybe before linting was in the pipeline?
| auto *opts = connect->mutable_options(); | ||
| opts->set_auto_subscribe(options.auto_subscribe); | ||
| opts->set_dynacast(options.dynacast); | ||
| opts->set_single_peer_connection(options.single_peer_connection); |
There was a problem hiding this comment.
may I ask why we stop calling this set_single_peer_connection() ?
There was a problem hiding this comment.
restored -- this was from a local branch that removed this due to SFUs not supporting single peer connection by default
bridge/src/rpc_manager.h
Outdated
| /// @param destination_identity Identity of the remote participant. | ||
| /// @param track_name Name of the track to mute. | ||
| /// @throws if the LocalParticipant requestTrackMute fails. | ||
| void requestTrackMute(const std::string &destination_identity, |
There was a problem hiding this comment.
why this TrackMute / TrackUnmute APIs belong to rpc_manager ? rather than the tracks ?
There was a problem hiding this comment.
this is to mute other tracks. Unclear naming, so i updated the function name to requestRemoteTrackMute/Unmute ? This was a feature asked for by Polymath.
| const std::string &method, const std::string &payload, | ||
| const std::optional<double> &response_timeout) { | ||
| assert(lp_ != nullptr); | ||
| return lp_->performRpc(destination_identity, method, payload, |
There was a problem hiding this comment.
I don't understand why we need such RpcManager ? it seems a wrapper to call local_participant APIs ?
There was a problem hiding this comment.
great question. The motivation behind the RPCManager was to be a single object that holds all RPC "work". I also think that people will be asking for "out of the box" RPC calls. For example Polymath asked for the ability to mute remote tracks. This RPC manager is a clean single place to add these "built in" RPC calls.
bridge/src/rpc_manager.h
Outdated
| /// Callback the bridge provides to execute a track action | ||
| /// (mute/unmute/release). Throws livekit::RpcError if the track is not found | ||
| /// or the action is invalid. | ||
| using TrackActionFn = std::function<void(const std::string &action, |
There was a problem hiding this comment.
why it takes std::string as action ? rather than enum (assuming it only suuport mute / unmute/release) ?
btw, not sure why we need this TrackActionFn either, it is a callback ?
There was a problem hiding this comment.
nice great point ill make it an enum (can always add more enums in the future) :)
and yeah its just a type for the callback function to make the callback setting a bit cleaner to read
stephen-derosa
force-pushed
the
sderosa/bridge-rpc
branch
from
927f6ed to
f508fec
Compare
RPCManager: bridge object for managing default and custom RPC calls bridge_rpc examples RPCManager tests
…rackActionFn action input an Enum
stephen-derosa
force-pushed
the
sderosa/bridge-rpc
branch
from
e38dd58 to
3237ad6
Compare
2 participants
Overview
RPC Manager for handling built in RPC calls and user registered RPC calls.
Built in RPC calls
Examples
examples/bridge_mute_unmute/
examples/bridge_rpc/
Testing